Django

Code

Ticket #8316 (closed: fixed)

Opened 4 months ago

Last modified 3 months ago

MySQL error with custom primary_key that is not an IntegerField

Reported by: julianb Assigned to: julianb
Milestone: 1.0 Component: Database layer (models, ORM)
Version: SVN Keywords: 1.0-blocker
Cc: sebastian@sspaeth.de Triage Stage: Accepted
Has patch: 1 Needs documentation: 0
Needs tests: 1 Patch needs improvement: 1

Description

Create a model with a custom primary key field that is not an IntegerField? and another with a foreign key pointing to it:

class Author(models.Model):
    id = models.PositiveIntegerField(primary_key=True)
    name = models.CharField(max_length=200)

class Article(models.Model):
    title = models.CharField(max_length=200)
    author = models.ForeignKey(Author)

Let's see how the SQL looks:

BEGIN;
CREATE TABLE `dbtest_author` (
    `id` integer UNSIGNED NOT NULL PRIMARY KEY,
    `name` varchar(200) NOT NULL
)
;
CREATE TABLE `dbtest_article` (
    `id` integer AUTO_INCREMENT NOT NULL PRIMARY KEY,
    `title` varchar(200) NOT NULL,
    `author_id` integer NOT NULL
)
;
ALTER TABLE `dbtest_article` ADD CONSTRAINT author_id_refs_id_6bbed69f FOREIGN KEY (`author_id`) REFERENCES `dbtest_author` (`id`);
CREATE INDEX `dbtest_article_author_id` ON `dbtest_article` (`author_id`);
COMMIT;

You can see that author_id should reference dbtest_author.id, but they have different data types.

Calling syncdb:

Creating table dbtest_author
Creating table dbtest_article
Traceback (most recent call last):
  File "...manage.py", line 11, in <module>
    execute_manager(settings)
  File "C:\Programme\django_src\django\core\management\__init__.py", line 334, in execute_manager
    utility.execute()
  File "C:\Programme\django_src\django\core\management\__init__.py", line 295, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "C:\Programme\django_src\django\core\management\base.py", line 77, in run_from_argv
    self.execute(*args, **options.__dict__)
  File "C:\Programme\django_src\django\core\management\base.py", line 96, in execute
    output = self.handle(*args, **options)
  File "C:\Programme\django_src\django\core\management\base.py", line 178, in handle
    return self.handle_noargs(**options)
  File "C:\Programme\django_src\django\core\management\commands\syncdb.py", line 80, in handle_noargs
    cursor.execute(statement)
  File "C:\Programme\django_src\django\db\backends\util.py", line 19, in execute
    return self.cursor.execute(sql, params)
  File "C:\Programme\Python25\lib\site-packages\MySQLdb\cursors.py", line 166, in execute
    self.errorhandler(self, exc, value)
  File "C:\Programme\Python25\lib\site-packages\MySQLdb\connections.py", line 35, in defaulterrorhandler
    raise errorclass, errorvalue
_mysql_exceptions.OperationalError: (1005, "Can't create table '.\\db\\#sql-5b4_8.frm' (errno: 150)")

MySQL is unable to add the constraint, because of the different types (integer, integer unsigned) used.

The problem goes back to [1970] where a foreign key pointing to an AutoField?, a PositiveIntegerField? or a PositiveSmallIntegerField? always becomes an IntegerField?.

Attachments

db-mysqltypeissue.diff (3.4 kB) - added by julianb on 08/14/08 12:17:31.
ForeignKey? that points to an AutoField? now becomes KeyField?
db-mysqltypeissue.2.diff (5.5 kB) - added by julianb on 08/14/08 16:26:18.
v2
db-mysqltypeissue.3.diff (5.4 kB) - added by julianb on 08/14/08 16:31:44.
v3, removed some more now unneeded imports
db-mysqltypeissue.4.diff (5.9 kB) - added by julianb on 08/14/08 17:03:52.
v4, effectively handling that as a special MySQL case
db-mysqltypeissue.5.diff (2.1 kB) - added by julianb on 08/17/08 04:55:42.
v5, removed some unnecessary stuff
db-mysqltypeissue.6.diff (2.1 kB) - added by julianb on 08/17/08 04:57:39.
v6, fixed an oversight
db-mysqltypetest.diff (1.1 kB) - added by julianb on 08/17/08 08:13:41.
Test case, currently fails when using MySQL+Innodb
db-mysqltypeissue.7.diff (3.3 kB) - added by julianb on 08/17/08 15:59:20.

Change History

08/14/08 12:17:31 changed by julianb

  • attachment db-mysqltypeissue.diff added.

ForeignKey? that points to an AutoField? now becomes KeyField?

08/14/08 16:26:18 changed by julianb

  • attachment db-mysqltypeissue.2.diff added.

v2

08/14/08 16:31:44 changed by julianb

  • attachment db-mysqltypeissue.3.diff added.

v3, removed some more now unneeded imports

08/14/08 16:35:40 changed by julianb

  • needs_better_patch changed.
  • needs_tests changed.
  • needs_docs changed.

Whoops, the patch makes no sense yet, because it reverses [1970] for every backend, not just MySQL.

08/14/08 17:03:52 changed by julianb

  • attachment db-mysqltypeissue.4.diff added.

v4, effectively handling that as a special MySQL case

08/16/08 17:57:33 changed by mtredinnick

  • needs_better_patch set to 1.
  • needs_tests set to 1.
  • stage changed from Unreviewed to Accepted.

Only looking at the final patch, it seems to effectively do nothing. Specific problems are:

  • You cannot remove OneToOneField from creation.py. It's a valid field type.
  • This new KeyField class is never used in any way other than as an IntegerField, so it serves no purpose. Maybe it had some point in an earlier patch, but if you replace it with IntegerField here, no functionality changes.

What you probably want to do is use something like the db_type() from rel_field around line 721 of django.db.models.field.related.

The missing tests that need to be written would have shown all those problems. Running the existing test suite would have shown that OneToOneField was used in more than one place.

This is a legimate bug, but the ultimate patch will probably only change about 3 lines of code plus add some tests.

08/17/08 04:48:39 changed by julianb

I ran existing tests and they showed no problems. I did not quite know how to write tests specifically for that issue, because it needs MySQL with Innodb...

08/17/08 04:55:42 changed by julianb

  • attachment db-mysqltypeissue.5.diff added.

v5, removed some unnecessary stuff

08/17/08 04:57:39 changed by julianb

  • attachment db-mysqltypeissue.6.diff added.

v6, fixed an oversight

08/17/08 08:13:41 changed by julianb

  • attachment db-mysqltypetest.diff added.

Test case, currently fails when using MySQL+Innodb

08/17/08 15:33:47 changed by mtredinnick

Please make one combined patch so that things stay in sync.

Also, better to put the tests in regressiontests/model_fields/, since these are just tests guarding against a regression, not example usage (which is how a log of the modeltests tests are used in our automatically generated example docs).

08/17/08 15:59:20 changed by julianb

  • attachment db-mysqltypeissue.7.diff added.

08/17/08 16:08:08 changed by julianb

Thanks for the tips.

I was wondering if keys_need_similar_types = True (in case of MySQL) is the best we can use here. Maybe something like unsigned_keys = True, keys_only_integer = False or so would be more reusable, but they don't really appeal to me. What do you think?

08/30/08 03:25:48 changed by spaetz

  • cc set to sebastian@sspaeth.de.

Ouch, got bitten by this today. Latest patch seems to have tests, can people look at it and reset the tests flag if appropriate?

08/31/08 15:54:51 changed by jacob

  • keywords set to 1.0-blocker.

08/31/08 19:49:04 changed by mtredinnick

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

(In [8782]) Fixed #8316 -- Put tighter restrictions on the type of Foreign Key fields created for MySQL (because MySQL + InnoDB has those restrictions). Patch from julianb.


Add/Change #8316 (MySQL error with custom primary_key that is not an IntegerField)




Change Properties
Action