[Django] #23801: IntegerField w/ max_length silently truncates integers
Django
----------------------------------------------+--------------------
Reporter: eykd | Owner: nobody
Type: Bug | Status: new
Component: Database layer (models, ORM) | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------------------+--------------------
An IntegerField with a max_length silently truncates integers. Observe:
{{{
>>> class MyModel(Model):
>>> num = IntegerField(max_length=2)
>>> m = MyModel.objects.create(num=42)
>>> m.num
42
>>> m.num = 427
>>> m.num
427
>>> m.save()
>>> m.num
42
}}}
This violates the Zen of Python: "Errors should never pass silently." It
has also has been a reliable source of fun bugs over the years. This is
long-standing Procrustean behavior, since at least v1.0 I think, if not
before, but it bit me for the nth time yesterday, so I'm filing a bug.
--
Ticket URL: <https://code.djangoproject.com/ticket/23801>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: eykd | Owner: nobody
Type: Bug | Status: new
(models, ORM) | Resolution:
Severity: Normal | Triage Stage:
Keywords: | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Comment:
I am not sure how to proceed given in #22637 "the opinion was that the
`max_length` option should not be forbidden in order to keep Django upward
compatible, but that it also does not make sense for the `IntegerField`. I
suppose we could add a warning using the checks framework (which could be
silenced by those intentionally using `max_length` on `IntegerField` for
some reason). Do you have any suggestions?
--
Ticket URL: <https://code.djangoproject.com/ticket/23801#comment:1>
Django
Reporter: eykd | Owner: nobody
Type: Bug | Status: new
(models, ORM) | Resolution:
Severity: Normal | Triage Stage:
Keywords: | Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by eykd):
Even a warning would be splendid. Silence is the killer here.
--
Ticket URL: <https://code.djangoproject.com/ticket/23801#comment:2>
Django
-------------------------------------+-------------------------------------
Reporter: eykd | Owner: nobody
Type: New feature | Status: new
Component: Database layer | Version: master
(models, ORM) | Resolution:
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by timgraham):
* stage: Unreviewed => Accepted
* type: Bug => New feature
--
Ticket URL: <https://code.djangoproject.com/ticket/23801#comment:3>
Django
-------------------------------------+-------------------------------------
Type: New feature | Status: new
Component: Database layer | Version: master
(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 MattBlack85):
What backend are you using? With SQLite and postgresql we cannot observe
this behaviour (truncating numbers)
--
Ticket URL: <https://code.djangoproject.com/ticket/23801#comment:4>
Django
-------------------------------------+-------------------------------------
Type: New feature | Status: new
Component: Database layer | Version: master
(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 eykd):
I'm using `django.db.backends.postgresql_psycopg2`, and I've seen the
behavior with Postgres 8.4 and 9.0. I should mention that we're on Django
1.4--I haven't tested this with a newer Django yet, but as I said, I've
been aware of the behavior since 1.0 or so. My code session above is
lightly edited from an actual session (obviously the model def needs to be
in a file and there's a syncdb or migration implicit as well).
I'll try reproducing the behavior in Django 1.7 later, just to be sure.
--
Ticket URL: <https://code.djangoproject.com/ticket/23801#comment:5>
Django
-------------------------------------+-------------------------------------
Type: New feature | Status: new
Component: Database layer | Version: master
(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 eykd):
Well bless me, now I can't reproduce this either, even on Django 1.1 and
Postgres 8.4. Perhaps I've been taking crazy pills all this time?
`max_length` appears to have no effect whatsoever. Please feel free to
invalidate this ticket, and I apologize for wasting your time!
--
Ticket URL: <https://code.djangoproject.com/ticket/23801#comment:6>
Django
-------------------------------------+-------------------------------------
Type: New feature | Status: new
Component: Database layer | Version: master
(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 burhan):
Can't reproduce this against sqlite or postgresql (9.3) with django 1.7.1
final
--
Ticket URL: <https://code.djangoproject.com/ticket/23801#comment:7>
Django
-------------------------------------+-------------------------------------
Type: New feature | Status: new
Component: Database layer | Version: master
(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 MattBlack85):
Adding a warning could be still maybe ok to just let people know about
that. Thoughts?
--
Ticket URL: <https://code.djangoproject.com/ticket/23801#comment:8>
Django
-------------------------------------+-------------------------------------
Reporter: eykd | Owner:
Component: Database layer | Status: assigned
(models, ORM) | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by MattBlack85):
* status: new => assigned
* owner: nobody => MattBlack85
--
Ticket URL: <https://code.djangoproject.com/ticket/23801#comment:9>
Django
-------------------------------------+-------------------------------------
Reporter: eykd | Owner:
Type: New feature | MattBlack85
Component: Database layer | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by MattBlack85):
I wrote a small patch for this, you can find it here.
https://github.com/MattBlack85/django/compare/gh-mb-fix_23801?expand=1
Is it ok for the intent?
--
Ticket URL: <https://code.djangoproject.com/ticket/23801#comment:10>
Django
-------------------------------------+-------------------------------------
Reporter: eykd | Owner:
Type: New feature | MattBlack85
Component: Database layer | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by collinanderson):
* has_patch: 0 => 1
Comment:
Hi, Do you want to hit the "Create pull request" button?
--
Ticket URL: <https://code.djangoproject.com/ticket/23801#comment:11>
Django
-------------------------------------+-------------------------------------
Reporter: eykd | Owner:
Type: New feature | MattBlack85
Component: Database layer | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
Has patch: 1 | checkin
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by timgraham):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/23801#comment:12>
Django
-------------------------------------+-------------------------------------
Reporter: eykd | Owner:
Type: New feature | MattBlack85
(models, ORM) | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
Has patch: 1 | checkin
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"e9d1f1182aaccaa8b60cf6a3491f0103d2bb0229"]:
{{{
#!CommitTicketReference repository=""
revision="e9d1f1182aaccaa8b60cf6a3491f0103d2bb0229"
Fixed #23801 -- Added warning when max_length is used with IntegerField
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23801#comment:13>
Django
-------------------------------------+-------------------------------------
| Procopio
Type: New feature | Status: closed
Component: Database layer | Version: master
(models, ORM) |
Keywords: | Triage Stage: Ready for
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"c512912463c11428e572fb409704b351b1b26dfd" c5129124]:
{{{
#!CommitTicketReference repository=""
revision="c512912463c11428e572fb409704b351b1b26dfd"
Refs #23801 -- Made integer field max_length warning show correct field
type.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23801#comment:14>