Opened 10 years ago

Closed 10 years ago

Last modified 8 years ago

#1165 closed defect (fixed)

[patch] Wrong column name in (superfluous?) constraint

Reported by: avandorp@… Owned by: adrian
Component: Core (Other) Version: master
Severity: major Keywords:
Cc: Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

The following (simplified) model gives the SQL error "ERROR: column
"invoice_number" does not exist":

class Invoice(meta.Model):
    invoice_number = meta.PositiveIntegerField(primary_key=True)

class Invoice_item(meta.Model): # General invoice item
    invoice_number = meta.ForeignKey(Invoice)
    id = meta.AutoField('ID', primary_key=True)

This is to expected because the generated SQL for Invoice_item looks like this:

CREATE TABLE "invoicing_invoice_items" (
    "invoice_number_id" integer CHECK ("invoice_number" >= 0) NOT NULL
REFERENCES "invoicing_invoices" ("invoice_number"),
    "id" serial NOT NULL PRIMARY KEY
);

Dody Suria Wijaya on the user mailinglist suggested the following change:

Index: django/core/management.py
===================================================================
--- django/core/management.py   (revision 1817)
+++ django/core/management.py   (working copy)
@@ -74,7 +74,7 @@
                 data_type = f.get_internal_type()
             col_type = db.DATA_TYPES[data_type]
             if col_type is not None:
-                field_output = [db.db.quote_name(f.column), col_type % rel_field.__dict__]
+                field_output = [db.db.quote_name(f.column), col_type % f.__dict__]
                 field_output.append('%sNULL' % (not f.null and 'NOT ' or ''))
                 if f.unique:
                     field_output.append('UNIQUE')

I don't know, whether that's all. Where does this additional _id come from? Isn't that only for primary keys? What's that CHECK clause doing there anyway, it's already in the referenced table?

Thanks for your work on Django!

Attachments (1)

djangopatch (1.1 KB) - added by avandorp@… 10 years ago.
Proposed patch

Download all attachments as: .zip

Change History (7)

comment:1 Changed 10 years ago by avandorp@…

  • Version set to SVN

The suggested change definitely isn't enough. With this change, a ForeignKeyField of (referenced) type CharField(maxlength=10) would get an SQL statement with varchar(None). Unfortunately I won't find time to delve any deeper the coming weeks.

comment:2 Changed 10 years ago by avandorp@…

The following patch works for me:

Index: django/core/management.py
===================================================================
--- django/core/management.py   (revision 1929)
+++ django/core/management.py   (working copy)
@@ -53,10 +53,14 @@
 def _is_valid_dir_name(s):
     return bool(re.search(r'^\w+$', s))

-# If the foreign key points to an AutoField, the foreign key should be an
-# IntegerField, not an AutoField. Otherwise, the foreign key should be the same
-# type of field as the field to which it points.
-get_rel_data_type = lambda f: (f.get_internal_type() == 'AutoField') and 'IntegerField' or f.get_internal_type()
+# If the foreign key points to an AutoField, a PositiveIntegerField or a
+# PositiveSmallIntegerField, the foreign key should be an
+# IntegerField, not of the referred field type. Otherwise, the foreign key should
+# be the same type of field as the field to which it points.
+get_rel_data_type = lambda f: (f.get_internal_type() == 'AutoField') and 'IntegerField' \
+    or (f.get_internal_type() == 'PositiveIntegerField') and 'IntegerField' \
+    or (f.get_internal_type() == 'PositiveSmallIntegerField') and 'SmallIntegerField' \
+    or f.get_internal_type()

 def get_sql_create(mod):
     "Returns a list of the CREATE TABLE SQL statements for the given module."

comment:3 Changed 10 years ago by anonymous

Arrgh. Sorry for the bugspam.

+    or (f.get_internal_type() == 'PositiveSmallIntegerField') and 'SmallIntegerField' \

should be:

+    or (f.get_internal_type() == 'PositiveSmallIntegerField') and 'IntegerField' \

If you like it smaller and more pythonique:

Index: django/core/management.py
===================================================================
--- django/core/management.py   (revision 1929)
+++ django/core/management.py   (working copy)
@@ -53,10 +53,13 @@
 def _is_valid_dir_name(s):
     return bool(re.search(r'^\w+$', s))

-# If the foreign key points to an AutoField, the foreign key should be an
-# IntegerField, not an AutoField. Otherwise, the foreign key should be the same
-# type of field as the field to which it points.
-get_rel_data_type = lambda f: (f.get_internal_type() == 'AutoField') and 'IntegerField' or f.get_internal_type()
+# If the foreign key points to an AutoField, a PositiveIntegerField or a
+# PositiveSmallIntegerField, the foreign key should be an
+# IntegerField, not the reffered field type. Otherwise, the foreign key should
+# be the same type of field as the field to which it points.
+get_rel_data_type = lambda f: (f.get_internal_type() in \
+    ('AutoField', 'PositiveIntegerField', 'PositiveSmallIntegerField')) and 'IntegerField' \
+    or f.get_internal_type()

 def get_sql_create(mod):
     "Returns a list of the CREATE TABLE SQL statements for the given module."

Changed 10 years ago by avandorp@…

Proposed patch

comment:4 Changed 10 years ago by avandorp@…

  • Summary changed from Wrong column name in (superfluous?) constraint to [patch] Wrong column name in (superfluous?) constraint

comment:5 Changed 10 years ago by adrian

  • Resolution set to fixed
  • Status changed from new to closed

(In [1970]) Fixed #1165 -- Fixed bug in generated CREATE TABLE statements where a primary key related to a PositiveIntegerField or PositiveSmallIntegerField. Thanks, avandorp@…

comment:6 Changed 8 years ago by anonymous

seduction Here's my post on how to get laid in the next month or two. This post does not maximize your chances of "getting really good at pickup" - in fact, it does the opposite.

Note: See TracTickets for help on using tickets.
Back to Top