Opened 9 years ago

Closed 9 years ago

Last modified 5 years ago

#23801 closed New feature (fixed)

Warn when max_length is used with IntegerField

Reported by: David Eyk Owned by: Mattia Procopio
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

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.

Change History (14)

comment:1 by Tim Graham, 9 years ago

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?

comment:2 by David Eyk, 9 years ago

Even a warning would be splendid. Silence is the killer here.

comment:3 by Tim Graham, 9 years ago

Summary: IntegerField w/ max_length silently truncates integersWarn when max_length is used with IntegerField
Triage Stage: UnreviewedAccepted
Type: BugNew feature

comment:4 by Mattia Procopio, 9 years ago

What backend are you using? With SQLite and postgresql we cannot observe this behaviour (truncating numbers)

comment:5 by David Eyk, 9 years ago

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.

comment:6 by David Eyk, 9 years ago

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!

comment:7 by Burhan Khalid, 9 years ago

Can't reproduce this against sqlite or postgresql (9.3) with django 1.7.1 final

comment:8 by Mattia Procopio, 9 years ago

Adding a warning could be still maybe ok to just let people know about that. Thoughts?

comment:9 by Mattia Procopio, 9 years ago

Owner: changed from nobody to Mattia Procopio
Status: newassigned

comment:10 by Mattia Procopio, 9 years ago

I wrote a small patch for this, you can find it here. https://github.com/django/django/pull/3620
Is it ok for the intent?

Last edited 9 years ago by Mattia Procopio (previous) (diff)

comment:11 by Collin Anderson, 9 years ago

Has patch: set

Hi, Do you want to hit the "Create pull request" button?

comment:12 by Tim Graham, 9 years ago

Triage Stage: AcceptedReady for checkin

comment:13 by Tim Graham <timograham@…>, 9 years ago

Resolution: fixed
Status: assignedclosed

In e9d1f1182aaccaa8b60cf6a3491f0103d2bb0229:

Fixed #23801 -- Added warning when max_length is used with IntegerField

comment:14 by Tim Graham <timograham@…>, 5 years ago

In c5129124:

Refs #23801 -- Made integer field max_length warning show correct field type.

Note: See TracTickets for help on using tickets.
Back to Top