Code

Opened 6 years ago

Closed 6 years ago

#6820 closed (fixed)

Error in django/core/management/commands/flush.py prevents runtests.py from running successfully under PostgreSQL 8.3

Reported by: tpherndon Owned by: jacob
Component: Core (Other) Version: master
Severity: Keywords:
Cc: russellm, john@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: UI/UX:

Description

With qs-rf 7297, I attempt to run runtests.py using PostgreSQL 8.3, and the tests fail with the following message:

Doctest: modeltests.field_subclassing.models.__test__.API_TESTS ... ok
Error: Database test_synapse couldn't be flushed. Possible reasons:
      * The database isn't running or isn't configured correctly.
      * At least one of the expected database tables doesn't exist.
      * The SQL was invalid.
    Hint: Look at the output of 'django-admin.py sqlflush'. That's the SQL this command wasn't able to run.
    The full error: cannot TRUNCATE "generic_relations_taggeditem" because it is being used by active queries in this session

The error is caused by the modeltests.fixtures test, and happens because there are non-core-django tables that are related to django-core tables that are being flushed by the command:

# Reset the database representation of this app.
# This will return the database to a clean initial state.
>>> management.call_command('flush', verbosity=0, interactive=False)

Since there are relationships between these tables and the django-core tables, the flush of the django-core tables fails, causing the test run to crash. The attached patch changes the sql_flush to affect more than the only_django tables, which allows for completion of runtests.py on PostgreSQL 8.3 with one error:

======================================================================
ERROR: Shortcuts for an object that has with a get_absolute_url method raises 404
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/herndonp/Downloads/django-qsrf/tests/regressiontests/views/tests/defaults.py", line 23, in test_shortcut_no_absolute_url
    for obj in Article.objects.all():
  File "/Library/Python/2.5/site-packages/django/db/models/query.py", line 66, in _result_iter
    self._fill_cache()
  File "/Library/Python/2.5/site-packages/django/db/models/query.py", line 455, in _fill_cache
    self._result_cache.append(self._iter.next())
  File "/Library/Python/2.5/site-packages/django/db/models/query.py", line 147, in iterator
    for row in self.query.results_iter():
  File "/Library/Python/2.5/site-packages/django/db/models/sql/query.py", line 168, in results_iter
    for rows in self.execute_sql(MULTI):
  File "/Library/Python/2.5/site-packages/django/db/models/sql/query.py", line 1348, in <lambda>
    return iter((lambda: cursor.fetchmany(GET_ITERATOR_CHUNK_SIZE)),
InterfaceError: cursor already closed

----------------------------------------------------------------------
Ran 240 tests in 724.716s

FAILED (errors=1)
Destroying test database...

The change allows run-to-completion with no errors on sqlite and mysql.

Attachments (3)

flush.patch (521 bytes) - added by tpherndon 6 years ago.
Patch for django/core/management/commands/flush.py to allow completion of runtests.py on PostgreSQL
flush.diff (535 bytes) - added by john_scott 6 years ago.
rollback.diff (470 bytes) - added by dougvanhorn 6 years ago.
Adds a rollback just before running the flush.

Download all attachments as: .zip

Change History (15)

Changed 6 years ago by tpherndon

Patch for django/core/management/commands/flush.py to allow completion of runtests.py on PostgreSQL

comment:1 Changed 6 years ago by mtredinnick

  • Cc russellm added
  • Keywords qs-rf removed
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

This isn't queryset-refactor related, since the same code exists in trunk.

Russell: this looks like the right solution and the logic makes sense. Any unintended side-effects you can think of (i.e. any reason we aren't already doing this)?

comment:2 Changed 6 years ago by russellm

  • Patch needs improvement set

The only_django option was added in r6013 to fix #5086. This ticket described problems that occurred when a Postgres database had tsearch2 tables installed. I'm guessing similar problems could occur if the database contained user-defined tables that were not of Django origin.

I'm inclined to agree with Adrian's analysis on ticket #5086 - that Django flush shouldn't touch/flush any table that it didn't create itself. However, this obviously causes difficulties with Postgres 8.3. I haven't had a chance to play with Postgres 8.3 yet - can anyone shed any light on what has changed with the internal table structure that is causing flush to fail? I suspect the right solution is to fix this specific failure, rather than rolling back the use of only_django in the flush command.

comment:3 Changed 6 years ago by mtredinnick

Oh, sorry.. I was lazy and didn't check the line annotations to see if it related to a ticket. You're right. I agree with the reasoning in #5086, too.

comment:4 Changed 6 years ago by tpherndon

I can tell you what is causing flush to fail, though not what has changed or when. django-admin.py sqlflush returns the following sql:

BEGIN;
TRUNCATE "auth_message", "auth_group", "auth_group_permissions", "auth_user", "auth_user_groups", "auth_user_user_permissions", "auth_permission", "django_content_type", "django_session", "django_site", "django_flatpage", "django_flatpage_sites", "django_admin_log";
SELECT setval('"auth_message_id_seq"', 1, false);
SELECT setval('"auth_group_id_seq"', 1, false);
SELECT setval('"auth_group_permissions_id_seq"', 1, false);
SELECT setval('"auth_user_id_seq"', 1, false);
SELECT setval('"auth_user_groups_id_seq"', 1, false);
SELECT setval('"auth_user_user_permissions_id_seq"', 1, false);
SELECT setval('"auth_permission_id_seq"', 1, false);
SELECT setval('"django_content_type_id_seq"', 1, false);
SELECT setval('"django_site_id_seq"', 1, false);
SELECT setval('"django_flatpage_id_seq"', 1, false);
SELECT setval('"django_flatpage_sites_id_seq"', 1, false);
SELECT setval('"django_admin_log_id_seq"', 1, false);
COMMIT;

and the output from running the above directly gives the following error message:

ERROR: cannot truncate a table referenced in a foreign key constraint
SQL state: 0A000
Detail: Table "comments_comment" references "auth_user".
Hint: Truncate table "comments_comment" at the same time, or use TRUNCATE ... CASCADE.

It looks to me like the only_django option is causing the flush command to skip over the models from django.contrib.comments. The interesting part here is that, while runtests.py sets settings.INSTALLED_APPS to ALWAYS_INSTALLED_APPS temporarily, I do *not* have django.contrib.comments in my normal INSTALLED_APPS. So, I modify my INSTALLED_APPS to include comments, and rerun the tests. The tests fail at the same point, but this time sqlflush tells a different story:

BEGIN;
TRUNCATE "auth_message", "auth_group", "auth_group_permissions", "auth_user", "auth_user_groups", "auth_user_user_permissions", "auth_permission", "django_content_type", "django_session", "django_site", "django_flatpage", "django_flatpage_sites", "django_admin_log", "comments_comment", "comments_freecomment", "comments_karmascore", "comments_moderatordeletion", "comments_userflag";
SELECT setval('"auth_message_id_seq"', 1, false);
SELECT setval('"auth_group_id_seq"', 1, false);
SELECT setval('"auth_group_permissions_id_seq"', 1, false);
SELECT setval('"auth_user_id_seq"', 1, false);
SELECT setval('"auth_user_groups_id_seq"', 1, false);
SELECT setval('"auth_user_user_permissions_id_seq"', 1, false);
SELECT setval('"auth_permission_id_seq"', 1, false);
SELECT setval('"django_content_type_id_seq"', 1, false);
SELECT setval('"django_site_id_seq"', 1, false);
SELECT setval('"django_flatpage_id_seq"', 1, false);
SELECT setval('"django_flatpage_sites_id_seq"', 1, false);
SELECT setval('"django_admin_log_id_seq"', 1, false);
SELECT setval('"comments_comment_id_seq"', 1, false);
SELECT setval('"comments_freecomment_id_seq"', 1, false);
SELECT setval('"comments_karmascore_id_seq"', 1, false);
SELECT setval('"comments_moderatordeletion_id_seq"', 1, false);
SELECT setval('"comments_userflag_id_seq"', 1, false);
COMMIT;

And when I run the above directly, I get:

ERROR: cannot truncate a table referenced in a foreign key constraint
SQL state: 0A000
Detail: Table "fixtures_regress_stuff" references "auth_user".
Hint: Truncate table "fixtures_regress_stuff" at the same time, or use TRUNCATE ... CASCADE.

The obvious first problem is that the only_django option is picking up its app list from my INSTALLED_APPS, and not from runtests.py's ALWAYS_INSTALLED_APPS. The second problem I see is that the fixtures test, in calling the flush command, also needs to flush at least its own tables. In both cases, it looks to me like the actual flush command is working as designed, and that the table-twiddling in runtests.py is causing the problem. Why the problem is surfacing under PostgreSQL 8.3, when it hasn't been an issue under 8.2.5, now that's an interesting question. The problem does not exist under sqlite3, nor under MySQL 5.0.45 using either MyISAM or InnoDB tables.

comment:5 Changed 6 years ago by mtredinnick

That's excellent information to have. Thankyou so much for taking the time to collect this. Should be helpful and it's really appreciated. Thanks. :-)

comment:6 Changed 6 years ago by tpherndon

My pleasure, Malcolm. Unfortunately, I spoke too soon. While the tests get past modeltests.fixtures, there are a number of foreign key problems further down, using MySQL 5.0.45 InnoDB. A sample:

IntegrityError: (1452, 'Cannot add or update a child row: a foreign key constraint fails (`test_biblio/views_article`, CONSTRAINT `author_id_refs_id_559385ef` FOREIGN KEY (`author_id`) REFERENCES `views_author` (`id`))')

In both this and my earlier comment on 3/26, these tests are being run against trunk 7363.

comment:7 Changed 6 years ago by anonymous

  • Cc john@… added

Adding myself to CC.

comment:8 Changed 6 years ago by john_scott

Just for clarity, I don't think that the above error doesn't have anything to do with running tests.

I was experiencing the same error running ./manage.py flush on a fresh db.

I noticed that in the TRUNCATE sql that the offending table was listed three times. I cut out the two extraneous table references and ran the raw SQL and then PostgreSQL 8.3 was a happy camper.

You can skip Django and trigger the error message directly in psql: TRUNCATE auth_user, auth_user;

Not sure why PostgreSQL is only now crying about it.

django_table_list in django.core.management.sql adds duplicate table names when it grabs the table names in M2M relationships.

See attached flush.diff for a simple way to make the tables list unique.

Changed 6 years ago by john_scott

comment:9 Changed 6 years ago by dougvanhorn

I was running into the same error as described by this bug (Error: Database test_synapse couldn't be flushed. Possible reasons:) while running against Postgres 8.3 and psycopg2 2.0.6.

However, running the output of sqlflush directly against the leftover test database did *not* yield any errors. After a bit of poking around in management.commands.flush I noticed that connection.connection.get_transaction_status() was in error just before running the 'flush' sql (due to a failing test).

I added a line to roll back the transaction (transaction.rollback()) just before executing the 'flush' sql and everything now works fine.

Maybe this is a different problem not related to those described above? I noticed that the Exception being caught was not the same as that generated by running the bad/incomplete TRUNCATE sql directly in a shell.

I'm adding a patch to show what I did.

Changed 6 years ago by dougvanhorn

Adds a rollback just before running the flush.

comment:10 Changed 6 years ago by john@…

I've been seeing this problem with Postgres 8.2.3, with psycopg 2.0.2.

dougvanhorn's rollback.diff patch seems to have solved the problem for now.

comment:11 Changed 6 years ago by jacob

  • Owner changed from tpherndon to jacob
  • Status changed from new to assigned

OK, done some work on this -- the real problem is that django.core.management.sql.django_table_list returns duplicate table names under certain cicumstances (as noted above. I think I'm just gonna fix it by making that into a set; the order is completely arbitrary, anyway, and set containment tests will be faster than anything we could jury-rig with lists.

comment:12 Changed 6 years ago by jacob

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

(In [7568]) Fixed #6820: flush no longer fails under PostgreSQL 8.3. WARNING: In the process I renamed a couple of internal functions in django.core.management.sql, so this is a backwards-incompatible change if you were using those internal functions.

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.