Re: [Django] #13666: Updates with F objects on decimal fields raise MySQL warnings
Django
-------------------------------------+-------------------------------------
Reporter: KyleMac | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.2
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by anonymous):
I recently encountered this exact same issue on MySQL 5.6.10 and Django
1.4.
The following SQL (which is what the Python MySQL adapter generates) fails
reliably for me:
{{{
UPDATE `main_account`
SET `balance` = `main_userprofile`.`balance` - '0.00007'
WHERE `main_account`.`id` = 1;
}}}
However, adding explicit casts to decimal fixes the issue.
{{{
UPDATE `main_account`
SET `balance` = `main_userprofile`.`balance` - CAST('0.00007' AS
DECIMAL(12, 8))
WHERE `main_account`.`id` = 1;
}}}
I assume this is because in the former case MySQL casts to a float, then
subtracts, introducing precision issues, while in the latter case we
explicitly cast directly to decimal of a certain precision.
I don't know what the solution is from a Django perspective, but thought
the information could be useful.
--
Ticket URL: <https://code.djangoproject.com/ticket/13666#comment:10>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Reporter: KyleMac | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.2
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by anonymous):
Is patching django.db.backends.mysql.django_conversions with cast() the
proper way to go?
--
Ticket URL: <https://code.djangoproject.com/ticket/13666#comment:11>
Django
-------------------------------------+-------------------------------------
Reporter: KyleMac | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by halfnibble):
* cc: josh@… (added)
* version: 1.2 => 1.7
Comment:
The Django ORM definitely needs to use CAST with MySQL. Otherwise, this
bug may manifest randomly on various architectures.
Read this on MySQL Type Conversion:
''"""Furthermore, the conversion from string to floating-point and from
integer to floating-point do not necessarily occur the same way. The
integer may be converted to floating-point by the CPU, whereas the string
is converted digit by digit in an operation that involves floating-point
multiplications.
''The results shown will vary on different systems, and can be affected by
factors such as computer architecture or the compiler version or
optimization level. One way to avoid such problems is to use CAST() so
that a value is not converted implicitly to a float-point number:"""''''
[http://dev.mysql.com/doc/refman/5.6/en/type-conversion.html]
--
Ticket URL: <https://code.djangoproject.com/ticket/13666#comment:12>
Django
-------------------------------------+-------------------------------------
Reporter: KyleMac | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.7
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by jarshwah):
Wouldn't it make sense to pass the number verbatim rather than
stringifying it? Or could that introduce inconsistencies with floats being
imprecise? What about the other backends, do they all accept string-like
decimal numbers and happily call them decimals?
If we want to go with a wrapping type though, it should be possible (from
1.8/master) to provide an `as_mysql()` for the Value expression.
Something like:
{{{
class Value(..):
def as_mysql(self, compiler, connection):
if self.output_field.get_internal_type() == 'DecimalField':
c = Cast(self, self.output_field)
return compiler.compile(c)
}}}
Where the `Cast` type above would need to be implemented in some way. The
implementation could be thorough and apply to all backends (quite a bit of
work), or it could be extremely simple, and only work for Decimal types on
MySQL. There's similar logic for Oracle with the `Coalesce` type and
conversions to NCLOB.
I'd be wary about providing the digits and precision arguments to the CAST
if they aren't exactly needed. If they can be avoided, then do so. Because
there's no nice way (currently) to pass that information.
--
Ticket URL: <https://code.djangoproject.com/ticket/13666#comment:13>
Django
-------------------------------------+-------------------------------------
Reporter: KyleMac | Owner: nobody
Component: Database layer | Version: 1.7
Severity: Normal | worksforme
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => worksforme
Comment:
I'm not able to reproduce anymore on MySQL 8 strict mode (minimum version
planned for 4.2) so they must have sorted out their decimal arithmetic
over the years so even if the `Cast` could be added by
`CombinedExpression` or even by MySQL's `combine_expression` I don't think
it's worth doing at this point.
Closing as ''worksforme'' as demonstrated by this test.
{{{#!diff
diff --git a/tests/model_fields/models.py b/tests/model_fields/models.py
index 9a64d51766..2a9bcd95cc 100644
--- a/tests/model_fields/models.py
+++ b/tests/model_fields/models.py
@@ -16,6 +16,10 @@
except ImportError:
Image = None
+class Account(models.Model):
+ name = models.CharField(max_length=100)
+ balance = models.DecimalField(decimal_places=2, max_digits=12)
+
class Foo(models.Model):
a = models.CharField(max_length=10)
diff --git a/tests/model_fields/test_decimalfield.py
b/tests/model_fields/test_decimalfield.py
index 912d55af72..a45e634ff4 100644
--- a/tests/model_fields/test_decimalfield.py
+++ b/tests/model_fields/test_decimalfield.py
@@ -3,10 +3,10 @@
from django.core import validators
from django.core.exceptions import ValidationError
-from django.db import models
+from django.db import models, connection
from django.test import TestCase
-from .models import BigD, Foo
+from .models import Account, BigD, Foo
class DecimalFieldTests(TestCase):
@@ -113,3 +113,14 @@ def test_roundtrip_with_trailing_zeros(self):
obj = Foo.objects.create(a="bar", d=Decimal("8.320"))
obj.refresh_from_db()
self.assertEqual(obj.d.compare_total(Decimal("8.320")),
Decimal("0"))
+
+ def test_update_difference(self):
+ acc = Account.objects.create(balance="10.00")
+ acc.balance = models.F("balance") - "1.79"
+ acc.save()
+ acc.refresh_from_db()
+ self.assertEqual(acc.balance, Decimal("8.21"))
+ with connection.cursor() as cursor:
+ cursor.execute("SHOW WARNINGS")
+ self.assertIsNone(cursor.fetchone())
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/13666#comment:14>