Opened 7 years ago

Closed 6 years ago

Last modified 6 years ago

#21966 closed Bug (needsinfo)

Incorrect usage of constraint_checks_disabled in tests

Reported by: wlodek Owned by: nobody
Component: Uncategorized Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no


If a backend implements enable/disable_constraint_checking (using database SET CONSTRAINTS) some of the tests fail.

The problem comes from the fact that the usage of context manager "constraint_checks_disabled" is based not on the assumptions about how it should work but on the particular implementation for core backends ie. that constraints are checked at transaction commit.

Because enable/disable_constraint_checking for those backends are empty constraint_checks_disabled context manager does nothing and the tests pass while in fact they should fail.

As I understand "assumptions about how it should work" are such that context manager constraint_checks_disabled is for constraints checks something like savepoints for transactions, ie. you can mark a set of statements inside a transaction such the constraints could not be met inside that set but should at the end of set/exit from context manager.

Affected are tests in 'backends', 'serializers_regress', ...

For example in test_disable_constraint_checks_context_manager save inside context manager violates constraints IntegrityError should be raised, it we want to check that it isn't raised we should correct the references before context manager exit.

    with connection.constraint_checks_disabled():
except IntegrityError:"IntegrityError should not have occurred.")

Beside that there is a question why enable/disable_constraint_checking are empty for core backends that could implement that like oracle/postgres? The fact that those backends make use of INITIALY DEFERRED constraints doesn't mean that those must be empty and so test_disable_constraint_checks_context_manager unusable.

Change History (3)

comment:1 Changed 6 years ago by ANUBHAV JOSHI

How can I reproduce this?

comment:2 Changed 6 years ago by Aymeric Augustin

Resolution: needsinfo
Status: newclosed

Indeed, constraint_checks_disabled and related APIs exist only because MySQL is unable to defer constraints checks until transaction commit. See #3615 for details.

I read your description a few times and I don't understand what your proposal actually is. Maybe you could take this to the DevelopersMailingList?

comment:3 Changed 6 years ago by Shai Berger

This actually started on the mailing list.

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