#27825: ORM Object constructor does not cast types of it's attributes when
called
by reference
-------------------------------------+-------------------------------------
Reporter: Oleg Belousov | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.10
(models, ORM) |
Severity: Normal | Resolution:
Keywords: ORM | Triage Stage:
| Someday/Maybe
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Josh Smeaton):
The original wording of this post is confusing, because it *seems* to
suggest that using `Model.objects.create(decimal_field=123.12)` will do
the expected thing and type cast to a Decimal, but it does not.
{{{
class Costing(models.Model):
cost = models.DecimalField(max_digits=20, decimal_places=2)
rev = models.DecimalField(max_digits=20, decimal_places=2)
ts = models.DateField(db_index=True)
In [31]: c = Costing.objects.create(cost=Decimal('123.12'), rev=123.12,
ts=datetime.now().date())
In [32]: print(type(c.rev))
<type 'float'>
In [33]: c.rev
Out[33]: 123.12
}}}
While this might look different to #24028 I think it is the same. In the
linked ticket, the related field has been cached and isn't reloaded and
brought through the various adapters and converters. Here, it's simply the
local field that hasn't been reloaded and gone through various
transformations.
The only fix that will work for every field is to refetch it from the
database, and rely on the adapters provided by the driver, plus the
converters provided by the backend and fields, to cast the type correctly.
That's a non-starter, because we'd need to do a SELECT for every INSERT or
UPDATE, which harms performance for all the users handing the constructor
correct data.
You could argue that save could accept a flag to do an automatic fetch,
but this wouldn't help you in your situation. It's unlikely that users
would pass in such a flag just in case bad data got in - they'd only use
it when they *know* they need to refresh from the database, like when
using expressions to update attributes. At that point, using
refresh_from_db is nearly as good.
> Personally, this bug (one way binding between application and db on
save) broke many of my tests and took a lot of my time.
The first bug was in your own program. Handing a float to a decimal field
is wrong. Let me show you a contrived example:
{{{
# Make the decimal field store a large decimal component
class Costing(models.Model):
cost = models.DecimalField(max_digits=20, decimal_places=2)
rev = models.DecimalField(max_digits=30, decimal_places=18)
ts = models.DateField(db_index=True)
In [3]: c = Costing.objects.create(cost=Decimal('123.12'), rev=0.1 + 0.1 +
0.1, ts=datetime.now().date())
In [4]: c.refresh_from_db()
In [5]: c.rev
Out[5]: Decimal('0.300000000000000044')
}}}
That said, the behaviour of django models here is certainly surprising
which is not a good thing. Decimal fields are among the worst culprits,
because they'll silently accept and save wrong data. I'd be more inclined
to accept a patch enforcing only decimals being assigned to decimal fields
(or having the descriptor wrap any arguments in a `Decimal()`, which would
be a breaking change for a lot of people, but would highlight a number of
broken programs. Digressing a little, I'm also surprised the decimal type
in python itself allows floats as arguments, because the same broken
behaviour is possible there.
> one way binding between application and db on save
We need to make this clearer in our docs I think. Without doing a full
refresh of the object, it's impossible to convert input to the eventual
output for every field type.
--
Ticket URL: <https://code.djangoproject.com/ticket/27825#comment:9>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/064.eeb37b9329c53bcfbf89129d7bd326d4%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
