Code

Opened 2 months ago

Closed 4 weeks 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

Description

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.

try:
    with connection.constraint_checks_disabled():
        a.save()
        a.reporter_id= self.r.id
        a.save()
except IntegrityError:
    self.fail("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.

Attachments (0)

Change History (2)

comment:1 Changed 6 weeks ago by anubhav9042

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

How can I reproduce this?

comment:2 Changed 4 weeks ago by aaugustin

  • Resolution set to needsinfo
  • Status changed from new to closed

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?

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.