Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

Fixed #8901 -- Corrected the PostgreSQL sequence reset code when fiel…

…d identifiers exceed the maximum identifier length. Thanks to adam@zuerchertech.com for the report, and Matt Hoskins for the patch.

git-svn-id: http://code.djangoproject.com/svn/django/trunk@13328 bcc190cf-cafb-0310-a4f2-bffc1f526a37
  • Loading branch information...
commit 21a690fcfeeb3fecef3baa50ac9a9f0c14260a7e 1 parent 9ebf411
Russell Keith-Magee authored June 07, 2010
1  AUTHORS
@@ -220,6 +220,7 @@ answer newbie questions, and generally made Django that much better:
220 220
     Kieran Holland <http://www.kieranholland.com>
221 221
     Sung-Jin Hong <serialx.net@gmail.com>
222 222
     Leo "hylje" Honkanen <sealage@gmail.com>
  223
+    Matt Hoskins <skaffenuk@googlemail.com>
223 224
     Tareque Hossain <http://www.codexn.com>
224 225
     Richard House <Richard.House@i-logue.com>
225 226
     Robert Rock Howard <http://djangomojo.com/>
3  django/db/backends/postgresql/creation.py
... ...
@@ -1,4 +1,5 @@
1 1
 from django.db.backends.creation import BaseDatabaseCreation
  2
+from django.db.backends.util import truncate_name
2 3
 
3 4
 class DatabaseCreation(BaseDatabaseCreation):
4 5
     # This dictionary maps Field objects to their associated PostgreSQL column
@@ -51,7 +52,7 @@ def sql_indexes_for_field(self, model, f, style):
51 52
 
52 53
             def get_index_sql(index_name, opclass=''):
53 54
                 return (style.SQL_KEYWORD('CREATE INDEX') + ' ' +
54  
-                        style.SQL_TABLE(qn(index_name)) + ' ' +
  55
+                        style.SQL_TABLE(qn(truncate_name(index_name,self.connection.ops.max_name_length()))) + ' ' +
55 56
                         style.SQL_KEYWORD('ON') + ' ' +
56 57
                         style.SQL_TABLE(qn(db_table)) + ' ' +
57 58
                         "(%s%s)" % (style.SQL_FIELD(qn(f.column)), opclass) +
30  django/db/backends/postgresql/operations.py
@@ -54,7 +54,9 @@ def field_cast_sql(self, db_type):
54 54
         return '%s'
55 55
 
56 56
     def last_insert_id(self, cursor, table_name, pk_name):
57  
-        cursor.execute("SELECT CURRVAL('\"%s_%s_seq\"')" % (table_name, pk_name))
  57
+        # Use pg_get_serial_sequence to get the underlying sequence name
  58
+        # from the table name and column name (available since PostgreSQL 8)
  59
+        cursor.execute("SELECT CURRVAL(pg_get_serial_sequence('%s','%s'))" % (table_name, pk_name))
58 60
         return cursor.fetchone()[0]
59 61
 
60 62
     def no_limit_value(self):
@@ -90,13 +92,14 @@ def sql_flush(self, style, tables, sequences):
90 92
             for sequence_info in sequences:
91 93
                 table_name = sequence_info['table']
92 94
                 column_name = sequence_info['column']
93  
-                if column_name and len(column_name) > 0:
94  
-                    sequence_name = '%s_%s_seq' % (table_name, column_name)
95  
-                else:
96  
-                    sequence_name = '%s_id_seq' % table_name
97  
-                sql.append("%s setval('%s', 1, false);" % \
  95
+                if not (column_name and len(column_name) > 0):
  96
+                    # This will be the case if it's an m2m using an autogenerated
  97
+                    # intermediate table (see BaseDatabaseIntrospection.sequence_list)
  98
+                    column_name = 'id'
  99
+                sql.append("%s setval(pg_get_serial_sequence('%s','%s'), 1, false);" % \
98 100
                     (style.SQL_KEYWORD('SELECT'),
99  
-                    style.SQL_FIELD(self.quote_name(sequence_name)))
  101
+                    style.SQL_TABLE(table_name),
  102
+                    style.SQL_FIELD(column_name))
100 103
                 )
101 104
             return sql
102 105
         else:
@@ -110,11 +113,15 @@ def sequence_reset_sql(self, style, model_list):
110 113
             # Use `coalesce` to set the sequence for each model to the max pk value if there are records,
111 114
             # or 1 if there are none. Set the `is_called` property (the third argument to `setval`) to true
112 115
             # if there are records (as the max pk value is already in use), otherwise set it to false.
  116
+            # Use pg_get_serial_sequence to get the underlying sequence name from the table name
  117
+            # and column name (available since PostgreSQL 8)
  118
+
113 119
             for f in model._meta.local_fields:
114 120
                 if isinstance(f, models.AutoField):
115  
-                    output.append("%s setval('%s', coalesce(max(%s), 1), max(%s) %s null) %s %s;" % \
  121
+                    output.append("%s setval(pg_get_serial_sequence('%s','%s'), coalesce(max(%s), 1), max(%s) %s null) %s %s;" % \
116 122
                         (style.SQL_KEYWORD('SELECT'),
117  
-                        style.SQL_FIELD(qn('%s_%s_seq' % (model._meta.db_table, f.column))),
  123
+                        style.SQL_TABLE(model._meta.db_table),
  124
+                        style.SQL_FIELD(f.column),
118 125
                         style.SQL_FIELD(qn(f.column)),
119 126
                         style.SQL_FIELD(qn(f.column)),
120 127
                         style.SQL_KEYWORD('IS NOT'),
@@ -123,9 +130,10 @@ def sequence_reset_sql(self, style, model_list):
123 130
                     break # Only one AutoField is allowed per model, so don't bother continuing.
124 131
             for f in model._meta.many_to_many:
125 132
                 if not f.rel.through:
126  
-                    output.append("%s setval('%s', coalesce(max(%s), 1), max(%s) %s null) %s %s;" % \
  133
+                    output.append("%s setval(pg_get_serial_sequence('%s','%s'), coalesce(max(%s), 1), max(%s) %s null) %s %s;" % \
127 134
                         (style.SQL_KEYWORD('SELECT'),
128  
-                        style.SQL_FIELD(qn('%s_id_seq' % f.m2m_db_table())),
  135
+                        style.SQL_TABLE(model._meta.db_table),
  136
+                        style.SQL_FIELD('id'),
129 137
                         style.SQL_FIELD(qn('id')),
130 138
                         style.SQL_FIELD(qn('id')),
131 139
                         style.SQL_KEYWORD('IS NOT'),
19  tests/regressiontests/backends/models.py
... ...
@@ -1,5 +1,7 @@
  1
+from django.conf import settings
1 2
 from django.db import models
2  
-from django.db import connection
  3
+from django.db import connection, DEFAULT_DB_ALIAS
  4
+
3 5
 
4 6
 class Square(models.Model):
5 7
     root = models.IntegerField()
@@ -8,6 +10,7 @@ class Square(models.Model):
8 10
     def __unicode__(self):
9 11
         return "%s ** 2 == %s" % (self.root, self.square)
10 12
 
  13
+
11 14
 class Person(models.Model):
12 15
     first_name = models.CharField(max_length=20)
13 16
     last_name = models.CharField(max_length=20)
@@ -15,11 +18,25 @@ class Person(models.Model):
15 18
     def __unicode__(self):
16 19
         return u'%s %s' % (self.first_name, self.last_name)
17 20
 
  21
+
18 22
 class SchoolClass(models.Model):
19 23
     year = models.PositiveIntegerField()
20 24
     day = models.CharField(max_length=9, blank=True)
21 25
     last_updated = models.DateTimeField()
22 26
 
  27
+# Unfortunately, the following model breaks MySQL hard.
  28
+# Until #13711 is fixed, this test can't be run under MySQL.
  29
+if settings.DATABASES[DEFAULT_DB_ALIAS]['ENGINE'] != 'django.db.backends.mysql':
  30
+    class VeryLongModelNameZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZ(models.Model):
  31
+        class Meta:
  32
+            # We need to use a short actual table name or
  33
+            # we hit issue #8548 which we're not testing!
  34
+            verbose_name = 'model_with_long_table_name'
  35
+        primary_key_is_quite_long_zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz = models.AutoField(primary_key=True)
  36
+        charfield_is_quite_long_zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz = models.CharField(max_length=100)
  37
+        m2m_also_quite_long_zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz = models.ManyToManyField(Person,blank=True)
  38
+
  39
+
23 40
 qn = connection.ops.quote_name
24 41
 
25 42
 __test__ = {'API_TESTS': """
53  tests/regressiontests/backends/tests.py
... ...
@@ -1,13 +1,17 @@
1 1
 # -*- coding: utf-8 -*-
2 2
 # Unit and doctests for specific database backends.
3 3
 import datetime
4  
-import models
5 4
 import unittest
  5
+
  6
+from django.conf import settings
  7
+from django.core import management
  8
+from django.core.management.color import no_style
6 9
 from django.db import backend, connection, DEFAULT_DB_ALIAS
7 10
 from django.db.backends.signals import connection_created
8  
-from django.conf import settings
9 11
 from django.test import TestCase
10 12
 
  13
+from regressiontests.backends import models
  14
+
11 15
 class Callproc(unittest.TestCase):
12 16
 
13 17
     def test_dbms_session(self):
@@ -76,6 +80,7 @@ def test_django_extract(self):
76 80
         classes = models.SchoolClass.objects.filter(last_updated__day=20)
77 81
         self.assertEqual(len(classes), 1)
78 82
 
  83
+
79 84
 class ParameterHandlingTest(TestCase):
80 85
     def test_bad_parameter_count(self):
81 86
         "An executemany call with too many/not enough parameters will raise an exception (Refs #12612)"
@@ -88,6 +93,50 @@ def test_bad_parameter_count(self):
88 93
         self.assertRaises(Exception, cursor.executemany, query, [(1,2,3),])
89 94
         self.assertRaises(Exception, cursor.executemany, query, [(1,),])
90 95
 
  96
+# Unfortunately, the following tests would be a good test to run on all
  97
+# backends, but it breaks MySQL hard. Until #13711 is fixed, it can't be run
  98
+# everywhere (although it would be an effective test of #13711).
  99
+if settings.DATABASES[DEFAULT_DB_ALIAS]['ENGINE'] != 'django.db.backends.mysql':
  100
+    class LongNameTest(TestCase):
  101
+        """Long primary keys and model names can result in a sequence name
  102
+        that exceeds the database limits, which will result in truncation
  103
+        on certain databases (e.g., Postgres). The backend needs to use
  104
+        the correct sequence name in last_insert_id and other places, so
  105
+        check it is. Refs #8901.
  106
+        """
  107
+
  108
+        def test_sequence_name_length_limits_create(self):
  109
+            """Test creation of model with long name and long pk name doesn't error. Ref #8901"""
  110
+            models.VeryLongModelNameZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZ.objects.create()
  111
+
  112
+        def test_sequence_name_length_limits_m2m(self):
  113
+            """Test an m2m save of a model with a long name and a long m2m field name doesn't error as on Django >=1.2 this now uses object saves. Ref #8901"""
  114
+            obj = models.VeryLongModelNameZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZ.objects.create()
  115
+            rel_obj = models.Person.objects.create(first_name='Django', last_name='Reinhardt')
  116
+            obj.m2m_also_quite_long_zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz.add(rel_obj)
  117
+
  118
+        def test_sequence_name_length_limits_flush(self):
  119
+            """Test that sequence resetting as part of a flush with model with long name and long pk name doesn't error. Ref #8901"""
  120
+            # A full flush is expensive to the full test, so we dig into the
  121
+            # internals to generate the likely offending SQL and run it manually
  122
+
  123
+            # Some convenience aliases
  124
+            VLM = models.VeryLongModelNameZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZ
  125
+            VLM_m2m = VLM.m2m_also_quite_long_zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz.through
  126
+            tables = [
  127
+                VLM._meta.db_table,
  128
+                VLM_m2m._meta.db_table,
  129
+            ]
  130
+            sequences = [
  131
+                {
  132
+                    'column': VLM._meta.pk.column,
  133
+                    'table': VLM._meta.db_table
  134
+                },
  135
+            ]
  136
+            cursor = connection.cursor()
  137
+            for statement in connection.ops.sql_flush(no_style(), tables, sequences):
  138
+                cursor.execute(statement)
  139
+
91 140
 
92 141
 def connection_created_test(sender, **kwargs):
93 142
     print 'connection_created signal'

0 notes on commit 21a690f

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