Primary key columns should be UNSIGNED

[Manuzhai <mail@…>]

This gets you twice the integer space, and seems to me to be a nice sanity check.

Don't know if the same would go for Postgres.

Not enough into the code right know to produce a patch myself.

[Adrian Holovaty]

Fixed in [171]. Thanks!

[Julian Bez]

This improvement got lost somehow. I spent half an hour looking in the svn logs to see if there was a reason why it's not still an unsigned integer, but I found none.

[Karen Tracey <kmtracey@…>]

It 'got lost' intentionally with the fix for #1500. I do not quite follow the statement there: "Unsigned int would be better, but because of the way AutoField? gets resolved to a signed integer (see #1165) and the fact that a PostgreSQL serial is signed, it cannot be without much more work." so do not know if it still applies as a reason why MySQL AutoFields cannot be unsigned. However it appears to have been an intentional change at one point so someone who does understand it should verify that switching back to unsigned won't cause any problems before flip-flopping the value again.

[Julian Bez]

What #1500 says is, that if a foreign key points to an AutoField, we would need to resolve it to the same unsigned integer in MySQL. Currently foreign keys become IntegerFields.

If we want to switch keys to unsigned for MySQL, it seems some kind of ForeignKeyField would be needed, which could then be, like AutoField, unsigned.

[Julian Bez]

This issue let me to a bug: #8316

With the patch there, we would have a KeyField and could change AutoField and KeyField to unsigned for MySQL.

[Malcolm Tredinnick]

This is an really an enhancement request. If somebody really needs it right now they can alter the table column (plus it won't affect existing setups anyway). It can wait until after 1.0 for reconsideration.

[philliptemple@…]

Change line 10 in django/db/backends/mysql/creation.py from:

'AutoField':         'integer AUTO_INCREMENT',

To:

'AutoField':         'integer UNSIGNED AUTO_INCREMENT',

In lines 709-713 of django/db/models/fields/related.py it casts PositiveIntegerField and SmallPositiveIntegerField, as well as AutoField, to IntegerField().db_type(). This makes me assume we do not need to make any changes by changing AutoField to UNSIGNED matching PositiveIntegerField which is already 'integer UNSIGNED'.

Phillip.

[(none)]

Milestone post-1.0 deleted

[Thomas Steinacher <tom@…>]

Any reason why Django still uses signed ints for auto-incremented primary keys?

CREATE TABLE `app_model` (
    `id` integer AUTO_INCREMENT NOT NULL PRIMARY KEY,

This seems silly to me.

[None]

Are people still interested in this change? Seems useful and wouldn't break anything from what i can tell? If so i can make a patch.

[Thomas Steinacher <tom@…>]

Yes, this is important, as using a signed integer is just wrong. There are never IDs below zero and you lose one bit of information.

[Piotr Czachur]

Yeah, we want it. It's also important because database won't allow to insert invalid data (negative IDs) - which is always good.
In fact MySQL by default will allow to insert such value (it will modify it before doing insert) and it will generate warning, but in STRICT/TRADITIONAL mode it will reject such value.

[Adam Nelson]

Resetting triage stage so hopefully a patch goes into the next major release or this ticket gets closed.

[buzzar]

Adding UNSIGNED the AutoField definition for MySQL

[ssspiochld]

Test project (tests/modeltests/custom_pk could be used but it would require to change the employee_code field definition to AutoField)

[ssspiochld]

As it was claimed by @adamnelson as Design decision needed I wont change the status as accepted/ready for checkin or should I?

[Adam Nelson]

Maybe send the ticket to the django-developers group ... It needs a commiter to approve and this ticket is so old it might be hidden.

[ssspiochld]

The patch from ​http://code.djangoproject.com/ticket/1500 extended the byte size of field
used for AutoField (mediumint->int) however by omitting UNSIGNED field from definition the id-space
is not used in the most efficient way. More about the MySQL field definitions and id-space can be found at
​http://dev.mysql.com/doc/refman/5.0/en/numeric-types.html

By applying the patch two profits are met (thats what tests in test_proj.zip check):

• negative values (invalid) for AutoField wont be permitted
• the usable id-space will become exceeded two times (from 231 to 232)

[Russell Keith-Magee]

Accepted for 2.0. We can't do this in a particularly friendly backwards compatible way, because it would require an ALTER TABLE for all existing foreign keys.

[Thomas Steinacher <tom@…>]

@russellm: What makes this backwards-incompatible? People who don't ALTER TABLE should not notice the change.

[Russell Keith-Magee]

The concern isn't about the table itself; it's about the foreign keys referencing the table. A new table pointing to an old table would have a foreign key that would allow invalid values; if you recreate a base table that has foreign keys pointing at it, you will be able to create instances that you can't refer to.

However, talking this over with Malcolm some more, we think we've got a way to do this. Lets add an 'unsigned=False' option to AutoField. By default, it needs to be False for backwards compatibility; when 2.0 comes around, we can change this to be unsigned by default.

The change needs to be made for other backends (e.g., serial under Postgres needs to be changed to bigserial if unsigned=True), but this provides a way to access unsigned representations in a completely opt-in way for people who want them, and provides a simple way to migrate forward.

[zeraien]

+1

[anonymous]

Replying to russellm:

However, talking this over with Malcolm some more, we think we've got a way to do this. Lets add an 'unsigned=False' option to AutoField. By default, it needs to be False for backwards compatibility; when 2.0 comes around, we can change this to be unsigned by default.

This makes a lot of sence else there will be issues with developers creating new models and finding all sorts of referencing issues. In the next major release this can then be changed to unsigned by default and will remove the need for the developer to set the flag by default.

[pzinovkin]

Submitted patch with unsigned option for AutoField.
Patch includes rel_db_type_cleanup.diff from #13774

[pzinovkin]

Made pull request ​https://github.com/django/django/pull/49

[pzinovkin]

Alternative approach to fix #56

[Anssi Kääriäinen]

To me the alternate approach seems fine. It does need some tests though, mainly to make sure the related field type matching works properly.

The alternate approach doesn't directly resolve this ticket's issue though. Unsigned is different than bigint. I wonder if adding also unsigned flag should be done?

[pzinovkin]

​https://github.com/django/django/pull/308

There is no need to use unsigned field to increase id-space. It will only introduce inconsistency between database engines.

[pzinovkin]

with tests

[tim@…]

I disagree with pzinovkin. Using unsigned integers basically doubles the amount of id's that can be used. That has benefits no matter which integer type is used. But in the case of an INT, 2 billion id's versus 4 billion for the same cost is substantial. Different RDBMS' are not built the same so trying to treat them all the same is a potential recipe for poor scalability.

[Chris Wilson]

Created a ​pull request with pzinovkin's patch and tests.

[kbcmdba@…]

MySQL 5.5.29 and up (plus other versions in other releases of MySQL) now require AUTO_INCREMENT columns to be UNSIGNED. The return value of LAST_INSERT_ID() had to change to BIGINT UNSIGNED because users who had BIGINT UNSIGNED column types were unable to get the ID value when the ID was greater than 9,223,372,036,554,775,807 (BIGINT MAX / 2 - 1 or about 9 quintillion) but still less than the highest possible BIGINT VALUE (18,446,744,023,709,551,615 or about 18 quintillion).

It no longer makes sense to allow negative AUTO_INCREMENT values by default in MySQL persistence layers. I strongly recommend changing the default data type on auto_increment columns to explicitly become UNSIGNED, especially when the persistence layer is known to be MySQL.

Here's the specific MySQL 5.5.29 release note:

• Incompatible Change: LAST_INSERT_ID(expr) did not work for expr values greater than the largest signed BIGINT value. Such arguments now are accepted, with some consequences for compatibility with previous versions:

LAST_INSERT_ID() now returns a BIGINT UNSIGNED value, not a BIGINT (signed) value.
LAST_INSERT_ID(expr) now returns an unsigned integer value, not a signed integer value.
For AUTO_INCREMENT columns, negative values are no longer supported.

(Bug #20964, Bug #11745891)

[Tim Graham]

The DatabaseCreation classes are now deprecated, so the patch will need to be refreshed to use SchemaEditor instead.

[Markus Amalthea Magnuson]

I've rewritten pzinovkin's patch based on the ​previous pull request from gcc:

​https://github.com/alimony/django/compare/master...alimony:unsigned-primary-keys

Did not open a pull request yet as it's not ready, most importantly the tests are failing:

As far as I can see, the new test_can_use_large_primary_keys test can never have passed on postgres. There is no concept of unsigned in postgres, meaning a primary key bigger than 2147483648 only fits in a bigint/bigserial. The latter is used for the AutoField itself in this patch, but as someone pointed out, having a foreign key reference such large primary key fails.

The updated AutoField in the patch ​determines the field type of a foreign key that references itself like this:

def rel_db_type(self, connection):
    db_type = 'PositiveIntegerField' if self.unsigned else 'IntegerField'
    return self._internal_to_db_type(db_type, connection)

But both PositiveIntegerField and IntegerField resolve to integer in the case of postgres.

Some ways it can ever do anything else, that I can think of now, are:

1. Set related_fields_match_type to True for postgres.
2. Make PositiveIntegerField mean a bigint for postgres.
3. Make the rel_db_type method of the new AutoField do some special treatment to return e.g. BigIntegerField for postgres if self.unsigned is true.

(The first two are of course out of the question.)

I want to write proper tests for this patch to make sure everything behaves on every database, but I guess there needs to be some sort of consensus first on how to solve the above. And people with far more knowledge on Django than me will have to reach that consensus :)

[Markus Amalthea Magnuson]

I'm not actively working on this, feel free to pick up where I left off, anyone.

[Collin Anderson]

​https://github.com/django/django/pull/8183

I just ran into a situation where I have a legacy mysql table that has a bigint(20) unsigned auto_increment primary key that I want to point a new ForeignKey to. Django is creating the foreign key as bigint(20) rather than bigint(20) unsigned, and mysql won't allow a foreign key constraint between the two.

So basically, I need the legacy table primary key to be a PositiveBigAutoField, and the ForeignKey needs to be a PositiveBigIntegerField.

I'm not super attached to adding this, but I think it could solve this ancient ticket.

[Collin Anderson]

Patch should address the latest feedback.

[Caio Ariede]

I'll try to revive this ticket

[Caio Ariede]

PR ​https://github.com/django/django/pull/11900

[Mariusz Felisiak]

I don't think it's necessary anymore to change the default behavior. We have PositiveBigIntegerField in Django 3.1 (#30987) and a way to change the default AutoField in Django 3.2+ (#31007). As a workaround folks can use DEFAULT_AUTO_FIELD = 'django.db.models.BigAutoField' or an explicit primary key id = models.PositiveBigIntegerField(primary_key=True), see ​the mailing list discussion.