Thoughts on solution to forward references in MySQL (#3615)

99 views
Skip to first unread message

Jim D.

unread,
Jun 27, 2011, 5:24:26 PM6/27/11
to Django developers
Hi all,

I spent some time last week and over the weekend nailing down a
solution for https://code.djangoproject.com/ticket/3615 . This is the
ticket about allowing forward references when loading data on the
MySQL InnoDB backend. My patch implements the proposed change
(disabling foreign key checks when the data is loaded) as well as a
straightforward SQL SELECT check for integrity after the data is
loaded, which if I understand it is the missing piece that has
prevented this ticket from moving forward for the last 4 years...

Anyhow, the patch should be 100% there so I'd love if someone could
check it out and either push it along or let me know if any changes
are required. It should be easy enough for me to address any issues
while the whole problem is in my head.

I'm using django-threadedcomments on a project, which has forward
references in one of its test fixtures, so I'm reminded of this issue
every time I run my project tests. I hate test errors! I'm hopeful the
latest patch is sufficient to get this issue resolved.

Thanks
Jim

Jacob Kaplan-Moss

unread,
Jun 27, 2011, 6:08:03 PM6/27/11
to django-d...@googlegroups.com
On Mon, Jun 27, 2011 at 4:24 PM, Jim D. <jim.d...@gmail.com> wrote:
> I spent some time last week and over the weekend nailing down a
> solution for https://code.djangoproject.com/ticket/3615 . This is the
> ticket about allowing forward references when loading data on the
> MySQL InnoDB backend. My patch implements the proposed change
> (disabling foreign key checks when the data is loaded) as well as a
> straightforward SQL SELECT check for integrity after the data is
> loaded, which if I understand it is the missing piece that has
> prevented this ticket from moving forward for the last 4 years...

Interesting approach -- I wouldn't have thought of this! It feels a
bit nasty to me, but it's rather close to how deferred constraints
work behind the scenes anyway. I'd like to see some feedback from
other people who're experienced with MySQL; I don't know enough about
any potential downsides to spot 'em. But I think you've found a way to
cut the knot of this problem, and barring a better solution I'd like
to check this one in.

> Anyhow, the patch should be 100% there so I'd love if someone could
> check it out and either push it along or let me know if any changes
> are required. It should be easy enough for me to address any issues
> while the whole problem is in my head.

I left some comments on the patch on the ticket.

> I'm using django-threadedcomments on a project, which has forward
> references in one of its test fixtures, so I'm reminded of this issue
> every time I run my project tests. I hate test errors! I'm hopeful the
> latest patch is sufficient to get this issue resolved.

Does this patch work for you (I'm assuming so)? A "this works for me
in production" goes a huge way towards me feeling comfortable checking
things in.

Jacob

Jim D.

unread,
Jun 27, 2011, 6:27:43 PM6/27/11
to django-d...@googlegroups.com
On Monday, June 27, 2011 3:08:03 PM UTC-7, Jacob Kaplan-Moss wrote:

I left some comments on the patch on the ticket.


Excellent just saw those. I'll take a look.
 

> I'm using django-threadedcomments on a project, which has forward
> references in one of its test fixtures, so I'm reminded of this issue
> every time I run my project tests. I hate test errors! I'm hopeful the
> latest patch is sufficient to get this issue resolved.

Does this patch work for you (I'm assuming so)? A "this works for me
in production" goes a huge way towards me feeling comfortable checking
things in.

Does it work in production is a hard question for me to answer, if I understood your question properly. In my projects, I really only touch the loaddata command when I'm running tests (I guess loading data via initial_data.json into a production DB sort of counts as running in production). I don't think anything in my proposed changes touches production code with possibly a few minor exceptions. I know that I can run the full Django test suite with MySQL in innodb and I do not get any fixture loading issues related to forward references.

It'd be great if some others could test this patch out as well. In particular, I can't be sure how well ti works on older versions of MySQL. There's no crazy magic at work here but, well, you never know.

If it works for others and we're all in agreement with the philosophical approach of the solution, I think it should be a fairly uncontroversial commit, since it's really primarily a fixture loading issue at its core. 

Jacob Kaplan-Moss

unread,
Jun 27, 2011, 6:37:13 PM6/27/11
to django-d...@googlegroups.com
On Mon, Jun 27, 2011 at 5:27 PM, Jim D. <jim.d...@gmail.com> wrote:
> Does it work in production is a hard question for me to answer, if I
> understood your question properly. In my projects, I really only touch the
> loaddata command when I'm running tests (I guess loading data via
> initial_data.json into a production DB sort of counts as running in
> production). I don't think anything in my proposed changes touches
> production code with possibly a few minor exceptions. I know that I can run
> the full Django test suite with MySQL in innodb and I do not get any fixture
> loading issues related to forward references.

Yeah, sorry -- I wasn't precise enough with my language. I meant
"works with a real-world project" -- since I don't use MySQL, I can't
test this out at all. So I need to rely on other people to tell me
"yeah, works fine."

> It'd be great if some others could test this patch out as well. In
> particular, I can't be sure how well ti works on older versions of MySQL.
> There's no crazy magic at work here but, well, you never know.

Mmhhm, that's a big one. This *is* MySQL we're talking about... :)

> If it works for others and we're all in agreement with the philosophical
> approach of the solution, I think it should be a fairly uncontroversial
> commit, since it's really primarily a fixture loading issue at its core.

If I get a couple-three reports of "worksforme" and if we can pin down
that this works with all the well-supported MySQL versions -- 4.1+,
essentially -- that's enough for me.

Jacob

Michael Blume

unread,
Jun 27, 2011, 7:09:35 PM6/27/11
to django-d...@googlegroups.com
nitpick: I got a few complaints about trailing whitespace when I applied:

/home/mike/sqlpatch.diff:36: trailing whitespace.
    
/home/mike/sqlpatch.diff:42: trailing whitespace.
        
/home/mike/sqlpatch.diff:126: trailing whitespace.
        
/home/mike/sqlpatch.diff:148: trailing whitespace.
    
/home/mike/sqlpatch.diff:212: trailing whitespace.
                                        



--
You received this message because you are subscribed to the Google Groups "Django developers" group.
To post to this group, send email to django-d...@googlegroups.com.
To unsubscribe from this group, send email to django-develop...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.


Michael Blume

unread,
Jun 27, 2011, 7:32:44 PM6/27/11
to django-d...@googlegroups.com
Just reloaded all our fixtures, and this seems to create no regressions with MySQL Server version: 5.0.51a-3ubuntu5.5 (Ubuntu)

Most of our tables are backed by MyISAM, though, so I'm not sure how much this helps.

Russell Keith-Magee

unread,
Jun 27, 2011, 7:52:05 PM6/27/11
to django-d...@googlegroups.com
On Tue, Jun 28, 2011 at 7:32 AM, Michael Blume <mi...@loggly.com> wrote:
> Just reloaded all our fixtures, and this seems to create no regressions with
> MySQL Server version: 5.0.51a-3ubuntu5.5 (Ubuntu)
> Most of our tables are backed by MyISAM, though, so I'm not sure how much
> this helps.

Unfortunately, not much. Your test has validated that the extra code
doesn't break anything under MyISAM, and this is certainly useful.
However, the root problem only exists with InnoDB because of its...
eclectic... implementation of row level constraints. MyISAM doesn't
have constraints, so your test hasn't exercised the part of the code
that needs to be exercised heavily.

Yours,
Russ Magee %-)

Jim Dalton

unread,
Jun 27, 2011, 8:14:37 PM6/27/11
to django-d...@googlegroups.com
On Jun 27, 2011, at 4:52 PM, Russell Keith-Magee wrote:

> Unfortunately, not much. Your test has validated that the extra code
> doesn't break anything under MyISAM, and this is certainly useful.
> However, the root problem only exists with InnoDB because of its...
> eclectic... implementation of row level constraints. MyISAM doesn't
> have constraints, so your test hasn't exercised the part of the code
> that needs to be exercised heavily.

One unintended benefit of the current patch is that it actually will (if I'm not mistaken) check for invalid foreign key references in MyISAM as well, since the check is run for all MySQL implementations (though not for any other backends). So MyISAM would normally silently allow bad foreign keys to be loaded from your fixture data, but with the patch you'll now get an error.

Michael Blume

unread,
Jun 27, 2011, 8:44:49 PM6/27/11
to django-d...@googlegroups.com
Couple questions:

I see a variable saved_objects being written, but I don't see it being accessed -- is this to ease future features, or am I missing a code path?

If I'm reading correctly, check_for_invalid_foreign_keys extends over all the rows in a table. loaddata is called by syncdb and South's migrate, not just when a db is set up, so this could easily wind up run over lots and lots of non-fixture data. I don't know MySQL's performance characteristics that well -- is this likely to be expensive?

Thanks very much for the patch -- managing dependencies in fixtures is a pain, and it'll be nice not to worry about it.

-Mike

Jim Dalton

unread,
Jun 27, 2011, 10:04:09 PM6/27/11
to django-d...@googlegroups.com
On Jun 27, 2011, at 5:44 PM, Michael Blume wrote:

> I see a variable saved_objects being written, but I don't see it being accessed -- is this to ease future features, or am I missing a code path?

Thanks good catch. This was a remnant from an earlier iteration of this patch, in which I tried (unsuccessfully) to call save() on any objects created during the fixture load after foreign key checks had been re-enabled, in hopes that the UPDATE call this generated would trigger a constraint check. It did not, because MySQL ignores UPDATE if the data being set matches what's already in the DB. I have some other changes I'm making in response to some of Jacob's comments on the ticket, so this will be gone in the next version of the patch.

> If I'm reading correctly, check_for_invalid_foreign_keys extends over all the rows in a table. loaddata is called by syncdb and South's migrate, not just when a db is set up, so this could easily wind up run over lots and lots of non-fixture data. I don't know MySQL's performance characteristics that well -- is this likely to be expensive?

This is a good question.

First, though it's true that loaddata is called in several places other than during tests, I would imagine it is rarely if ever called during a normal request/response cycle in production. I'm operating under the assumption that the performance of this patch is important but not as critical as it would be if this was a command used in production.

So that caveat aside, I will try later to find a large data set I can run the query on to get an idea of what kind of performance hit the check entails. If anyone else has a large data set with two related tables, you can try it out as well and report your results. The basic structure of the SQL statement being run is as follows:

SELECT REFERRING.`id`, REFERRING.`id_pointing_to_some_related_table ` FROM `some_table` as REFERRING
LEFT JOIN `some_related_table` as REFERRED
ON (REFERRING.`id_pointing_to_some_related_table` = REFERRED.`id`)
WHERE REFERRING.`id_pointing_to_some_related_table ` IS NOT NULL
AND REFERRED.`id` IS NULL

And keep in mind this is being called for *each* relation in the table. So that'll run through all the rows once for every relation. Of course, it won't return anything unless you have bad data, so the i/o aspect should be minimal.

If we notice an "unacceptable" performance hit the likely solution would be to scope that select statement to only rows that were added during the load data routine. Ideally I'm hoping to steer clear of that, because it entails more work and complexity in loaddata, since you'd have to track all the IDs that were added and then pass them along here. Wouldn't be a big deal but that's why I only wanted to do it if necessary.

Thanks for your input and help testing this btw!

Jim

Jim D.

unread,
Jun 27, 2011, 11:26:08 PM6/27/11
to django-d...@googlegroups.com
On Monday, June 27, 2011 7:04:09 PM UTC-7, Jim D. wrote:

So that caveat aside, I will try later to find a large data set I can run the query on to get an idea of what kind of performance hit the check entails. If anyone else has a large data set with two related tables, you can try it out as well and report your results.

Okay, I just ran this on an InnoDB with 228k rows that's about 8.5MB in size ("REFERRING"), with a related table of 248 rows to join on ("REFERRED"), with no invalid records:

SELECT REFERRING.`id`, REFERRING.`id_pointing_to_some_related_table ` FROM `some_table` as REFERRING
LEFT JOIN `some_related_table` as REFERRED
ON (REFERRING.`id_pointing_to_some_related_table` = REFERRED.`id`)
WHERE REFERRING.`id_pointing_to_some_related_table ` IS NOT NULL
AND REFERRED.`id` IS NULL


I'm getting this consistently under 1 second, around 700 or 800 ms generally. Not great but not horrible. I didn't do any sophisticated profiling or setup for this or anything.

I'm not sure what kind of performance we might think is acceptable, but for the dominant use cases of load data I think it's fine.

My thinking at this point would be the performance is "good enough" for the scope of the current ticket, and that if better performance were required or desired, that could be facilitated under a separate ticket, assuming there was community demand for it.

Karen Tracey

unread,
Jun 28, 2011, 8:27:05 AM6/28/11
to django-d...@googlegroups.com
On Mon, Jun 27, 2011 at 11:26 PM, Jim D. <jim.d...@gmail.com> wrote:
My thinking at this point would be the performance is "good enough" for the scope of the current ticket, and that if better performance were required or desired, that could be facilitated under a separate ticket, assuming there was community demand for it.

I agree. I tried that select with a ~1.5 million row referring table crossing tables with ~150,000 and ~17,000 row tables and they completed in less than a second. I think fixing this issue on this backend is well worth adding possibly several seconds to loaddata time there.

Also, though I don't have MySQL 4 handy to test on, I'd be astonished if there were any issue there compared to MySQL 5. The set foreign_key_check command is certainly supported in MySLQ 4 and the select being issued to do the check is nothing special at all.

I have not had time to try out the patch, but did look at it. Doesn't the base implementation of disable_foreign_key_checks need to return False instead of just passing? The return value is used in loaddata processing to decide whether it's necessary to re-enable/check.

Thanks for working on this -- wish I'd thought of that idea two years ago!

Karen

































Jim Dalton

unread,
Jun 28, 2011, 8:44:50 AM6/28/11
to django-d...@googlegroups.com
On Jun 28, 2011, at 5:27 AM, Karen Tracey wrote:

Also, though I don't have MySQL 4 handy to test on, I'd be astonished if there were any issue there compared to MySQL 5. The set foreign_key_check command is certainly supported in MySLQ 4 and the select being issued to do the check is nothing special at all. 

Agreed. I did find some circumstantial evidence to support the idea that ti would work on MySQL 4 here: http://dev.mysql.com/doc/refman/5.1/en/innodb-foreign-key-constraints.html#c3009 This comment from 2003 (when MySQL 4 was still in beta) described the technique. If it worked in 2003 I have to imagine we're in good shape.

Slightly higher concern is the new get_key_relations() method. Since the information_schema table is not available before MySQL 5 there is a workaround in there for version 4. The contents of this method were originally part of get_indexes() and I just moved them to their own method (that's why you don't see it in the commit). So assuming that function worked fine previously in Django, I don't anticipate this will have a problem either.


I have not had time to try out the patch, but did look at it. Doesn't the base implementation of disable_foreign_key_checks need to return False instead of just passing? The return value is used in loaddata processing to decide whether it's necessary to re-enable/check.

It actually doesn't *need* to return False; pass is the same as not returning anything or returning None. The boolean check just treats it the same way as False. "Should it?" is another question. On the one hand it's a bit more clear, this value is called and always returns False, unless a backend has overridden it. On the other hand, pass is in keeping with other methods in that class that are meant to be overridden in backends, so I went with pass to emphasize that aspect of the code.


Thanks for working on this -- wish I'd thought of that idea two years ago!

You're welcome. It was actually fun to solve a cold case :)

Karen Tracey

unread,
Jun 28, 2011, 9:25:50 AM6/28/11
to django-d...@googlegroups.com
On Tue, Jun 28, 2011 at 8:44 AM, Jim Dalton <jim.d...@gmail.com> wrote:

I have not had time to try out the patch, but did look at it. Doesn't the base implementation of disable_foreign_key_checks need to return False instead of just passing? The return value is used in loaddata processing to decide whether it's necessary to re-enable/check.

It actually doesn't *need* to return False; pass is the same as not returning anything or returning None. The boolean check just treats it the same way as False. "Should it?" is another question. On the one hand it's a bit more clear, this value is called and always returns False, unless a backend has overridden it. On the other hand, pass is in keeping with other methods in that class that are meant to be overridden in backends, so I went with pass to emphasize that aspect of the code.

Hmm, well, I did not know that falling off the end of a method with pass guaranteed a return value of False or None (and can't find that noted in the doc here: http://docs.python.org/tutorial/controlflow.html#pass-statements) so in my mind explicitly returning False would be clearer/better...

Cheers,
Karen



Cal Leeming [Simplicity Media Ltd]

unread,
Jun 28, 2011, 9:29:34 AM6/28/11
to django-d...@googlegroups.com
Sorry for the noobish question but, could someone explain the definition of "forward references"?? Is this a MySQL or a django term?? Google wasn't very forthcoming :X

Thanks

Cal

Jim Dalton

unread,
Jun 28, 2011, 9:38:08 AM6/28/11
to django-d...@googlegroups.com
On Jun 28, 2011, at 6:25 AM, Karen Tracey wrote:

> It actually doesn't *need* to return False; pass is the same as not returning anything or returning None. The boolean check just treats it the same way as False. "Should it?" is another question. On the one hand it's a bit more clear, this value is called and always returns False, unless a backend has overridden it. On the other hand, pass is in keeping with other methods in that class that are meant to be overridden in backends, so I went with pass to emphasize that aspect of the code.
>
> Hmm, well, I did not know that falling off the end of a method with pass guaranteed a return value of False or None (and can't find that noted in the doc here:http://docs.python.org/tutorial/controlflow.html#pass-statements) so in my mind explicitly returning False would be clearer/better...

Ah, okay. To be clear, the return value actually has nothing to do specifically with the use of pass or not. The behavior of Python is to return None from any function that does not explicitly return a value. This is from the exact same page in the Python docs you linked to:

"In fact, even functions without a return statement do return a value, albeit a rather boring one. This value is called None (it’s a built-in name). Writing the value None is normally suppressed by the interpreter if it would be the only value written."

Obviously it's a fairly minor point but we want things to be as clear as possible. Anyone else wants to weigh in on "pass" vs. an explicit return value?

Jim Dalton

unread,
Jun 28, 2011, 9:41:03 AM6/28/11
to django-d...@googlegroups.com
On Jun 28, 2011, at 6:29 AM, Cal Leeming [Simplicity Media Ltd] wrote:

> Sorry for the noobish question but, could someone explain the definition of "forward references"?? Is this a MySQL or a django term?? Google wasn't very forthcoming :X

Jacob actually requested that I add a note in the database docs on the topic this patch addresses. So as a way of answering your question I'll paste that note verbatim here:

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.

Is that note illuminating to you? If not, then presumably it needs work :)

Cal Leeming [Simplicity Media Ltd]

unread,
Jun 28, 2011, 9:44:12 AM6/28/11
to django-d...@googlegroups.com
Ah, that makes perfect sense now. It's the same principle when doing a .sql import, you disable foreign_key_checks, import, then enable.

Thanks for explaining this!

Cal


--

Karen Tracey

unread,
Jun 28, 2011, 10:12:41 AM6/28/11
to django-d...@googlegroups.com
On Tue, Jun 28, 2011 at 9:38 AM, Jim Dalton <jim.d...@gmail.com> wrote:
"In fact, even functions without a return statement do return a value, albeit a rather boring one. This value is called None (it’s a built-in name). Writing the value None is normally suppressed by the interpreter if it would be the only value written."

I guess I did know that at some point, but many many years of writing C code instilled a lot of caution about falling off the end of functions without explicitly returning something. C is not so forgiving as to guarantee a specific value in such cases. Even though Python guarantees it, I think explicit in these cases is better. But that could just be due to my background.

Karen

Andy Dustman

unread,
Jun 28, 2011, 10:35:14 AM6/28/11
to django-d...@googlegroups.com
On Mon, Jun 27, 2011 at 6:08 PM, Jacob Kaplan-Moss <ja...@jacobian.org> wrote:
> On Mon, Jun 27, 2011 at 4:24 PM, Jim D. <jim.d...@gmail.com> wrote:
>> I spent some time last week and over the weekend nailing down a
>> solution for https://code.djangoproject.com/ticket/3615 . This is the
>> ticket about allowing forward references when loading data on the
>> MySQL InnoDB backend. My patch implements the proposed change
>> (disabling foreign key checks when the data is loaded) as well as a
>> straightforward SQL SELECT check for integrity after the data is
>> loaded, which if I understand it is the missing piece that has
>> prevented this ticket from moving forward for the last 4 years...
>
> Interesting approach -- I wouldn't have thought of this! It feels a
> bit nasty to me, but it's rather close to how deferred constraints
> work behind the scenes anyway. I'd like to see some feedback from
> other people who're experienced with MySQL; I don't know enough about
> any potential downsides to spot 'em. But I think you've found a way to
> cut the knot of this problem, and barring a better solution I'd like
> to check this one in.

Disabling foreign key checks is exactly what mysqldump puts at the
start of the dump. Presumably your database will have no bad
references to start with, so the resulting dump shouldn't have any
either, and thus be safe to load with foreign key checks off.

http://dev.mysql.com/doc/refman/5.5/en/innodb-foreign-key-constraints.html
(for reference)
--
Question the answers

akaariai

unread,
Jun 28, 2011, 10:53:13 AM6/28/11
to Django developers
On Jun 28, 12:24 am, "Jim D." <jim.dal...@gmail.com> wrote:
> I spent some time last week and over the weekend nailing down a
> solution forhttps://code.djangoproject.com/ticket/3615. This is the
> ticket about allowing forward references when loading data on the
> MySQL InnoDB backend. My patch implements the proposed change
> (disabling foreign key checks when the data is loaded) as well as a
> straightforward SQL SELECT check for integrity after the data is
> loaded, which if I understand it is the missing piece that has
> prevented this ticket from moving forward for the last 4 years...

This is probably not concurrency-safe if the tables are not locked for
the duration of the fixture loading. I don't know if this will ever be
used in situations where concurrency is an issue. Test fixture loading
is certainly not a problematic use-case.

- Anssi

Marco Paolini

unread,
Jun 28, 2011, 12:07:36 PM6/28/11
to django-d...@googlegroups.com
Karen Tracey ha scritto:
a bit off-topic, but...

if no "return" statement is reached, the function will return None. See
"None" paragraph in [1]

[1]
http://docs.python.org/reference/datamodel.html#the-standard-type-hierarchy

Marco

Jim D.

unread,
Jul 6, 2011, 5:05:33 PM7/6/11
to django-d...@googlegroups.com
Are any core devs interested in taking a closer look at the current patch on this ticket (https://code.djangoproject.com/ticket/3615)? It's been through several rounds of revision after discussion here and on the ticket comment thread.

As of right now the patch should apply cleanly and pass tests in fixtures_regress. It solves the problem of IntegrityError exceptions when loading a fixture in MySQL, and it also applies foreign key constraint checks for both MySQL and SQLite to ensure valid data is being loaded. Last, it offers two hooks for other backends to implement constraint checks after fixtures are loaded.

The issues I still had questions on:

* There is a test error in fixtures_regress.TestFixtures.test_loaddata_raises_error_when_fixture_has_invalid_foreign_key() if a backend does not implement a solution to check for foreign keys. Basically I have MySQL and SQlite covered, but not postgresql or Oracle. As I mention in the ticket,  the work done here https://code.djangoproject.com/ticket/11665 covers Postgresql, so all it needs is an implementation from an Oracle user.

* There's a DB feature can_defer_constraint_checks .  I couldn't find much by way of documentation or or usage of this feature. But I was trying to figure out if the work we are doing here is what this feature refers to, and if so, if we should be marking this as True for MySQL and SQLite with this implementation. I'm not sure there is other behavior that is required or expected in that attribute. Anyhow, that might also be a path forward for the issue I raise above (ie. we could skip the test if can_defer_constraint_checks is not True).

I know there is other very similar work being coordinated elsewhere, so hopefully someone with an an eye on the big picture can help me see this through the last mile. Aside from the minor details above, this should be ready for checkin.

Thanks!

Russell Keith-Magee

unread,
Jul 6, 2011, 7:34:07 PM7/6/11
to django-d...@googlegroups.com
On Thu, Jul 7, 2011 at 5:05 AM, Jim D. <jim.d...@gmail.com> wrote:
> Are any core devs interested in taking a closer look at the current patch on
> this ticket (https://code.djangoproject.com/ticket/3615)? It's been through
> several rounds of revision after discussion here and on the ticket comment
> thread.

Interested - most certainly. The issue (as always) is finding the time :-)

I'm certainly grateful for your efforts; this is a long standing issue
that I'd love to see addressed; I'm just not in a position to commit
to anything. I'll put this on my list of things to look at, but I'm
not getting a whole lot of time to look at that list at the moment.

> * There's a DB feature can_defer_constraint_checks .  I couldn't find much
> by way of documentation or or usage of this feature. But I was trying to
> figure out if the work we are doing here is what this feature refers to, and
> if so, if we should be marking this as True for MySQL and SQLite with this
> implementation. I'm not sure there is other behavior that is required or
> expected in that attribute. Anyhow, that might also be a path forward for
> the issue I raise above (ie. we could skip the test if
> can_defer_constraint_checks is not True).

This feature flag exists essentially to verify whether MySQL InnoDB is
currently in use. It isn't used during normal runtime; it's purely a
test skipping flag. It's used to identify tests that need to be
skipped because the test data requires a circular reference which
(historically) hasn't been possible under InnoDB because constraints
aren't checked.

Essentially, this feature flag shouldn't need to exist at all; it only
exists so that InnoDB passes the test suite without errors. If we are
able to resolve the issue that allows MySQL to use forward references
in data, then this feature flag shouldn't be required at all.

Yours,
Russ Magee %-)

Andy Dustman

unread,
Jul 6, 2011, 8:22:06 PM7/6/11
to django-d...@googlegroups.com

If you aren't using InnoDB, then it probably doesn't matter if you turn foreign key checks on or off: it becomes a no-op. There are some other storage engines that support transactions, and some of them might "do the right thing" with respect to deferred foreign key checks, but I think it does no harm to disable the checks on loading fixtures on these engines. In other words, always turn them off when loading fixtures (and back on afterwards) and don't care about the storage engine and it should be fine. Keep it simple.

Carl Meyer

unread,
Jul 6, 2011, 9:15:34 PM7/6/11
to django-d...@googlegroups.com
Hi Russell and Jim,

This isn't true since the introduction of the new deletion code last
fall. The feature flag is used in db/models/deletion.py to detect
whether it is necessary to null out nullable FKs prior to deletion. On
backends which can defer constraints, this nulling-out would add a
needless extra query. On backends which can't defer, nulling-out means
the nullable FK relationship doesn't dictate a deletion-ordering
constraint, which lets us handle circular FK references as long as at
least one of them is nullable (otherwise we'd have to just throw up our
hands and hope for the best, likely resulting in an integrity error).

Since this use of the flag is unrelated to fixture loading, the
fixture-loading fix should not change the value of the flag for any backend.

Carl

Reply all
Reply to author
Forward
0 new messages