Opened 14 years ago

Closed 8 years ago

#13203 closed Bug (fixed)

Deletion error of model with generic relation to inherited model

Reported by: ramusus Owned by: nobody
Component: contrib.contenttypes Version: 1.1
Severity: Normal Keywords: content_type
Cc: ramusus@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I found that if we have 2 simple models connected by GenericRelation:

from django.contrib.comments.models import Comment
class NewComment(Comment):
    title = models.CharField(max_length=60)

class PersonWithComments(models.Model):
    comments = generic.GenericRelation(NewComment, object_id_field='object_pk')

It's impossible to delete PersonWithComments instance. With postgresql_psycopg2 and mysql DB drivers I always get error:

Traceback (most recent call last):
  File "/home/ram/PycharmProjects/m/web_site/generic/tests.py", line 75, in test_inherited_models_delete
    p.delete()
  File "/home/ram/PycharmProjects/m/web_site/django/db/models/base.py", line 610, in delete
    delete_objects(seen_objs, using)
  File "/home/ram/PycharmProjects/m/web_site/django/db/models/query.py", line 1282, in delete_objects
    del_query.delete_batch_related(pk_list, using=using)
  File "/home/ram/PycharmProjects/m/web_site/django/db/models/sql/subqueries.py", line 69, in delete_batch_related
    self.do_query(f.m2m_db_table(), where, using=using)
  File "/home/ram/PycharmProjects/m/web_site/django/db/models/sql/subqueries.py", line 27, in do_query
    self.get_compiler(using).execute_sql(None)
  File "/home/ram/PycharmProjects/m/web_site/django/db/models/sql/compiler.py", line 727, in execute_sql
    cursor.execute(sql, params)
  File "/home/ram/PycharmProjects/m/web_site/django/db/backends/postgresql_psycopg2/base.py", line 44, in execute
    return self.cursor.execute(query, args)
DatabaseError: column "object_pk" does not exist
LINE 1: DELETE FROM "generic_newcomment" WHERE ("object_pk" IN (E'23...

With sqlite3 everithing is ok! I don't know, may be this issue related to http://code.djangoproject.com/ticket/11263, but there tells nothing about deleting.

This error appears because fields 'object_pk' and 'content_type' are not in NewComment, they are in Comment model from Comments app. And there is no checks for these fields inside ContentTypes apps.

I attach tests for recreating this issue and patch for solving it.

Attachments (3)

13203_r12773.tests.diff (1.8 KB ) - added by ramusus 14 years ago.
tests for recreating error
13203_r12773.diff (1.4 KB ) - added by ramusus 14 years ago.
patch for solving issue
13203_r12773_2.diff (1.4 KB ) - added by ramusus 14 years ago.
Update patch with respect to russellm comment (ver 2)

Download all attachments as: .zip

Change History (17)

by ramusus, 14 years ago

Attachment: 13203_r12773.tests.diff added

tests for recreating error

by ramusus, 14 years ago

Attachment: 13203_r12773.diff added

patch for solving issue

comment:1 by Russell Keith-Magee, 14 years ago

Patch needs improvement: set
Triage Stage: UnreviewedAccepted

If I'm understanding the patch correctly, most of the change to m2m_table_name() is to discover the name of the model on which the object_id_field and content_type_field are located. This can be replaced by using a call to get_fields_with_model(). This returns tuples of (field, model) that describe the actual model on which fields can be found without the need to manually traverse the model heirarchy.

by ramusus, 14 years ago

Attachment: 13203_r12773_2.diff added

Update patch with respect to russellm comment (ver 2)

comment:2 by Gabriel Hurley, 13 years ago

Component: Contrib appscontrib.contenttypes

comment:3 by Luke Plant, 13 years ago

Type: Bug

comment:4 by Luke Plant, 13 years ago

Severity: Normal

comment:5 by Aymeric Augustin, 12 years ago

UI/UX: unset

Change UI/UX from NULL to False.

comment:6 by Aymeric Augustin, 12 years ago

Easy pickings: unset

Change Easy pickings from NULL to False.

comment:7 by Ramiro Morales, 11 years ago

See also #12885.

comment:8 by Ramiro Morales, 11 years ago

Updated the proposed fix to current master: https://github.com/ramiro/django/compare/13203

There isn't actually need of running any test code. Just by providing a models design like the one added in the patch is enough to get the syncdb process to the test DB to crash under Postgres because of invalid DDL:

$ ~/django_test/pg84 generic_relations_regress -v2
Python version: 3.3.0 (default, Oct  7 2012, 14:43:21)
[GCC 4.6.3]
Importing application generic_relations_regress
Creating test database for alias 'default' ('test_django_default')...
Creating tables ...
Creating table django_content_type
Creating table auth_permission
Creating table auth_group_permissions
Creating table auth_group
Creating table auth_user_groups
Creating table auth_user_user_permissions
Creating table auth_user
Creating table django_site
Creating table django_flatpage_sites
Creating table django_flatpage
Creating table django_redirect
Creating table django_session
Creating table django_comments
Creating table django_comment_flags
Creating table django_admin_log
Creating table generic_relations_regress_link
Creating table generic_relations_regress_place
Creating table generic_relations_regress_restaurant
Creating table generic_relations_regress_address
Creating table generic_relations_regress_person
Creating table generic_relations_regress_charlink
Creating table generic_relations_regress_textlink
Creating table generic_relations_regress_oddrelation1
Creating table generic_relations_regress_oddrelation2
Creating table generic_relations_regress_note
Creating table generic_relations_regress_contact
Creating table generic_relations_regress_organization_contacts
Creating table generic_relations_regress_organization
Creating table generic_relations_regress_company
Creating table generic_relations_regress_reference
Creating table generic_relations_regress_venue
Installing custom SQL ...
Installing indexes ...
Traceback (most recent call last):
  File "django/upstream/django/db/backends/postgresql_psycopg2/base.py", line 54, in execute
    return self.cursor.execute(query, args)
psycopg2.ProgrammingError: column "id" of relation "generic_relations_regress_reference" does not exist


During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "django/upstream/django/core/management/commands/flush.py", line 62, in handle_noargs
    cursor.execute(sql)
  File "django/upstream/django/db/backends/postgresql_psycopg2/base.py", line 58, in execute
    six.reraise(utils.DatabaseError, utils.DatabaseError(*tuple(e.args)), sys.exc_info()[2])
  File "django/upstream/django/utils/six.py", line 312, in reraise
    raise value.with_traceback(tb)
  File "django/upstream/django/db/backends/postgresql_psycopg2/base.py", line 54, in execute
    return self.cursor.execute(query, args)
django.db.utils.DatabaseError: column "id" of relation "generic_relations_regress_reference" does not exist


During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "./runtests.py", line 323, in <module>
    options.failfast, args)
  File "./runtests.py", line 166, in django_tests
    failures = test_runner.run_tests(test_labels, extra_tests=extra_tests)
  File "django/upstream/django/test/simple.py", line 368, in run_tests
    old_config = self.setup_databases()
  File "django/upstream/django/test/simple.py", line 316, in setup_databases
    self.verbosity, autoclobber=not self.interactive)
  File "django/upstream/django/db/backends/creation.py", line 305, in create_test_db
    database=self.connection.alias)
  File "django/upstream/django/core/management/__init__.py", line 161, in call_command
    return klass.execute(*args, **defaults)
  File "django/upstream/django/core/management/base.py", line 255, in execute
    output = self.handle(*args, **options)
  File "django/upstream/django/core/management/base.py", line 385, in handle
    return self.handle_noargs(**options)
  File "django/upstream/django/core/management/commands/flush.py", line 70, in handle_noargs
    The full error: %s""" % (connection.settings_dict['NAME'], e))
django.core.management.base.CommandError: Database test_django_default 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: column "id" of relation "generic_relations_regress_reference" does not exist

comment:9 by Ramiro Morales, 11 years ago

See also #9501.

comment:10 by Ramiro Morales, 11 years ago

#18297 was closed as duplicate of this.

comment:11 by Tim Graham, 8 years ago

Patch needs improvement: unset

It seems this was fixed in Django 1.6 by 97774429aeb54df4c09895c07cd1b09e70201f7d. I've added a regression test.

comment:12 by Simon Charette, 8 years ago

Triage Stage: AcceptedReady for checkin

The added test covers the reported use cases, LGTM given the tests pass.

comment:13 by Tim Graham <timograham@…>, 8 years ago

In 58c7ff3:

Refs #13203, #9501 -- Added a test for generic relations to child models.

Fixed in 97774429aeb54df4c09895c07cd1b09e70201f7d.

comment:14 by Tim Graham, 8 years ago

Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.
Back to Top