Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Fixed #22001 -- Ensure db_type is respected.

db_parameters should respect an already existing db_type method and
return that as its type string. In particular, this was causing some
fields from gis to not be generated.

Thanks to @bigsassy and @blueyed for their work on the patch.

Also fixed #22260
  • Loading branch information...
commit d22b291890c1736a40c0ad97448c7318df2eebb2 1 parent 37f7f23
@mjtamlyn mjtamlyn authored
View
1  AUTHORS
@@ -147,6 +147,7 @@ answer newbie questions, and generally made Django that much better:
Ricardo Javier Cárdenes Medina <ricardo.cardenes@gmail.com>
Jeremy Carbaugh <jcarbaugh@gmail.com>
Graham Carlyle <graham.carlyle@maplecroft.net>
+ Eric Palakovich Carr <carreric@gmail.com>
Juan Catalano <catalanojuan@gmail.com>
Antonio Cavedoni <http://cavedoni.com/>
cedric@terramater.net
View
0  django/contrib/gis/tests/migrations/__init__.py
No changes.
View
33 django/contrib/gis/tests/migrations/migrations/0001_initial.py
@@ -0,0 +1,33 @@
+from django.db import models, migrations
+import django.contrib.gis.db.models.fields
+
+
+# Used for regression test of ticket #22001: https://code.djangoproject.com/ticket/22001
+class Migration(migrations.Migration):
+
+ operations = [
+ migrations.CreateModel(
+ name='Neighborhood',
+ fields=[
+ (u'id', models.AutoField(verbose_name=u'ID', serialize=False, auto_created=True, primary_key=True)),
@bmispelon Collaborator

The u prefix breaks Python 3.2.

Was that file generated automatically?

@apollo13 Owner

I think so, migrations create them from time to time -- this should be a release blocker for 3.2 /cc @andrewgodwin

@andrewgodwin Owner

They still pop up occasionally, I'm trying to quell them (it's because repr() is used sometimes to make the string). Open a bug, but it's not a release blocker IMO - it's a very obvious bug to fix when you get the error, and it only happens if you make the migration on 2.7 and then run it on 3.2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ ('name', models.TextField(unique=True)),
+ ('geom', django.contrib.gis.db.models.fields.MultiPolygonField(srid=4326, null=True)),
+ ],
+ options={
+ },
+ bases=(models.Model,),
+ ),
+ migrations.CreateModel(
+ name='Household',
+ fields=[
+ (u'id', models.AutoField(verbose_name=u'ID', serialize=False, auto_created=True, primary_key=True)),
+ ('neighborhood', models.ForeignKey(to='gis.Neighborhood', to_field=u'id', null=True)),
+ ('address', models.TextField()),
+ ('zip_code', models.IntegerField(null=True, blank=True)),
+ ('geom', django.contrib.gis.db.models.fields.PointField(srid=4326, null=True, geography=True)),
+ ],
+ options={
+ },
+ bases=(models.Model,),
+ )
+ ]
View
0  django/contrib/gis/tests/migrations/migrations/__init__.py
No changes.
View
47 django/contrib/gis/tests/migrations/test_commands.py
@@ -0,0 +1,47 @@
+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.core.management import call_command
+from django.db import connection
+from django.test import override_settings, override_system_checks, TransactionTestCase
+
+
+class MigrateTests(TransactionTestCase):
+ """
+ Tests running the migrate command in Geodjango.
+ """
+ available_apps = ["django.contrib.gis"]
+
+ def get_table_description(self, table):
+ with connection.cursor() as cursor:
+ return connection.introspection.get_table_description(cursor, table)
+
+ def assertTableExists(self, table):
+ with connection.cursor() as cursor:
+ self.assertIn(table, connection.introspection.get_table_list(cursor))
+
+ def assertTableNotExists(self, table):
+ with connection.cursor() as cursor:
+ self.assertNotIn(table, connection.introspection.get_table_list(cursor))
+
+ @override_system_checks([])
+ @override_settings(MIGRATION_MODULES={"gis": "django.contrib.gis.tests.migrations.migrations"})
+ def test_migrate_gis(self):
+ """
+ Tests basic usage of the migrate command when a model uses Geodjango
+ fields. Regression test for ticket #22001:
+ https://code.djangoproject.com/ticket/22001
+ """
+ # Make sure no tables are created
+ self.assertTableNotExists("migrations_neighborhood")
+ self.assertTableNotExists("migrations_household")
+ # Run the migrations to 0001 only
+ call_command("migrate", "gis", "0001", verbosity=0)
+ # Make sure the right tables exist
+ self.assertTableExists("gis_neighborhood")
+ self.assertTableExists("gis_household")
+ # Unmigrate everything
+ call_command("migrate", "gis", "zero", verbosity=0)
+ # Make sure it's all gone
+ self.assertTableNotExists("gis_neighborhood")
+ self.assertTableNotExists("gis_household")
View
24 django/db/models/fields/__init__.py
@@ -511,7 +511,7 @@ def db_type(self, connection):
connection.
"""
# The default implementation of this method looks at the
- # backend-specific DATA_TYPES dictionary, looking up the field by its
+ # backend-specific data_types dictionary, looking up the field by its
# "internal type".
#
# A Field class can implement the get_internal_type() method to specify
@@ -525,24 +525,20 @@ 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.
- params = self.db_parameters(connection)
- if params['type']:
- if params['check']:
- return "%s CHECK (%s)" % (params['type'], params['check'])
- else:
- return params['type']
- return None
+ data = DictWrapper(self.__dict__, connection.ops.quote_name, "qn_")
+ try:
+ return connection.creation.data_types[self.get_internal_type()] % data
+ except KeyError:
+ return None
def db_parameters(self, connection):
"""
- Replacement for db_type, providing a range of different return
- values (type, checks)
+ Extension of db_type(), providing a range of different return
+ values (type, checks).
+ This will look at db_type(), allowing custom model fields to override it.
"""
data = DictWrapper(self.__dict__, connection.ops.quote_name, "qn_")
- try:
- type_string = connection.creation.data_types[self.get_internal_type()] % data
- except KeyError:
- type_string = None
+ type_string = self.db_type(connection)
try:
check_string = connection.creation.data_type_check_constraints[self.get_internal_type()] % data
except KeyError:
View
5 tests/field_subclassing/fields.py
@@ -74,3 +74,8 @@ def get_db_prep_save(self, value, connection):
if value is None:
return None
return json.dumps(value)
+
+
+class CustomTypedField(models.TextField):
+ def db_type(self, connection):
+ return 'custom_field'
View
10 tests/field_subclassing/tests.py
@@ -3,9 +3,10 @@
import inspect
from django.core import serializers
+from django.db import connection
from django.test import TestCase
-from .fields import Small
+from .fields import Small, CustomTypedField
from .models import DataModel, MyModel, OtherModel
@@ -104,3 +105,10 @@ def test_subfieldbase_plays_nice_with_module_inspect(self):
data = dict(inspect.getmembers(MyModel))
self.assertIn('__module__', data)
self.assertEqual(data['__module__'], 'field_subclassing.models')
+
+
+class TestDbType(TestCase):
+
+ def test_db_parameters_respects_db_type(self):
+ f = CustomTypedField()
+ self.assertEqual(f.db_parameters(connection)['type'], 'custom_field')

1 comment on commit d22b291

@bmispelon

The u prefix breaks Python 3.2.

Was that file generated automatically?

@apollo13

I think so, migrations create them from time to time -- this should be a release blocker for 3.2 /cc @andrewgodwin

@andrewgodwin

They still pop up occasionally, I'm trying to quell them (it's because repr() is used sometimes to make the string). Open a bug, but it's not a release blocker IMO - it's a very obvious bug to fix when you get the error, and it only happens if you make the migration on 2.7 and then run it on 3.2.

@bmispelon
Collaborator

FYI, this commit seems to have created a regression under postgres (and probably other DBs too): https://code.djangoproject.com/ticket/23416

Please sign in to comment.
Something went wrong with that request. Please try again.