Validate that IntegerField is a valid Signed value for databases that don't do it themselves.

[Cal Leeming]

Hey,

So I found that, if you attempt to store a number larger than the SIGNED INT maximum, for some reason, Django (or the db) auto converts it to the maximum allowed, rather than throwing an error. This really causes some strange issues, especially when your app logic *thinks* you've managed to save the correct value lol.

Suggested patch might be, enforcing a default maximum, which throws a ValueError whenever you try saving it into the database??

If this would be an appropriate way forward, I'd be more than happy to submit a patch.

Lmk

Cal

PS) To test for yourself, try using the following:

class TestModel(models.Model):
    test_field = models.IntegerField(db_index = True)

TestModel(
    test_field = 3517601254
)

TestModel.objects.get(test_field = 3517601254)

[Aymeric Augustin]

Since this could be database dependent, could you mention which database you are using (software, version, 32/64bits)? Thanks!

[Cal Leeming]

Sorry, I should have thought to include that!

MySQL Server Version: MySQL 5.1.50-log
MySQL Protocol: 10
Django	1.2.4
Engine: InnoDB
Server: 64bit

Also, on a side note, I tried putting a larger value into the database, and it failed with an error, rather than reducing the number to the max allowed.

[Alex Gaynor]

I'm almost positive this is MySQL being MySQL. If you can reproduce with another database please feel free to reopen.

[Cal Leeming]

mysql> INSERT INTO ddcms_sessionip (`ip`) VALUES(35176012444
);
Query OK, 1 row affected
mysql>

Lol, damn it, you're right. Man, that's absolutely retarded, why on earth would they allow that :S

Do you not think we should protect against this by adding a default min/max for unsigned/signed integer fields anyway??

Cal

[Cal Leeming]

Changed to reopened, to find out whether we should be protecting against this anyway??

[Alex Gaynor]

Fair point, marking as DDN because I can't decide how I feel. (Edited the title of this issue to reflect the new status).

[Aymeric Augustin]

Generally Django doesn't try to prevent people from shooting themselves into the foot with MySQL.

We should check what Django does when someone tries to insert a 20 char string in a 10 char field. This is the most common data-loss bug of MySQL. The int truncation should be handled in the same way.

[Cal Leeming]

Usually, when the data you are inserting, isn't supported on the database (string truncation), it just issues a Warning exception.

Personally, I think when this event is encountered, a warning should be raised (like the char fields), to say the number has been truncated to the highest available.

[Karen Tracey]

I'm a little confused by your MySQL shell snippet not showing at least a warning for a case where data has to be adjusted. When I try what you show, I get a warning, though it does not prevent a row from being saved to the DB:

mysql> insert into testme (`val`) values (35176012444);
Query OK, 1 row affected, 1 warning (0.00 sec)

mysql> show warnings;
+---------+------+-------------------------------------------------------+
| Level   | Code | Message                                               |
+---------+------+-------------------------------------------------------+
| Warning | 1264 | Out of range value adjusted for column 'val' at row 1 |
+---------+------+-------------------------------------------------------+
1 row in set (0.00 sec)

mysql> select * from testme;
+------------+
| val        |
+------------+
| 2147483647 |
+------------+
1 row in set (0.00 sec)

Note though, that this behavior is dependent on the SQL mode in effect. MySQL can be set to reject out-of-range values:

mysql> SELECT @@SESSION.sql_mode;
+--------------------+
| @@SESSION.sql_mode |
+--------------------+
|                    |
+--------------------+
1 row in set (0.00 sec)

mysql> set session sql_mode='traditional';
Query OK, 0 rows affected (0.00 sec)

mysql> SELECT @@SESSION.sql_mode;
+-------------------------------------------------------------------------------------------------------------------------------+
| @@SESSION.sql_mode                                                                                                            |
+-------------------------------------------------------------------------------------------------------------------------------+
| STRICT_TRANS_TABLES,STRICT_ALL_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,TRADITIONAL,NO_AUTO_CREATE_USER |
+-------------------------------------------------------------------------------------------------------------------------------+
1 row in set (0.00 sec)

mysql> delete from testme;
Query OK, 1 rows affected (0.00 sec)

mysql> insert into testme (`val`) values (35176012444);
ERROR 1264 (22003): Out of range value adjusted for column 'val' at row 1
mysql> select * from testme;
Empty set (0.00 sec)

Marking this wontfix -- if you want errors for this kind of situation then I think the right way to do that is to set the DB to report the error. (And you can set sql mode to be strict via server config, you do not need to do it per-session as shown above.)

[Cal Leeming]

If this is the case, then we need to make it easier for users to know how to set the DB to report the error. Also, we should make it default to reporting these problems, via settings.py. But, by setting SQL mode to 'traditional', what other can of worms do we potentially open? If MySQL wanted to tell us these things, why doesn't it do it by default?? I really don't think it is appropriate to just mark this as 'wontfix', without at least defaulting to the correct behaviour.

[Aymeric Augustin]

Please do not reopen tickets marked as wontfix by core developers. See the guidelines here: ​http://docs.djangoproject.com/en/dev/internals/contributing/#reporting-bugs (7^th bullet point)

More generally, MySQL is broken in many ways, and documenting them belongs to MySQL's docs, not Django's. By the way, it's fairly clear in Django's docs that PostgreSQL is the recommended database engine.

[Cal Leeming]

Sorry, I didn't realise I had to follow up 'wontfix' on django-developers. I will send a mail to that now.

[Cal Leeming]

Further discussion has taken place in these two threads:

​http://groups.google.com/group/django-developers/browse_thread/thread/f0b8ddbda03a2d8e
​http://groups.google.com/group/django-developers/browse_thread/thread/22dd636f421823fd/a058fd6c31ee09e3

I've submitted two new tickets for this:

[Django] #15939: SignedIntegerField / UnsignedIntegerField as part of the core fields.py

[Django] #15940: Patch database documentation to explain why using sql_mode=strict is important

[Petr Marhoun <petr.marhoun@…>]

I am not sure if I should reopen ticket closed by a core developer, especially after direct note. But I think that the resolution was based on invalid imformation - that only MySQL is broken, SQLite and recommended PostgreSQL are OK. But I think it is not true. Documentation of PostgreSQL say: "integer ---- 4 bytes ---- typical choice for integer ---- -2147483648 to +2147483647":

​http://www.postgresql.org/docs/9.0/interactive/datatype-numeric.html

Moreover, BigIntegerField in Django checks interval [2 ⁶³, 2 ⁶³ - 1] so for consistency IntegerField could check interval [2 ³¹, 2 ³¹ -1].

I prepare patch with two tests - one (for BigIntegerField) passes on all tested databases (PostgreSQL 8.4.8, MySQL 5.1.54 with InnoDB, SQlite from Python 2.7.1 - everything on Ubuntu 11.04 64bit). The almost same second one (for IntegerField) failes on PostgreSQL with "DatabaseError: integer out of range" and on MySQL with "AssertionError" (saved number is truncated). (But I do not know if these two databases are so important that they should define interval common for all databases.)

[Petr Marhoun <petr.marhoun@…>]

Sorry, I can't read - now I see that this ticket is about hiding of an database error, not about this database error. So there is another ticket: #16747 - this one is closed again.