Defered constraint checks flush after `post_delete` signal

[Alex Madjar]

Repro steps:

1. Use a database that supports can_defer_constraint_checks (tested on postgres)
2. Have a model (A) with a null=True ForeignKey or OneToOneField (F) to another model (B)
3. Connect a function to listen to post_delete on (B) that pulls objects of type (A) from the db (using a = A.objects.get... etc )
4. Try to access a.F for an object a that used to be associated with the deleted instance of B that was just deleted

Expectation: a.F_id is None

Actually: a.F_id is the id of the deleted instance and a.F raises an ObjectDoesNotExist exception

Note: doesn't repro on fields where null=False or on sqlite. My diagnosis is that A's field isn't being properly cleared (or A objects deleted in the case of on delete CASCADE) because the constraint checks aren't being flushed until _after_ the post_delete signal fires

[Alex Madjar]

Only tried on CASCADE deletes, I think it blames to this code - ​https://github.com/django/django/blob/master/django/db/models/deletion.py#L15

[Tim Graham]

If you could provide a test case for Django's test suite, that would be helpful.

[Tim Graham]

sample app to reproduce

[Tim Graham]

Seems legitimate. I attached a sample project I used to reproduce. The first test method shows the failure.

[Can Sarıgöl]

​PR

[Annabelle Wiegart]

I'm unable to reproduce the described behavior with the attached sample project. The related Choice instance seems to be deleted immediately. The post_delete callback function produces the error t26630.models.Choice.DoesNotExist: Choice matching query does not exist. Is there still a bug that needs to be fixed?

[Jacob Walls]

Thanks, Annabelle, I'm seeing the same. However, I think this is a symptom of the test project needing to be updated. If you connect a post_delete signal on Choice as well, you can undo the effect of the optimization from #30856 and trigger the failing code path again.

​Here is a fiddle on DryORM

I'm not sure exactly how to fix this, but we should probably examine the solution space a bit more before closing this one.

If you're curious how I dug around for this, I noticed the original report suggesting that one of the objects was not being cascade deleted, but as you saw, it was. So I set a breakpoint and noticed we were hitting a "fast delete" path, which led me to #30856 driving more traffic into that path.

[Annabelle Wiegart]

Thank you for your clarifications, Jacob. i looked at the DryORM example and a bit into the Collector class and the "fast delete" path.

I'm a bit unsure about the expected behavior though. The ticket description says "Expectation: a.F_id is None". Since the "fast delete" path leads to the deletion of a not being deferred, I suppose the expected behavior would be exactly this, even if a post_delete signal is attached to Choice as well?

[Jacob Walls]

I'm also a little unsure. I think the gist of the report is that doing database queries during a post_delete listener is problematic, because you can query Choice rows by question_id and get misleading results (as those rows are in the middle of being cascade deleted).

I'm reading the original report to suggest reordering the post_delete signal to run after constraints are flushed (and all cascade deletes have happened). I doubt that would be backward compatible.

In #30835 something similar was suggested, and Simon explained you can use on_commit() to defer execution of the callback (to a point when all cascade deletions have been made). I think I'd suggest rewriting the signal handler that way to avoid this problem with unstable Choice results.

I'm inclined to close this as duplicate of #30835. As mentioned there, if there's a proposal for a docs tweak in post_delete we could consider it. (In that case, do reopen #30835!)

Thanks for helping out with triage.