#3615: Can't define forward references in fixtures using MySQL with InnoDB
-------------------------------------+-------------------------------------
Reporter: russellm | Owner: nobody
Type: Bug | Status: new
Milestone: | Component: Database layer
Version: SVN | (models, ORM)
Resolution: | Severity: Normal
Triage Stage: Accepted | Keywords: mysql innodb myisam
Needs documentation: 0 | reference fixture
Patch needs improvement: 0 | Has patch: 1
UI/UX: 0 | Needs tests: 0
| Easy pickings: 0
-------------------------------------+-------------------------------------
Changes (by jsdalton):
* needs_better_patch: 1 => 0
Comment:
Okay, an improved patch is now attached. I'm unchecking the patch needs
improvement flag; if anyone has any issues or concerns please feel free to
point them out and re-check the flag and I will take a look.
As mentioned in my previous comment, this patch allows for loading of data
with forward references in MySQL by turning foreign key checks off
temporarily. It also offers a mechanism to check the data after it was
entered to ensure invalid foreign keys were not entered while checks were
disabled.
The improved patch just cleans up a few rough edges from the previous
version.
* Added tests for `DatabaseIntrospection.get_key_columns()`
* Create meaningful error message when invalid row found. The message
includes which row, values and columns triggered the missing foreign key
error, which is useful for debugging bad data in fixture.
* Raise error on first bad row. As soon as
`check_for_invalid_foreign_keys()` hits a bad row, it raises an error.
Reason for this discussed below.
* Tweaked the API. The API is now more explicit:
`disable_foreign_key_checks()` and `enable_foreign_key_checks()`. These
now return boolean values (always True) when implemented. This means you
can determine if foreign key checks were disabled and then re-enable and
check for invalid foreign keys only if they were.
* Added tests disable/enable checks around necessary tests. I ran through
the test suite and tried to find places where MySQL required this pattern
for the test to load properly. I found one or two spots that Karen had in
her original patch and included those.
* Document new methods. Documentation was added as needed.
I didn't want to spend any more time gold plating this, but here are
possible ways this could be improved. I don't think any are really
necessary or in the scope of this ticket though.
* Improve fixture error reporting. As mentioned above, I decided to
trigger foreign key errors when using `check_for_invalid_foreign_keys()`
instead of collecting all bad rows and reporting them in the `loaddata`
error. The reason is that if we are going to make a feature for MySQL like
this, it should probably be implemented in all backends. It could be
useful when debugging bad fixture data, but I think it's unnecessary.
* Scope foreign key checks to given IDs. Right now,
`check_for_invalid_foreign_keys()` looks at the entire table and raises an
`IntegrityError` if it finds any bad data. `loaddata` would then roll back
the fixture load. An issue could arise where bad data is already
preexisting in the table where data is being loaded, and thus new data
would not be loaded because of the `IntegrityError` this would raise, even
though the new fixture data loaded does not actually have any problems.
Conceivably, you could pass a list of IDs along with the table name and
limit the checks to only those, but I felt this was an edge case and not
worth the complexity.
* Implement as context manager. It'd be cool to do...
{{{
with connection.foreign_key_checks_disabled():
# Load some data with forward references
}}}
...and then implement the checks in the `__exit__` method of the context
manager, but I couldn't think of a clean way to pass any of the tables
touched during any load data activities back to that method for checks.
Since this is really only used in loaddata and not much anywhere else it's
also probably overkill IMO.
I've tested this on MySQL 5.1.4 with InnoDB tables and it looks good. As I
said, if anyone spots any issues let me know.
--
Ticket URL: <https://code.djangoproject.com/ticket/3615#comment:26>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/django-updates?hl=en.
