[Django] #31683: Overridden methods ignored in models.DecimalField subclass
Django
-------------------------------------+-------------------------------------
Reporter: | Owner: nobody
maxrothman |
Type: Bug | Status: new
Component: Database | Version: 2.2
layer (models, ORM) |
Severity: Release | Keywords: custom-model-fields
blocker |
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
Django: 2.2.4
Python: 3.6.9
Mysql: 5.6.47
It appears to be impossible to override any methods in subclasses of
{{{models.DecimalField}}} under certain circumstances. Consider the
following:
{{{#!python
class Field1(models.DecimalField):
def get_prep_value(self, value):
1/0
class Field2(models.PositiveIntegerField):
def get_prep_value(self, value):
1/0
class TestModel1(models.Model):
m = Field1(max_digits=5, decimal_places=2)
class TestModel2(models.Model):
m = Field2()
TestModel1(m=1).save() # No error!
TestModel2(m=1).save() # Error, as expected
}}}
I'd expect to get an error when saving {{{TestModel1}}} because
get_prep_value should throw a {{{ZeroDivisionError}}}, but as far as I can
tell, {{{Field1.get_prep_value}}} is never called.
This bug appears to be somewhat sensitive to platform conditions. I've
been able to replicate it consistently on my machine, but another one of
my colleagues who tried was not.
--
Ticket URL: <https://code.djangoproject.com/ticket/31683>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: closed
Component: Database layer | Version: 2.2
(models, ORM) |
Severity: Normal | Resolution: invalid
Keywords: custom-model-fields | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => invalid
* severity: Release blocker => Normal
Comment:
Overriding works fine. `DecimalField` doesn't call `get_prep_value()` on
`.save()` because it has a custom `get_db_prep_save()` implementation:
{{{
>>> Field1().get_prep_value('asdasd')
ZeroDivisionError: division by zero
>>> TestModel1.objects.filter(m='3.44')
ZeroDivisionError: division by zero
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/31683#comment:1>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: closed
Component: Database layer | Version: 2.2
(models, ORM) |
Severity: Normal | Resolution: invalid
Keywords: custom-model-fields | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Max Rothman):
The docs do not mention anywhere that {{{get_prep_value}}} is expected to
be called in {{{get_db_prep_value}}} on save, nor that {{{DecimalField}}}
behaves differently from most other fields in this respect. Could this be
reopened as a documentation bug?
--
Ticket URL: <https://code.djangoproject.com/ticket/31683#comment:2>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: closed
Component: Database layer | Version: 2.2
(models, ORM) |
Severity: Normal | Resolution: invalid
Keywords: custom-model-fields | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by felixxm):
> The docs do not mention anywhere that {{{get_prep_value}}} is expected
to be called in {{{get_db_prep_value}}} on save,...
It's not required, I just wanted to explain why exception is raised in
`.save()` for `PositiveIntegerField`.
> ...nor that {{{DecimalField}}} behaves differently from most other
fields in this respect. Could this be reopened as a documentation bug?
I don't think so, we cannot document implementation details of all builtin
fields.
--
Ticket URL: <https://code.djangoproject.com/ticket/31683#comment:3>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: closed
Component: Database layer | Version: 2.2
(models, ORM) |
Severity: Normal | Resolution: invalid
Keywords: custom-model-fields | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Max Rothman):
So as a django user the way I was supposed to find this out was to read
the source code? The fact that this issue exists is proof that there is at
least some kind of documentation issue.
--
Ticket URL: <https://code.djangoproject.com/ticket/31683#comment:4>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: closed
Component: Database layer | Version: 2.2
(models, ORM) |
Severity: Normal | Resolution: invalid
Keywords: custom-model-fields | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by felixxm):
Replying to [comment:4 Max Rothman]:
> So as a django user the way I was supposed to find this out was to read
the source code?
Writing a custom model field is quite advanced usage. You can create a
subclass of `Field` and implement methods described in the How-to. If you
want to create a subclass of the builtin fields you can also implement all
methods or check an implementation and use relations between them. I don't
see any contradiction.
> The fact that this issue exists is proof that there is at least some
kind of documentation issue.
I don't see any issue here. If you want to raise an exception when
converting values to database values you should override
`get_db_prep_value()` as [https://docs.djangoproject.com/en/dev/howto
/custom-model-fields/#converting-query-values-to-database-values
documented].
--
Ticket URL: <https://code.djangoproject.com/ticket/31683#comment:5>