Code

Opened 5 years ago

Closed 3 years ago

Last modified 3 years ago

#11101 closed (fixed)

Oracle fails admin_view tests when the whole suite is run -- transaction problem?

Reported by: mboersma Owned by: mboersma
Component: Database layer (models, ORM) Version: master
Severity: Keywords: oracle transaction test
Cc: ikelly, andrewsk Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

If you run either the "auth" or "admin_view" tests separately, they pass the Oracle backend. But if you run them together, the second one causes an insert error. Oracle thinks a constraint was violated because we're trying to INSERT another row with the username "super". But the previous INSERT should have been rolled back by the test runner, is my understanding.

C:\projects\django-trunk\tests>runtests.py --settings=testsettings.oracle auth admin_views
======================================================================
FAIL: Doctest: django.contrib.auth.tests.__test__.BASIC_TESTS
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\projects\django-trunk\django\test\_doctest.py", line 2180, in runTest
    raise self.failureException(self.format_failure(new.getvalue()))
AssertionError: Failed doctest test for django.contrib.auth.tests.__test__.BASIC_TESTS
  File "C:\projects\django-trunk\django\contrib\auth\tests\__init__.py", line unknown line number, in BASIC_TESTS

----------------------------------------------------------------------
File "C:\projects\django-trunk\django\contrib\auth\tests\__init__.py", line ?, in django.contrib.auth.tests.__test__.BASIC_TESTS
Failed example:
    super = User.objects.create_superuser('super', 'super@example.com', 'super')
Exception raised:
    Traceback (most recent call last):
      File "C:\projects\django-trunk\django\test\_doctest.py", line 1267, in __run
        compileflags, 1) in test.globs
      File "<doctest django.contrib.auth.tests.__test__.BASIC_TESTS[22]>", line 1, in <module>
        super = User.objects.create_superuser('super', 'super@example.com', 'super')
      File "C:\projects\django-trunk\django\contrib\auth\models.py", line 109, in create_superuser
        u = self.create_user(username, email, password)
      File "C:\projects\django-trunk\django\contrib\auth\models.py", line 105, in create_user
        user.save()
      File "C:\projects\django-trunk\django\db\models\base.py", line 410, in save
        self.save_base(force_insert=force_insert, force_update=force_update)
      File "C:\projects\django-trunk\django\db\models\base.py", line 486, in save_base
        result = manager._insert(values, return_id=update_pk)
      File "C:\projects\django-trunk\django\db\models\manager.py", line 177, in _insert
        return insert_query(self.model, values, **kwargs)
      File "C:\projects\django-trunk\django\db\models\query.py", line 1055, in insert_query
        return query.execute_sql(return_id)
      File "C:\projects\django-trunk\django\db\models\sql\subqueries.py", line 320, in execute_sql
        cursor = super(InsertQuery, self).execute_sql(None)
      File "C:\projects\django-trunk\django\db\models\sql\query.py", line 2359, in execute_sql
        cursor.execute(sql, params)
      File "C:\projects\django-trunk\django\db\backends\oracle\base.py", line 433, in execute
        raise e
    IntegrityError: ORA-00001: unique constraint (TEST_MBOERSMA.SYS_C00152043) violated

----------------------------------------------------------------------
File "C:\projects\django-trunk\django\contrib\auth\tests\__init__.py", line ?, in django.contrib.auth.tests.__test__.BASIC_TESTS
Failed example:
    super.is_superuser
Exception raised:
    Traceback (most recent call last):
      File "C:\projects\django-trunk\django\test\_doctest.py", line 1267, in __run
        compileflags, 1) in test.globs
      File "<doctest django.contrib.auth.tests.__test__.BASIC_TESTS[23]>", line 1, in <module>
        super.is_superuser
    AttributeError: type object 'super' has no attribute 'is_superuser'
----------------------------------------------------------------------
File "C:\projects\django-trunk\django\contrib\auth\tests\__init__.py", line ?, in django.contrib.auth.tests.__test__.BASIC_TESTS
Failed example:
    super.is_active
Exception raised:
    Traceback (most recent call last):
      File "C:\projects\django-trunk\django\test\_doctest.py", line 1267, in __run
        compileflags, 1) in test.globs
      File "<doctest django.contrib.auth.tests.__test__.BASIC_TESTS[24]>", line 1, in <module>
        super.is_active
    AttributeError: type object 'super' has no attribute 'is_active'
----------------------------------------------------------------------
File "C:\projects\django-trunk\django\contrib\auth\tests\__init__.py", line ?, in django.contrib.auth.tests.__test__.BASIC_TESTS
Failed example:
    super.is_staff
Exception raised:
    Traceback (most recent call last):
      File "C:\projects\django-trunk\django\test\_doctest.py", line 1267, in __run
        compileflags, 1) in test.globs
      File "<doctest django.contrib.auth.tests.__test__.BASIC_TESTS[25]>", line 1, in <module>
        super.is_staff
    AttributeError: type object 'super' has no attribute 'is_staff'


----------------------------------------------------------------------
Ran 102 tests in 30.906s

FAILED (failures=1)

A simple workaround is to change the username in auth to "super1" or "superduper" or somesuch. But let's not do that until we know what's really going on here.

Attachments (1)

11101.diff (1.2 KB) - added by ikelly 3 years ago.

Download all attachments as: .zip

Change History (15)

comment:1 Changed 5 years ago by kmtracey

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

This is bizarre. rollback doesn't appear to be working for Oracle...or a commit is sneaking in somehow? When I add some prints to the test code that does the rollback and in the connection _rollback code I see this for Oracle:

test_password_change_fails_with_invalid_old_password (django.contrib.auth.tests.views.ChangePasswordTest) ... ok
In _fixture_teardown before rollback: 3 User objects.
_rollback calling rollback() on <cx_Oracle.Connection to test_system@XE>
Exiting _fixture_teardown after rollback: 3 User objects.

whereas on sqlite, for example, you can see the rollback deletes the User objects that were added as part of the admin_views fixtures used by the test:

test_password_change_fails_with_invalid_old_password (django.contrib.auth.tests.views.ChangePasswordTest) ... ok
In _fixture_teardown before rollback: 3 User objects.
_rollback calling rollback() on <sqlite3.Connection object at 0x011CBCA0>
Exiting _fixture_teardown after rollback: 0 User objects.

I'm confused.

comment:2 Changed 5 years ago by kmtracey

Adding a print to commit doesn't reveal that it's being called after the fixtures are loaded for the test, though I can see it's called earlier (as expected):

Trying absolute path for initial_data.json.bz2 fixture 'initial_data'...
No json fixture 'initial_data' in absolute path.
_commit calling commit() on <cx_Oracle.Connection to test_system@XE>
No fixtures found.
_fixture_setup about to load fixtures: 0 User objects.
test_password_change_fails_with_invalid_old_password (django.contrib.auth.tests.views.ChangePasswordTest) ... ok
In _fixture_teardown before rollback: 3 User objects.
_rollback calling rollback() on <cx_Oracle.Connection to test_system@XE>
Exiting _fixture_teardown after rollback: 3 User objects.

comment:3 follow-up: Changed 5 years ago by mboersma

Ian and I were just looking at this and think we may have the problem.

When the admin_views tests run, we load several XML fixtures. This has the effect of resetting the Oracle sequences to match the new high-water-mark PK:

DECLARE
    startvalue integer;
    cval integer;
BEGIN
    LOCK TABLE "AUTH_USER" IN SHARE MODE;
    SELECT NVL(MAX("ID"), 0) INTO startvalue FROM "AUTH_USER";
    SELECT "AUTH_USER_SQ".nextval INTO cval FROM dual;
    cval := startvalue - cval;
    IF cval != 0 THEN
        EXECUTE IMMEDIATE 'ALTER SEQUENCE "AUTH_USER_SQ" MINVALUE 0 INCREMENT BY '||cval;
        SELECT "AUTH_USER_SQ".nextval INTO cval FROM dual;
        EXECUTE IMMEDIATE 'ALTER SEQUENCE "AUTH_USER_SQ" INCREMENT BY 1';
    END IF;
    COMMIT;
END;

Even if our PL/SQL code here didn't contain an explicit COMMIT, we think the EXECUTE IMMEDIATE would probably close the previous transaction. So we've inadvertently committed a row that was supposed to be rolled back at the end of the tests. Then a later test tries to insert a row that collides.

Not sure how to fix it yet, but it seems likely this is what's going on. Also, we think it's highly unlikely to crop up outside of the Django test suite itself.

comment:4 Changed 5 years ago by Alex

  • Triage Stage changed from Unreviewed to Accepted

comment:5 in reply to: ↑ 3 Changed 4 years ago by jtiai

Replying to mboersma:

Ian and I were just looking at this and think we may have the problem.

When the admin_views tests run, we load several XML fixtures. This has the effect of resetting the Oracle sequences to match the new high-water-mark PK:

Even if our PL/SQL code here didn't contain an explicit COMMIT, we think the EXECUTE IMMEDIATE would probably close the previous transaction. So we've inadvertently committed a row that was supposed to be rolled back at the end of the tests. Then a later test tries to insert a row that collides.

It's not that EXECUTE IMMEDIATE but "ALTER SEQUENCE" that causes implicit commit before DDL is executed. EXECUTE IMMEDIATE (as being just PL/SQL construct for dynamic SQL execution) doesn't itself cause any implicit commit/rollbacks.

comment:6 Changed 4 years ago by russellm

This also appears to affect the multi-db branch. There are three failures in the multiple_databases.QueryTestCase when this TestCase is run after the multiple_databases.FixtureTestCase.

comment:7 Changed 3 years ago by ramiro

Our move from doctests to unit tests for the Django test has exacerbated this problem and it can be seen even in the most basic tests that use fixtures, e.g. modeltests.fixtures.FixtureLoadingTests.{test_compress_format_loading,test_compressed_loading} in that order. The second test method doesn't get a clean DB state, when it starts the DB contains an Article record loaded from the initial_data fixture (expected) AND the record loaded by the loaddata management call in the previous test method (unexpected).

Failure rate of our suite under Oracle is currently very high because this basic unit test assumption is broken.

AFAICS, this issue can affect any app test case that uses fixtures, not only the Django test suite.

Changed 3 years ago by ikelly

comment:8 Changed 3 years ago by ikelly

I've attached a patch that appears to fix the problem, although I haven't yet run the entire test suite to verify. The patch drops the ALTER SEQUENCE statements from the reset PL/SQL and instead just spins the sequence until the sequence value is large enough to avoid a conflict with an existing row. This way the current transaction is unaffected, at the cost of reduced efficiency and that the reset code will no longer ever decrease the sequence value. The latter is arguably a good thing anyway.

comment:9 Changed 3 years ago by ikelly

  • Has patch set

comment:10 Changed 3 years ago by andrewsk

  • Cc andrewsk added

comment:11 follow-up: Changed 3 years ago by ikelly

After going through the test suite, here's what I've found:

The views, admin_views, and model_formsets test suites are still getting a single 'unique constraint violated' error each. I don't know why yet, and it may be unrelated, but at least that's a heck of a lot better than before.

The backends test suite is getting a couple of errors that look like this:

DatabaseError: ORA-02091: transaction rolled back
ORA-02291: integrity constraint (DJANGO_TEST_DEFAULT.SYS_C001650670) violated - parent key not found

And the fixtures_regress test suite is getting a failure that is caused by the patch:

======================================================================
FAIL: test_duplicate_pk (regressiontests.fixtures_regress.tests.TestFixtures)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/ikelly/projects/django.trunk/tests/regressiontests/fixtures_regress/tests.py", line 60, in test_duplicate_pk
    self.assertEqual(animal.id, 2)
AssertionError: 12L != 2

This happens because the sequence value no longer gets decreased when the sequence is reset. That's not really what the test is meant to be testing, however, and I would suggest that it perhaps should be replaced with assertGreaterEqual.

There are a handful of other failures that don't appear to be related to this issue.

comment:12 in reply to: ↑ 11 Changed 3 years ago by ramiro

Replying to ikelly:

After going through the test suite, here's what I've found:

The views, admin_views, and model_formsets test suites are still getting a single 'unique constraint violated' error each. I don't know why yet, and it may be unrelated, but at least that's a heck of a lot better than before.

The backends test suite is getting a couple of errors that look like this:

DatabaseError: ORA-02091: transaction rolled back
ORA-02291: integrity constraint (DJANGO_TEST_DEFAULT.SYS_C001650670) violated - parent key not found

This one isn't caused by the fix you implemented so it can be included with the last group you detailed (There are a handful of other failures that don't appear to be related to this issue. ). r14320 needed to be ported to Oracle (I started playing with it because I wanted to be sure I could test things). I committed a fix for it in r14510.

comment:13 Changed 3 years ago by ikelly

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

(In [14537]) Fixed #11101: Rewrote the sequence reset SQL for Oracle to prevent it from performing an implicit commit that caused all fixtures to be automatically committed, causing a large number of test failures.

comment:14 Changed 3 years ago by ikelly

(In [14538]) [1.2.X] Backport of r14537 from trunk.

Fixed #11101: Rewrote the sequence reset SQL for Oracle to prevent it from performing an implicit commit that caused all fixtures to be automatically committed, causing a large number of test failures.

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.