Django

Code

Ticket #2720 (closed: fixed)

Opened 2 years ago

Last modified 4 months ago

Wrong syntax generated for foreign keys under MySQL

Reported by: anonymous Assigned to: nobody
Milestone: Component: Database layer (models, ORM)
Version: Keywords: innodb foreign key
Cc: plesur@maya.com, freakboy@iinet.net.au, not.com@gmail.com Triage Stage: Accepted
Has patch: 1 Needs documentation: 0
Needs tests: 0 Patch needs improvement: 1

Description (Last modified by adrian)

Django uses the following syntax to define foreign keys under MySQL/InnoDB:

CREATE TABLE `system_script_queue` (
    `id` integer AUTO_INCREMENT NOT NULL PRIMARY KEY,
    `execution_time` datetime NOT NULL,
    `period` integer NOT NULL,
    `system_script_id` integer NOT NULL REFERENCES `system_script` (`id`)
);

Using that syntax, maybe 1/3 of my foreign key constraints are being ignored by the DB.

If I'm reading this tech note correctly (http://dev.mysql.com/doc/refman/5.0/en/example-foreign-keys.html), that's the wrong syntax to use. The correct syntax is this one:

CREATE TABLE `system_script_queue` (
    `id` integer AUTO_INCREMENT NOT NULL PRIMARY KEY,
    `system_script_id` integer NOT NULL,
    `execution_time` datetime NOT NULL,
    `period` integer NOT NULL,
    foreign key (`system_script_id`) references `system_script` (`id`)
);

Using this syntax, all of my foreign key constraints seem to be successfully created.

Attachments

constraints.diff (3.3 kB) - added by Joakim Sernbrant <serbaut@gmail.com> on 09/13/06 17:03:09.
I think this patch should work.
ticket-2720.diff (3.3 kB) - added by Michael Radziej <mir@noris.de> on 01/29/07 22:13:25.
Updated patch, tested for postgresql8.1 and mysql4.1
any-order2720.diff (7.2 kB) - added by yary h <spm-django@yary.ack.org> on 02/09/07 19:54:52.
works for any ordering and for M2M references across different apps

Change History

09/13/06 13:41:15 changed by plesur@maya.com

(...just adding my email address....)

09/13/06 13:44:41 changed by ubernostrum

Have you double-checked that the foreign keys aren't being created later on as constraints? IIRC Django often creates the table without FOREIGN KEY constraints, and then adds those later in the run, once all the tables are created (this avoids problems caused by the order in which the tables are created).

09/13/06 14:06:04 changed by plesur@maya.com

Would that be evident in "manage.py sqlindexes <appname>"? I'm not seeing any constraints added there.

When I look at the SQL output by "manage.py sql <appname>", I see the first syntax used above; when I drop the table and recreate it with that foreign-key syntax, the table is created with no foreign key (it's consistent for a given table - if it fails once, it always fails).

When I drop the table and recreate it with the second syntax used above, it always succeeds.

09/13/06 14:54:38 changed by Joakim Sernbrant <serbaut@gmail.com>

With mysql/innodb you need to use the CONSTRAINT... syntax, the first form is just a "comment" (this fact is hidden in the docs somewhere):

CREATE TABLE `test_bar` (
    `id` integer AUTO_INCREMENT NOT NULL PRIMARY KEY,
    `foo_id` integer NOT NULL REFERENCES `test_foo` (`id`),
    CONSTRAINT FOREIGN KEY (`foo_id`) REFERENCES `test_foo` (`id`)
);

I have a patch for this and some other db related things I will submit in the near future.

09/13/06 15:44:20 changed by anonymous

Patch: Excellent, thank you! Will the patch be announced here, or should I be watching somewhere else?

09/13/06 17:03:09 changed by Joakim Sernbrant <serbaut@gmail.com>

  • attachment constraints.diff added.

I think this patch should work.

09/13/06 17:04:39 changed by Joakim Sernbrant <serbaut@gmail.com>

Note sure if this is compatible with all the other backends but my guess is that it is.

09/13/06 17:43:44 changed by plesur@maya.com

That seems to have done just what I needed - thanks again!

09/26/06 11:29:39 changed by adrian

  • description changed.

Fixed formatting in description.

01/29/07 21:26:05 changed by Michael Radziej <mir@noris.de>

  • cc set to plesur@maya.com.
  • has_patch set to 1.
  • summary changed from Wrong syntax generated for foreign keys under MySQL/InnoDB 5.0.22 to Wrong syntax generated for foreign keys under MySQL.
  • stage changed from Unreviewed to Ready for checkin.

mysql is strange, and this is a real bug in Django. Only foreign keys to models defined later escape this bug, because a different syntax is chosen for these. mysql4 and 5 are affected. The docs in mysql are pretty explicit that the used syntax is more or less read and thrown away (thank you, mysql, what a great idea ...)

Here's a model definition:

from django.db import models

class A(models.Model):
  pass

class B(models.Model):
  a = models.ForeignKey(A)

After syncdb, watch this:

mysql> select * from testapp_a;
Empty set (0.00 sec)

mysql> insert into testapp_b values (1,2);
Query OK, 1 row affected (0.03 sec)

I checked the patch, and it resolves the problem. Thanks, plesur@maya.com!

01/29/07 21:50:36 changed by Michael Radziej <mir@noris.de>

  • needs_better_patch set to 1.
  • stage changed from Ready for checkin to Accepted.

Ouch, the patch doesn't work with postgresql. I used 8.1.4:

mir@mahlzahn:testproj$ python manage.py syncdb
Creating table auth_message
Creating table auth_group
Creating table auth_user
Creating table auth_permission
Traceback (most recent call last):
  File "manage.py", line 11, in ?
    execute_manager(settings)
  File "/home/mir/lib/python/django/core/management.py", line 1462, in execute_manager
    execute_from_command_line(action_mapping, argv)
  File "/home/mir/lib/python/django/core/management.py", line 1370, in execute_from_command_line
    action_mapping[action](int(options.verbosity), options.interactive)
  File "/home/mir/lib/python/django/core/management.py", line 511, in syncdb
    cursor.execute(statement)
  File "/home/mir/lib/python/django/db/backends/util.py", line 12, in execute
    return self.cursor.execute(sql, params)
  File "/home/mir/lib/python/django/db/backends/postgresql/base.py", line 43, in execute
    return self.cursor.execute(sql, [smart_basestring(p, self.charset) for p in params])
psycopg.ProgrammingError: FEHLER:  Fehler »Syntaxfehler« bei »FOREIGN« at character 274

CREATE TABLE "auth_group_permissions" (
    "id" serial NOT NULL PRIMARY KEY,
    "group_id" integer NOT NULL REFERENCES "auth_group" ("id"),
    "permission_id" integer NOT NULL REFERENCES "auth_permission" ("id"),
    UNIQUE ("group_id", "permission_id"),
    CONSTRAINT FOREIGN KEY ("group_id") REFERENCES "auth_group" ("id"),
    CONSTRAINT FOREIGN KEY ("permission_id") REFERENCES "auth_permission" ("id")
);

01/29/07 22:13:25 changed by Michael Radziej <mir@noris.de>

  • attachment ticket-2720.diff added.

Updated patch, tested for postgresql8.1 and mysql4.1

01/29/07 22:19:28 changed by russellm

  • cc changed from plesur@maya.com to plesur@maya.com, freakboy@iinet.net.au.

(follow-up: ↓ 13 ) 01/29/07 22:25:10 changed by russellm

How does this patch interact with #3390 - in particular, the test for forward references in deserialization data? When I worked up that patch, I was surprised that there weren't any MySQL complaints about data consistency, but if I'm understanding this problem correctly, the constraints may not be operating correctly on MySQL...

(in reply to: ↑ 12 ) 01/29/07 22:51:23 changed by Michael Radziej <mir@noris.de>

Replying to russellm:

How does this patch interact with #3390 - in particular, the test for forward references in deserialization data? When I worked up that patch, I was surprised that there weren't any MySQL complaints about data consistency, but if I'm understanding this problem correctly, the constraints may not be operating correctly on MySQL...

Hmm, our two patches conflict with each other, and I'm too tired now to resolve the conflicts.

I got failures when I tried #3390 (without #2720) on a postgresql backend. You can probably get similar results when you reorder your model classes so that you only have forward references.

02/09/07 19:54:52 changed by yary h <spm-django@yary.ack.org>

  • attachment any-order2720.diff added.

works for any ordering and for M2M references across different apps

02/09/07 20:02:17 changed by yary h <spm-django@yary.ack.org>

  • cc changed from plesur@maya.com, freakboy@iinet.net.au to plesur@maya.com, freakboy@iinet.net.au, not.com@gmail.com.

_get_many_to_many_sql_for_model was assuming that all referred-to models were already created, that breaks when there's a ManyToMany? field referring to a model in a different app. I just uploaded a version of this patch which fixes that.

I'm having troulbe uploading tests, so get the 2k tarball here:http://yary.ack.org/intrapp.tgz

These tests took a while to get right, and still could use more refinement. They work well to ferret out the problem with MySQL/InnoDB, but will give spurious errors on a database that does not enforce foreign keys.

09/15/07 19:19:12 changed by PhiR

#4193 is also related to foreign keys.

09/15/07 19:48:48 changed by brosner

The patch I posted on #4193 should solve this problem. The issue was that models are cached in a Python dict which can have random orderings when iterated over. By using a SortedDict the ordering is maintained and SQL creation works fine.

06/18/08 11:47:15 changed by glassfordm

I believe the fix described in ticket #5729, which has already been accepted, fixes this issue in another way. Should this ticket be closed?

06/18/08 18:46:19 changed by russellm

  • status changed from new to closed.
  • resolution set to fixed.

Closing the ticket due to multiple reports that it has been fixed.


Add/Change #2720 (Wrong syntax generated for foreign keys under MySQL)




Change Properties
Action