Index: docs/ref/databases.txt =================================================================== --- docs/ref/databases.txt (revision 16483) +++ docs/ref/databases.txt (working copy) @@ -142,6 +142,18 @@ The InnoDB_ engine is fully transactional and supports foreign key references and is probably the best choice at this point in time. +.. versionchanged:: 1.4 + +In previous versions of Django, fixtures with forward references (i.e. +relations to rows that have not yet been inserted into the database) would fail +to load when using the InnoDB storage engine. This was due to the fact that InnoDB +deviates from the SQL standard by checking foreign key constraints immediately +instead of deferring the check until the transaction is committed. This +problem has been resolved in Django 1.4. Fixture data is now loaded with foreign key +checks turned off; foreign key checks are then re-enabled when the data has +finished loading, at which point the entire table is checked for invalid foreign +key references and an `IntegrityError` is raised if any are found. + .. _storage engines: http://dev.mysql.com/doc/refman/5.5/en/storage-engines.html .. _MyISAM: http://dev.mysql.com/doc/refman/5.5/en/myisam-storage-engine.html .. _InnoDB: http://dev.mysql.com/doc/refman/5.5/en/innodb.html Index: django/db/backends/sqlite3/base.py =================================================================== --- django/db/backends/sqlite3/base.py (revision 16483) +++ django/db/backends/sqlite3/base.py (working copy) @@ -204,7 +204,7 @@ self.connection.create_function("django_format_dtdelta", 5, _sqlite_format_dtdelta) connection_created.send(sender=self.__class__, connection=self) return self.connection.cursor(factory=SQLiteCursorWrapper) - + def close(self): # If database is in memory, closing the connection destroys the # database. To prevent accidental data loss, ignore close requests on Index: django/db/backends/sqlite3/introspection.py =================================================================== --- django/db/backends/sqlite3/introspection.py (revision 16483) +++ django/db/backends/sqlite3/introspection.py (working copy) @@ -103,6 +103,35 @@ return relations + def get_key_columns(self, cursor, table_name): + """ + Returns a list of (column_name, referenced_table_name, referenced_column_name) for all + key columns in given table. + """ + key_columns = [] + + # Schema for this table + cursor.execute("SELECT sql FROM sqlite_master WHERE tbl_name = %s AND type = %s", [table_name, "table"]) + results = cursor.fetchone()[0].strip() + results = results[results.index('(')+1:results.rindex(')')] + + # Walk through and look for references to other tables. SQLite doesn't + # really have enforced references, but since it echoes out the SQL used + # to create the table we can look for REFERENCES statements used there. + for field_index, field_desc in enumerate(results.split(',')): + field_desc = field_desc.strip() + if field_desc.startswith("UNIQUE"): + continue + + m = re.search('"(.*)".*references (.*) \(["|](.*)["|]\)', field_desc, re.I) + if not m: + continue + + # This will append (column_name, referenced_table_name, referenced_column_name) to key_columns + key_columns.append(tuple([s.strip('"') for s in m.groups()])) + + return key_columns + def get_indexes(self, cursor, table_name): """ Returns a dictionary of fieldname -> infodict for the given table, Index: django/db/backends/mysql/base.py =================================================================== --- django/db/backends/mysql/base.py (revision 16483) +++ django/db/backends/mysql/base.py (working copy) @@ -25,7 +25,7 @@ from MySQLdb.converters import conversions from MySQLdb.constants import FIELD_TYPE, CLIENT -from django.db import utils +from django.db import utils, IntegrityError from django.db.backends import * from django.db.backends.signals import connection_created from django.db.backends.mysql.client import DatabaseClient @@ -349,3 +349,19 @@ raise Exception('Unable to determine MySQL version from version string %r' % self.connection.get_server_info()) self.server_version = tuple([int(x) for x in m.groups()]) return self.server_version + + def disable_constraint_checking(self): + """ + Disables foreign key checks, primarily for use in adding rows with forward references. Always returns True, + to indicate constraint checks have been disabled. + """ + self.cursor().execute('SET foreign_key_checks=0') + return True + + def enable_constraint_checking(self): + """ + Re-enable foreign key checks after they have been disabled. Always returns True, + to indicate checks have been re-enabled. + """ + self.cursor().execute('SET foreign_key_checks=1') + return True Index: django/db/backends/mysql/introspection.py =================================================================== --- django/db/backends/mysql/introspection.py (revision 16483) +++ django/db/backends/mysql/introspection.py (working copy) @@ -51,10 +51,21 @@ representing all relationships to the given table. Indexes are 0-based. """ my_field_dict = self._name_to_index(cursor, table_name) - constraints = [] + constraints = self.get_key_columns(cursor, table_name) relations = {} + for my_fieldname, other_table, other_field in constraints: + other_field_index = self._name_to_index(cursor, other_table)[other_field] + my_field_index = my_field_dict[my_fieldname] + relations[my_field_index] = (other_field_index, other_table) + return relations + + def get_key_columns(self, cursor, table_name): + """ + Returns a list of (column_name, referenced_table_name, referenced_column_name) for all + key columns in given table. + """ + key_columns = [] try: - # This should work for MySQL 5.0. cursor.execute(""" SELECT column_name, referenced_table_name, referenced_column_name FROM information_schema.key_column_usage @@ -62,7 +73,7 @@ AND table_schema = DATABASE() AND referenced_table_name IS NOT NULL AND referenced_column_name IS NOT NULL""", [table_name]) - constraints.extend(cursor.fetchall()) + key_columns.extend(cursor.fetchall()) except (ProgrammingError, OperationalError): # Fall back to "SHOW CREATE TABLE", for previous MySQL versions. # Go through all constraints and save the equal matches. @@ -74,15 +85,9 @@ if match == None: break pos = match.end() - constraints.append(match.groups()) + key_columns.append(match.groups()) + return key_columns - for my_fieldname, other_table, other_field in constraints: - other_field_index = self._name_to_index(cursor, other_table)[other_field] - my_field_index = my_field_dict[my_fieldname] - relations[my_field_index] = (other_field_index, other_table) - - return relations - def get_indexes(self, cursor, table_name): """ Returns a dictionary of fieldname -> infodict for the given table, Index: django/db/backends/__init__.py =================================================================== --- django/db/backends/__init__.py (revision 16483) +++ django/db/backends/__init__.py (working copy) @@ -239,6 +239,51 @@ if self.savepoint_state: self._savepoint_commit(sid) + def disable_constraint_checking(self): + """ + Backends can implement as needed to temporarily disable foreign key constraint + checking. If a backend does not require post-load constraint checking, then this method + can be overridden and False can be returned. + """ + return True + + def enable_constraint_checking(self): + """ + Backends can implement as needed to re-enable foreign key constraint checking. + """ + pass + + def check_constraints(self, table_names): + """ + Checks each table name in table-names for rows with invalid foreign key references. This method is + intended to be used in conjunction with `disable_constraint_checking()` and `enable_constraint_checking()`, to + determine if rows with invalid references were entered while constraint checks were off. + + Raises an IntegrityError on the first invalid foreign key reference encountered (if any) and provides + detailed information about the invalid reference in the error message. + + Backends can override this method if they can more directly apply constraint checking (e.g. via "SET CONSTRAINTS + ALL IMMEDIATE") + """ + cursor = self.cursor() + for table_name in table_names: + key_columns = self.introspection.get_key_columns(cursor, table_name) + for column_name, referenced_table_name, referenced_column_name in key_columns: + primary_key_column_name = [c[0] for c in self.introspection.get_indexes(cursor, table_name).iteritems() if c[1]['primary_key']][0] + cursor.execute(""" + SELECT REFERRING.`%s`, REFERRING.`%s` FROM `%s` as REFERRING + LEFT JOIN `%s` as REFERRED + ON (REFERRING.`%s` = REFERRED.`%s`) + WHERE REFERRING.`%s` IS NOT NULL + AND REFERRED.`%s` IS NULL""" + % (primary_key_column_name, column_name, table_name, referenced_table_name, + column_name, referenced_column_name, column_name, referenced_column_name)) + for bad_row in cursor.fetchall(): + raise IntegrityError("The row in table '%s' with primary key '%s' has an invalid \ +foreign key: %s.%s contains a value '%s' that does not have a corresponding value in %s.%s." + % (table_name, bad_row[0], table_name, column_name, bad_row[1], + referenced_table_name, referenced_column_name)) + def close(self): if self.connection is not None: self.connection.close() @@ -870,6 +915,13 @@ return sequence_list + def get_key_columns(self, cursor, table_name): + """ + Backends should override this to return a list of (column_name, referenced_table_name, + referenced_column_name) for all key columns in given table. + """ + return [] + class BaseDatabaseClient(object): """ This class encapsulates all backend-specific methods for opening a Index: django/db/backends/dummy/base.py =================================================================== --- django/db/backends/dummy/base.py (revision 16483) +++ django/db/backends/dummy/base.py (working copy) @@ -34,6 +34,7 @@ get_table_description = complain get_relations = complain get_indexes = complain + get_key_columns = complain class DatabaseWrapper(BaseDatabaseWrapper): operators = {} Index: django/core/management/commands/loaddata.py =================================================================== --- django/core/management/commands/loaddata.py (revision 16483) +++ django/core/management/commands/loaddata.py (working copy) @@ -8,7 +8,7 @@ from django.core import serializers from django.core.management.base import BaseCommand from django.core.management.color import no_style -from django.db import connections, router, transaction, DEFAULT_DB_ALIAS +from django.db import connections, router, transaction, DEFAULT_DB_ALIAS, models as db_models from django.db.models import get_apps from django.utils.itercompat import product @@ -166,12 +166,21 @@ (format, fixture_name, humanize(fixture_dir))) try: objects = serializers.deserialize(format, fixture, using=using) + constraint_checking_disabled = connection.disable_constraint_checking() for obj in objects: objects_in_fixture += 1 if router.allow_syncdb(using, obj.object.__class__): loaded_objects_in_fixture += 1 models.add(obj.object.__class__) obj.save(using=using) + + # If we disabled constraint checks, then we should re-enable them and check for + # any invalid keys that might have been added + if constraint_checking_disabled: + connection.enable_constraint_checking() + table_names = [model._meta.db_table for model in models] + connection.check_constraints(table_names=table_names) + loaded_object_count += loaded_objects_in_fixture fixture_object_count += objects_in_fixture label_found = True Index: tests/modeltests/serializers/tests.py =================================================================== --- tests/modeltests/serializers/tests.py (revision 16483) +++ tests/modeltests/serializers/tests.py (working copy) @@ -5,7 +5,7 @@ from django.conf import settings from django.core import serializers -from django.db import transaction +from django.db import transaction, connection from django.test import TestCase, TransactionTestCase, Approximate from django.utils import simplejson, unittest @@ -252,8 +252,10 @@ transaction.enter_transaction_management() transaction.managed(True) objs = serializers.deserialize(self.serializer_name, self.fwd_ref_str) + connection.disable_constraint_checking() for obj in objs: obj.save() + connection.enable_constraint_checking() transaction.commit() transaction.leave_transaction_management() Index: tests/regressiontests/serializers_regress/tests.py =================================================================== --- tests/regressiontests/serializers_regress/tests.py (revision 16483) +++ tests/regressiontests/serializers_regress/tests.py (working copy) @@ -383,7 +383,9 @@ objects = [] instance_count = {} for (func, pk, klass, datum) in test_data: + connection.disable_constraint_checking() objects.extend(func[0](pk, klass, datum)) + connection.enable_constraint_checking() # Get a count of the number of objects created for each class for klass in instance_count: Index: tests/regressiontests/introspection/tests.py =================================================================== --- tests/regressiontests/introspection/tests.py (revision 16483) +++ tests/regressiontests/introspection/tests.py (working copy) @@ -96,6 +96,11 @@ # That's {field_index: (field_index_other_table, other_table)} self.assertEqual(relations, {3: (0, Reporter._meta.db_table)}) + def test_get_key_columns(self): + cursor = connection.cursor() + key_columns = connection.introspection.get_key_columns(cursor, Article._meta.db_table) + self.assertEqual(key_columns, [(u'reporter_id', Reporter._meta.db_table, u'id')]) + def test_get_indexes(self): cursor = connection.cursor() indexes = connection.introspection.get_indexes(cursor, Article._meta.db_table) Index: tests/regressiontests/fixtures_regress/fixtures/forward_ref_bad_data.json =================================================================== --- tests/regressiontests/fixtures_regress/fixtures/forward_ref_bad_data.json (revision 0) +++ tests/regressiontests/fixtures_regress/fixtures/forward_ref_bad_data.json (revision 0) @@ -0,0 +1,17 @@ +[ + { + "pk": 1, + "model": "fixtures_regress.book", + "fields": { + "name": "Cryptonomicon", + "author": 3 + } + }, + { + "pk": "4", + "model": "fixtures_regress.person", + "fields": { + "name": "Neal Stephenson" + } + } +] \ No newline at end of file Index: tests/regressiontests/fixtures_regress/fixtures/forward_ref.json =================================================================== --- tests/regressiontests/fixtures_regress/fixtures/forward_ref.json (revision 0) +++ tests/regressiontests/fixtures_regress/fixtures/forward_ref.json (revision 0) @@ -0,0 +1,17 @@ +[ + { + "pk": 1, + "model": "fixtures_regress.book", + "fields": { + "name": "Cryptonomicon", + "author": 4 + } + }, + { + "pk": "4", + "model": "fixtures_regress.person", + "fields": { + "name": "Neal Stephenson" + } + } +] \ No newline at end of file Index: tests/regressiontests/fixtures_regress/tests.py =================================================================== --- tests/regressiontests/fixtures_regress/tests.py (revision 16483) +++ tests/regressiontests/fixtures_regress/tests.py (working copy) @@ -12,7 +12,7 @@ from django.core.management.commands.dumpdata import sort_dependencies from django.core.management.base import CommandError from django.db.models import signals -from django.db import transaction +from django.db import transaction, IntegrityError from django.test import TestCase, TransactionTestCase, skipIfDBFeature, \ skipUnlessDBFeature @@ -362,6 +362,35 @@ """[{"pk": %d, "model": "fixtures_regress.widget", "fields": {"name": "grommet"}}]""" % widget.pk ) + + def test_loaddata_works_when_fixture_has_forward_refs(self): + """ + Regression for #3615 - Forward references cause fixtures not to load in MySQL (InnoDB) + """ + management.call_command( + 'loaddata', + 'forward_ref.json', + verbosity=0, + commit=False + ) + self.assertEqual(Book.objects.all()[0].id, 1) + self.assertEqual(Person.objects.all()[0].id, 4) + + def test_loaddata_raises_error_when_fixture_has_invalid_foreign_key(self): + """ + Regression for #3615 - Ensure data with nonexistent child key references raises error + """ + stderr = StringIO() + management.call_command( + 'loaddata', + 'forward_ref_bad_data.json', + verbosity=0, + commit=False, + stderr=stderr, + ) + self.assertTrue( + stderr.getvalue().startswith('Problem installing fixture') + ) class NaturalKeyFixtureTests(TestCase):