Opened 12 years ago

Closed 12 years ago

#17530 closed Bug (fixed)

Fixture loading with deferred constraints broken for Postgres for fixtures from multiple dirs

Reported by: Luke Plant Owned by: nobody
Component: Core (Management commands) Version: 1.3
Severity: Release blocker Keywords:
Cc: L.Plant.98@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

If you call 'loaddata' such that fixtures are loaded from multiple directories, and these fixtures depend on each other, with Postgres you can get a constraint exception, e.g.

Problem installing fixture '/home/luke/devel/mezzanine/mezzanine/forms/fixtures/mezzanine.json': Traceback (most recent call last):
  File "/home/luke/devel/django/trunk/django/core/management/commands/loaddata.py", line 207, in handle
    connection.check_constraints(table_names=table_names)
  File "/home/luke/devel/django/trunk/django/db/backends/postgresql_psycopg2/base.py", line 129, in check_constraints
    self.cursor().execute('SET CONSTRAINTS ALL IMMEDIATE')
  File "/home/luke/devel/django/trunk/django/db/backends/util.py", line 36, in execute
    return self.cursor.execute(sql, params)
  File "/home/luke/devel/django/trunk/django/db/backends/postgresql_psycopg2/base.py", line 52, in execute
    return self.cursor.execute(query, args)
IntegrityError: insert or update on table "forms_form" violates foreign key constraint "page_ptr_id_refs_id_1d5610fd"
DETAIL:  Key (page_ptr_id)=(3) is not present in table "pages_page".

This is contrary to the docs, which says that although the order in which fixtures are created is undefined, it doesn't matter and forward references will work because the constraints are only checked at the end.

The bug is obvious from eyeballing the code: the call to check_constraints occurs inside the 'for fixture_dir in fixture_dirs' iteration, instead of outside it.

This is a regression since 1.3.X, almost certainly from [16590], and is a release blocker.

Attachments (2)

17530_fix.diff (10.1 KB ) - added by Luke Plant 12 years ago.
Possible fix
17530_fix_and_tests.diff (15.5 KB ) - added by Luke Plant 12 years ago.
corrected patch, hopefully, with tests

Download all attachments as: .zip

Change History (6)

by Luke Plant, 12 years ago

Attachment: 17530_fix.diff added

Possible fix

comment:1 by Luke Plant, 12 years ago

Cc: L.Plant.98@… added
Has patch: set
Needs tests: set

The patch I attached is the most naive attempt to fix this - it just pulls out the with connection.constraint_checks_disabled(): line and the connection.check_constraints line from the loop. It does fix my issue. I haven't even run the test suite, and I haven't created a test yet. The patch might, nonetheless, be correct.

by Luke Plant, 12 years ago

Attachment: 17530_fix_and_tests.diff added

corrected patch, hopefully, with tests

comment:2 by Luke Plant, 12 years ago

Needs tests: unset

The previous patch was not correct, since it did not handle exceptions properly, and in fact the 'with connection.constraint_checks_disabled()' line needed to be a further level of iteration out.

The revised patch contains tests, and all are passing with Postgres, sqlite, and MySQL Innodb. The diff is confusing due to lots of indentation changes, but actually is not that complex:

1) it moves the nesting of the main 'try/except Exception' and the 'with' block to outside the top level iteration, so that check_constraints is called after all fixtures are installed.

2) it adds an inner 'try/finally' to deal with the fixture.close() call.

3) it also removes an 'else' clause that is not required due to a 'return' at the end of the 'if label_found' branch, in order to remove a level of indentation.

If someone else could review this, it would help.

comment:3 by Claude Paroz, 12 years ago

Triage Stage: UnreviewedReady for checkin

Yes, it's easier to review side-by-side! Seems totally fine. Ran once again the test suite with Postgres.

comment:4 by Luke Plant, 12 years ago

Resolution: fixed
Status: newclosed

In [17372]:

Fixed #17530 - Fixture loading with deferred constraints broken for Postgres for fixtures from multiple dirs/files

Thanks to claudep for the review.

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