Opened 13 months ago

Closed 13 months ago

Last modified 13 months ago

#22272 closed Bug (fixed)

model.DecimalField with decimal_places=0

Reported by: merb Owned by: nobody
Component: Database layer (models, ORM) Version: 1.7-alpha-2
Severity: Release blocker Keywords:
Cc: merb Triage Stage: Ready for checkin
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Currently I can't create DecimalField with decimal_places=0 on Postgresql with psycopg2 (sqlite3 works as expected)

In PostgreSQL I can create numeric fields with a scale of zero: http://www.postgresql.org/docs/9.3/static/datatype-numeric.html

But the current implementation disallows this: (The Exception I hit)

Running migrations:
  Applying envisia_article.0001_initial...DEBUG:django.db.backends:(0.002) 
            SELECT c.relname
            FROM pg_catalog.pg_class c
            LEFT JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace
            WHERE c.relkind IN ('r', 'v', '')
                AND n.nspname NOT IN ('pg_catalog', 'pg_toast')
                AND pg_catalog.pg_table_is_visible(c.oid); args=None
DEBUG:django.db.backends.schema:CREATE TABLE "envisia_article_solderresist" ("id" serial NOT NULL PRIMARY KEY, "name" varchar(100) NULL, "name_en" varchar(100) NULL); (params [])
DEBUG:django.db.backends:(0.003) CREATE TABLE "envisia_article_solderresist" ("id" serial NOT NULL PRIMARY KEY, "name" varchar(100) NULL, "name_en" varchar(100) NULL); args=[]
DEBUG:django.db.backends.schema:CREATE TABLE "envisia_article_bestpressurecolor" ("id" serial NOT NULL PRIMARY KEY, "name" varchar(100) NULL, "name_en" varchar(100) NULL); (params [])
DEBUG:django.db.backends:(0.001) CREATE TABLE "envisia_article_bestpressurecolor" ("id" serial NOT NULL PRIMARY KEY, "name" varchar(100) NULL, "name_en" varchar(100) NULL); args=[]
DEBUG:django.db.backends.schema:CREATE TABLE "envisia_article_cuinside" ("id" serial NOT NULL PRIMARY KEY, "value" numeric(10, None) NOT NULL); (params [])
DEBUG:django.db.backends:(0.001) CREATE TABLE "envisia_article_cuinside" ("id" serial NOT NULL PRIMARY KEY, "value" numeric(10, None) NOT NULL); args=[]
Traceback (most recent call last):
  File "/Users/schmitch/Programme/virtualenv/envisia_dashboard/lib/python3.3/site-packages/Django-1.7a2-py3.3.egg/django/db/backends/utils.py", line 61, in execute
    return self.cursor.execute(sql, params)
psycopg2.DataError: invalid input syntax for integer: "none"
LINE 1: ...nside" ("id" serial NOT NULL PRIMARY KEY, "value" numeric(10...
                                                             ^


The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "./manage.py", line 10, in <module>
    execute_from_command_line(sys.argv)
  File "/Users/schmitch/Programme/virtualenv/envisia_dashboard/lib/python3.3/site-packages/Django-1.7a2-py3.3.egg/django/core/management/__init__.py", line 427, in execute_from_command_line
    utility.execute()
  File "/Users/schmitch/Programme/virtualenv/envisia_dashboard/lib/python3.3/site-packages/Django-1.7a2-py3.3.egg/django/core/management/__init__.py", line 419, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/Users/schmitch/Programme/virtualenv/envisia_dashboard/lib/python3.3/site-packages/Django-1.7a2-py3.3.egg/django/core/management/base.py", line 287, in run_from_argv
    self.execute(*args, **options.__dict__)
  File "/Users/schmitch/Programme/virtualenv/envisia_dashboard/lib/python3.3/site-packages/Django-1.7a2-py3.3.egg/django/core/management/base.py", line 336, in execute
    output = self.handle(*args, **options)
  File "/Users/schmitch/Programme/virtualenv/envisia_dashboard/lib/python3.3/site-packages/Django-1.7a2-py3.3.egg/django/core/management/commands/migrate.py", line 145, in handle
    executor.migrate(targets, plan, fake=options.get("fake", False))
  File "/Users/schmitch/Programme/virtualenv/envisia_dashboard/lib/python3.3/site-packages/Django-1.7a2-py3.3.egg/django/db/migrations/executor.py", line 60, in migrate
    self.apply_migration(migration, fake=fake)
  File "/Users/schmitch/Programme/virtualenv/envisia_dashboard/lib/python3.3/site-packages/Django-1.7a2-py3.3.egg/django/db/migrations/executor.py", line 94, in apply_migration
    migration.apply(project_state, schema_editor)
  File "/Users/schmitch/Programme/virtualenv/envisia_dashboard/lib/python3.3/site-packages/Django-1.7a2-py3.3.egg/django/db/migrations/migration.py", line 97, in apply
    operation.database_forwards(self.app_label, schema_editor, project_state, new_state)
  File "/Users/schmitch/Programme/virtualenv/envisia_dashboard/lib/python3.3/site-packages/Django-1.7a2-py3.3.egg/django/db/migrations/operations/models.py", line 28, in database_forwards
    schema_editor.create_model(model)
  File "/Users/schmitch/Programme/virtualenv/envisia_dashboard/lib/python3.3/site-packages/Django-1.7a2-py3.3.egg/django/db/backends/schema.py", line 243, in create_model
    self.execute(sql, params)
  File "/Users/schmitch/Programme/virtualenv/envisia_dashboard/lib/python3.3/site-packages/Django-1.7a2-py3.3.egg/django/db/backends/schema.py", line 95, in execute
    cursor.execute(sql, params)
  File "/Users/schmitch/Programme/virtualenv/envisia_dashboard/lib/python3.3/site-packages/Django-1.7a2-py3.3.egg/django/db/backends/utils.py", line 77, in execute
    return super(CursorDebugWrapper, self).execute(sql, params)
  File "/Users/schmitch/Programme/virtualenv/envisia_dashboard/lib/python3.3/site-packages/Django-1.7a2-py3.3.egg/django/db/backends/utils.py", line 61, in execute
    return self.cursor.execute(sql, params)
  File "/Users/schmitch/Programme/virtualenv/envisia_dashboard/lib/python3.3/site-packages/Django-1.7a2-py3.3.egg/django/db/utils.py", line 93, in __exit__
    six.reraise(dj_exc_type, dj_exc_value, traceback)
  File "/Users/schmitch/Programme/virtualenv/envisia_dashboard/lib/python3.3/site-packages/Django-1.7a2-py3.3.egg/django/utils/six.py", line 535, in reraise
    raise value.with_traceback(tb)
  File "/Users/schmitch/Programme/virtualenv/envisia_dashboard/lib/python3.3/site-packages/Django-1.7a2-py3.3.egg/django/db/backends/utils.py", line 61, in execute
    return self.cursor.execute(sql, params)
django.db.utils.DataError: invalid input syntax for integer: "none"
LINE 1: ...nside" ("id" serial NOT NULL PRIMARY KEY, "value" numeric(10...
                                                             ^

As you can see, since I've runnning with debug is that numeric() is numeric(10, None) instead of numeric(10) or numeric(10, 0).
Which is a bug and shouldn't occur.

Change History (11)

comment:1 Changed 13 months ago by merb

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Django 1.6 will work and make the model to numeric(10, 0) even South worked like this and made it from numeric(10, X) to numeric(10, 0) if i changed the value.

comment:2 Changed 13 months ago by merb

  • Cc merb added

comment:3 Changed 13 months ago by bmispelon

  • Triage Stage changed from Unreviewed to Accepted

I'm able to reproduce the issue using this simple model:

class Foo(models.Model):
    foo = models.DecimalField(max_digits=7, decimal_places=0)

As noted, this works in 1.6 so the release blocker flag is warranted too.

comment:4 Changed 13 months ago by bmispelon

  • Has patch set
  • Needs tests set

The following patch seems to fix the issue (you have to regenerate the migration file):

  • django/db/models/fields/__init__.py

    diff --git a/django/db/models/fields/__init__.py b/django/db/models/fields/__init__.py
    index 16a0700..dd88471 100644
    a b class DecimalField(Field): 
    13791379
    13801380    def deconstruct(self):
    13811381        name, path, args, kwargs = super(DecimalField, self).deconstruct()
    1382         if self.max_digits:
     1382        if self.max_digits is not None:
    13831383            kwargs['max_digits'] = self.max_digits
    1384         if self.decimal_places:
     1384        if self.decimal_places is not None:
    13851385            kwargs['decimal_places'] = self.decimal_places
    13861386        return name, path, args, kwargs

comment:5 Changed 13 months ago by merb

  • Patch needs improvement set

Currently this patch needs improvement since:

max_digits needs to be positive in the PostgreSQL documentation (I think in other databases this max_digits needs to be positive as well)

http://www.postgresql.org/docs/9.3/static/datatype-numeric.html

Also MySQL needs a positive value for max_digits aswell. Only decimal_places could be zero or positive.

http://www.postgresql.org/docs/9.3/static/datatype-numeric.html

comment:6 Changed 13 months ago by bmispelon

This patch only fixes the regression, but unless I'm mistaken, earlier versions of Django don't validate the value of max_digits, do they?

This should be a separate ticket imho.

comment:7 Changed 13 months ago by bmispelon

  • Needs tests unset
  • Patch needs improvement unset

Actually, there are already checks in place to make sure max_digits>0 and decimal_places>=0: https://github.com/django/django/blob/master/django/db/models/fields/__init__.py#L1342

So I'm not sure I understand what comment:5 is referring to.

I've got a patch with tests now ready for committing: https://github.com/bmispelon/django/commit/37f7f233f5f30c28ea60a7fbc272ffd296e2dbe1
Any objection to me pushing it?

comment:8 Changed 13 months ago by charettes

  • Triage Stage changed from Accepted to Ready for checkin

LGTM, fix and test are pretty straightforward.

comment:9 Changed 13 months ago by Baptiste Mispelon <bmispelon@…>

  • Resolution set to fixed
  • Status changed from new to closed

In 37f7f233f5f30c28ea60a7fbc272ffd296e2dbe1:

Fixed #22272 -- Fixed regression in DecimalField when using decimal_places=0.

Thanks to trac user merb for the report.

comment:10 Changed 13 months ago by merb

  • Needs tests set
  • Patch needs improvement set
  • Triage Stage changed from Ready for checkin to Accepted

Oh thanks, i didn't saw the checks. Currently I was really busy.
Btw. thanks for the quick fix. ;)

Err wtf. what did I do.. Trac is really aweful.

Last edited 13 months ago by merb (previous) (diff)

comment:11 Changed 13 months ago by merb

  • Has patch unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Accepted to Ready for checkin
Note: See TracTickets for help on using tickets.
Back to Top