Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#33232 closed Bug (invalid)

DecimalField rounding error when upgrading from 3.1 to 3.2 on Postgres

Reported by: Michał Szczepańczyk Owned by: nobody
Component: Database layer (models, ORM) Version: 3.2
Severity: Normal Keywords: decimal, decimalfield, rounding, postgres
Cc: Simon Charette Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Example:

from django.db import models

class Foo(models.Model):
    amount = models.DecimalField(decimal_places=2, max_digits=12)

from myapp.models import Foo
f = Foo(amount='1.125')
f.save()
f.refresh_from_db()

# the issue is here
f.amount  # returns Decimal('1.12') in 3.1 and Decimal('1.13') in 3.2

Executed query for 3.2:

INSERT INTO "myapp_foo" ("amount") VALUES (1.125) RETURNING "myapp_foo"."id"

Executed query for 3.1:

INSERT INTO "myapp_foo" ("amount") VALUES ('1.12') RETURNING "myapp_foo"."id"

Issue was reproduced only when using postgres database.

Change History (6)

comment:1 by Michał Szczepańczyk, 3 years ago

Summary: DecimalField rounding error when upgrading from 3.1 to 3.2DecimalField rounding error when upgrading from 3.1 to 3.2 on Postgres

comment:2 by Mariusz Felisiak, 3 years ago

Cc: Simon Charette added
Resolution: invalid
Status: newclosed

This behavior was changed in 9c5c9bd7091b60fdccc405890dc4f44a8010e954 however I believe that the current one is correct, because we longer cast Decimals to strings. If you need to round differently I recommend to do so before passing values to the field.

comment:3 by Michał Szczepańczyk, 3 years ago

That's for the response. In such case I think it would be good to mention the change in release docs.

Another consequence of this change is that we have different results between sqlite and postgres. When using sqlite the following query is generated:

INSERT INTO "myapp_foo" ("amount") VALUES ('1.12')

in reply to:  3 comment:4 by Mariusz Felisiak, 3 years ago

Replying to Michał Szczepańczyk:

That's for the response. In such case I think it would be good to mention the change in release docs.

I will try to figure out how to mention it in docs.

Another consequence of this change is that we have different results between sqlite and postgres. When using sqlite the following query is generated:

INSERT INTO "myapp_foo" ("amount") VALUES ('1.12')

Yes but it returns 1.13 as in MySQL and Oracle.

comment:5 by Simon Charette, 3 years ago

SQLite is also not a stellar example of ambiguous data type handling as detailed in their documentation. Since it doesn't have a proper type to store decimal data the rounding happens on the Django side.

comment:6 by Alex, 3 years ago

The problem is that right now if I just want to round down the value I have to get the precision of the field from the model internals, which is a bit ugly I think. And also this has to be done everywhere where this field is updated. Why can't we keep everything that django.db.backends.utils.format_number does, but the last line that converts the value to string? There's a lot of flexibility that Python Decimal provides that's not available in SQL.

The code I had before:

            with localcontext() as ctx:
                ctx.rounding = ROUND_DOWN
                self.items.update(
                    split=Decimal("100") * split / self.number_of_items
                )

The code I have to have now:

            with localcontext() as ctx:
                value = Decimal("100") * split / number_of_items
                ctx.rounding = ROUND_DOWN
                max_digits = Model.split.field.max_digits
                decimal_places = Model.split.field.decimal_places
                if max_digits is not None:
                    ctx.prec = max_digits
                if decimal_places is not None:
                    value = value.quantize(Decimal(1).scaleb(-decimal_places))
                self.items.update(
                    split=value
                )
Note: See TracTickets for help on using tickets.
Back to Top