Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Ability to make id column unsigned. #49

Closed
wants to merge 1 commit into from

3 participants

Pavel Zinovkin Anssi Kääriäinen Daniel Swarbrick
Anssi Kääriäinen
Collaborator

The field type changes for PostgreSQL doesn't seem valid - it should be something like integer check (val > 0). Same for Oracle (add check) and for SQLite if there is support for checks (likely yes).

Otherwise quick cursory glance of the patch doesn't raise any blocker issues.

Pavel Zinovkin

I am familiar only with MySQL, but I did little research.

PostgreSQL: serial and bigserial are equivalent, except last creates bigint column. Also bigserial suggested by russelm in ticket comments.
http://www.postgresql.org/docs/9.1/static/datatype-numeric.html#DATATYPE-SERIAL

SQLite: "Every row of every table has an 64-bit signed integer ROWID. ... If a table contains a column of type INTEGER PRIMARY KEY, then that column becomes an alias for the ROWID.". Looks good for me.
http://sqlite.org/autoinc.html

Oracle: Sequence and trigger used to fill in auto field. Existing type (INTEGER (11)) looks big enough.

Anssi Kääriäinen
Collaborator

I would take an approach where unsigned means "constrained to positive values" not anything regarding 2 billion vs 4 billion limits... That is, the important part is using an unsigned datatype if that is available, and restricting the values to positive values if possible. I just can't see how unsigned=True should change the datatype to bigserial.

Also, this pull request does not contain docs or tests - so this is not ready to be pulled in.

Pavel Zinovkin

You definitely should read related ticket. Whole point is not unsigned int itself, but "twice the integer space".

Anssi Kääriäinen
Collaborator

My take is that the issue is that MySQL allows for unsigned int fields - and you get thus 2x the integer space for free. On PostgreSQL unsigned integer fields are not supported, and thus you will not get the space for free. Now, if you want to specify the field as "guaranteed to hold 32bit unsigned integer values" then yes, bigserial is needed. You do get 2**32 space instead of 2x the space however.

I still stand by my point: it does not make sense to make signed=True to change the datatype from 4-byte signed integer to 8-byte signed integer on PostgreSQL. unsigned should mean negative values are not allowed.

Pavel Zinovkin

I see you point. Name of parameter was suggested in ticket comments. And I agree - it may not be clear.
About PostgreSQL bigserial - it's values range restricted to to this range according with docs: 1 to 9223372036854775807
http://www.postgresql.org/docs/8.4/static/datatype-numeric.html
I don't know if it works when you try to set value of field with sql.

How about to name parameter 'bigint=True' and set for mysql bigint as type. So for PostgreSQL and MySQL values range and price of storage will be equal (8 bytes, 1-9223372036854775807).

Anssi Kääriäinen
Collaborator

Interestingly enough the bigserial does accept negative values (tested on 9.1), despite the documentation.

So, I suggest this:

  • document the field as taking at least 32-bit unsigned integer values (up to 2**32 -1).
  • use bigserial on PostgreSQL, number(11) on Oracle, check SQLite for correct type
  • add check(col > 0) to the DB
Daniel Swarbrick

There's no reason why serial and bigserial wouldn't accept negative values. They're simply syntactical sugar around integer and bigint column types respectively, which create a sequence with an initial nextval() of 1. If you wanted to you, could reset the sequence's values to a negative value, and nextval() would count down (up?) to zero, then into positive numbers. I can't think of an immediate use case for this though ;-)

Pavel Zinovkin pzinovkin closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on May 7, 2012
  1. Pavel Zinovkin
This page is out of date. Refresh to see the latest.
3  django/db/backends/__init__.py
View
@@ -847,7 +847,8 @@ def convert_values(self, value, field):
internal_type = field.get_internal_type()
if internal_type == 'DecimalField':
return value
- elif internal_type and internal_type.endswith('IntegerField') or internal_type == 'AutoField':
+ elif (internal_type and internal_type.endswith('IntegerField')
+ or internal_type.endswith('AutoField')):
return int(value)
elif internal_type in ('DateField', 'DateTimeField', 'TimeField'):
return value
1  django/db/backends/mysql/creation.py
View
@@ -6,6 +6,7 @@ class DatabaseCreation(BaseDatabaseCreation):
# be interpolated against the values of Field.__dict__ before being output.
# If a column type is set to None, it won't be included in the output.
data_types = {
+ 'UnsignedAutoField': 'integer unsigned AUTO_INCREMENT',
'AutoField': 'integer AUTO_INCREMENT',
'BooleanField': 'bool',
'CharField': 'varchar(%(max_length)s)',
1  django/db/backends/oracle/creation.py
View
@@ -15,6 +15,7 @@ class DatabaseCreation(BaseDatabaseCreation):
# output (the "qn_" prefix is stripped before the lookup is performed.
data_types = {
+ 'UnsignedAutoField': 'NUMBER(11)',
'AutoField': 'NUMBER(11)',
'BooleanField': 'NUMBER(1) CHECK (%(qn_column)s IN (0,1))',
'CharField': 'NVARCHAR2(%(max_length)s)',
1  django/db/backends/postgresql_psycopg2/creation.py
View
@@ -10,6 +10,7 @@ class DatabaseCreation(BaseDatabaseCreation):
# be interpolated against the values of Field.__dict__ before being output.
# If a column type is set to None, it won't be included in the output.
data_types = {
+ 'UnsignedAutoField': 'bigserial',
'AutoField': 'serial',
'BooleanField': 'boolean',
'CharField': 'varchar(%(max_length)s)',
3  django/db/backends/sqlite3/base.py
View
@@ -191,7 +191,8 @@ def convert_values(self, value, field):
internal_type = field.get_internal_type()
if internal_type == 'DecimalField':
return util.typecast_decimal(field.format_number(value))
- elif internal_type and internal_type.endswith('IntegerField') or internal_type == 'AutoField':
+ elif (internal_type and internal_type.endswith('IntegerField')
+ or internal_type.endswith('AutoField')):
return int(value)
elif internal_type == 'DateField':
return parse_date(value)
1  django/db/backends/sqlite3/creation.py
View
@@ -7,6 +7,7 @@ class DatabaseCreation(BaseDatabaseCreation):
# thing" given more verbose field definitions, so leave them as is so that
# schema inspection is more useful.
data_types = {
+ 'UnsignedAutoField': 'integer',
'AutoField': 'integer',
'BooleanField': 'bool',
'CharField': 'varchar(%(max_length)s)',
43 django/db/models/fields/__init__.py
View
@@ -205,6 +205,13 @@ def clean(self, value, model_instance):
self.run_validators(value)
return value
+ def _internal_to_db_type(self, internal_type, connection):
+ data = DictWrapper(self.__dict__, connection.ops.quote_name, "qn_")
+ try:
+ return connection.creation.data_types[internal_type] % data
+ except KeyError:
+ return None
+
def db_type(self, connection):
"""
Returns the database column data type for this field, for the provided
@@ -225,12 +232,14 @@ def db_type(self, connection):
# mapped to one of the built-in Django field types. In this case, you
# can implement db_type() instead of get_internal_type() to specify
# exactly which wacky database column type you want to use.
- data = DictWrapper(self.__dict__, connection.ops.quote_name, "qn_")
- try:
- return (connection.creation.data_types[self.get_internal_type()]
- % data)
- except KeyError:
- return None
+ return self._internal_to_db_type(self.get_internal_type(), connection)
+
+ def rel_db_type(self, connection):
+ """
+ Returns the database column data type for related field referencing
+ to this.
+ """
+ return self.db_type(connection)
@property
def unique(self):
@@ -514,14 +523,20 @@ class AutoField(Field):
'invalid': _(u"'%s' value must be an integer."),
}
- def __init__(self, *args, **kwargs):
+ def __init__(self, verbose_name=None, name=None, unsigned=False, **kwargs):
assert kwargs.get('primary_key', False) is True, \
"%ss must have primary_key=True." % self.__class__.__name__
kwargs['blank'] = True
- Field.__init__(self, *args, **kwargs)
+ self.unsigned = unsigned
+ Field.__init__(self, verbose_name, name, **kwargs)
def get_internal_type(self):
- return "AutoField"
+ return 'AutoField' if not self.unsigned else 'UnsignedAutoField'
+
+ def rel_db_type(self, connection):
+ db_type = 'IntegerField' if not self.unsigned \
+ else 'PositiveIntegerField'
+ return self._internal_to_db_type(db_type, connection)
def to_python(self, value):
if value is None:
@@ -1139,6 +1154,11 @@ class PositiveIntegerField(IntegerField):
def get_internal_type(self):
return "PositiveIntegerField"
+ def rel_db_type(self, connection):
+ if connection.features.related_fields_match_type:
+ return self.db_type(connection)
+ return self._internal_to_db_type('IntegerField', connection)
+
def formfield(self, **kwargs):
defaults = {'min_value': 0}
defaults.update(kwargs)
@@ -1150,6 +1170,11 @@ class PositiveSmallIntegerField(IntegerField):
def get_internal_type(self):
return "PositiveSmallIntegerField"
+ def rel_db_type(self, connection):
+ if connection.features.related_fields_match_type:
+ return self.db_type(connection)
+ return self._internal_to_db_type('IntegerField', connection)
+
def formfield(self, **kwargs):
defaults = {'min_value': 0}
defaults.update(kwargs)
13 django/db/models/fields/related.py
View
@@ -1018,19 +1018,8 @@ def formfield(self, **kwargs):
return super(ForeignKey, self).formfield(**defaults)
def db_type(self, connection):
- # The database column type of a ForeignKey is the column type
- # of the field to which it points. An exception is if the ForeignKey
- # points to an AutoField/PositiveIntegerField/PositiveSmallIntegerField,
- # in which case the column type is simply that of an IntegerField.
- # If the database needs similar types for key fields however, the only
- # thing we can do is making AutoField an IntegerField.
rel_field = self.rel.get_related_field()
- if (isinstance(rel_field, AutoField) or
- (not connection.features.related_fields_match_type and
- isinstance(rel_field, (PositiveIntegerField,
- PositiveSmallIntegerField)))):
- return IntegerField().db_type(connection=connection)
- return rel_field.db_type(connection=connection)
+ return rel_field.rel_db_type(connection=connection)
class OneToOneField(ForeignKey):
"""
Something went wrong with that request. Please try again.