Opened 9 years ago

Closed 8 years ago

#3480 closed (fixed)

sqlreset drops more tables than it should for GenericRelations to separate apps

Reported by: rpuls@… Owned by: nobody
Component: Database layer (models, ORM) Version: master
Severity: Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: no
Easy pickings: UI/UX:

Description

A model from app A has a GenericRelation to another model in app B. When I reset app A, Django outputs DROP TABLE statements for the model from B. Example:

apps/test/models.py:

from django.db import models
from generictestcase.apps.meta.models import Tag

class Article(models.Model):
        tags = models.GenericRelation(Tag)

apps/meta/models.py:

from django.db import models
from django.contrib.contenttypes.models import ContentType

class Tag(models.Model):
        content_type = models.ForeignKey(ContentType)
        object_id = models.PositiveIntegerField()
        object = models.GenericForeignKey()

I use "python manage.py syncdb" to create the database tables (sqlite3 in my case), then "python manage.py sqlreset test" to generate SQL:

BEGIN;
DROP TABLE "meta_tag";
DROP TABLE "test_article";
CREATE TABLE "test_article" (
    "id" integer NOT NULL PRIMARY KEY
);
COMMIT;

Django shouldn't drop the "meta_tag" table here, because it lives in the "meta" app.

Attachments (1)

3480.diff (1.6 KB) - added by jpd <jd@…> 8 years ago.
Patch to check for GenericRel type at deletion (already done at creation)

Download all attachments as: .zip

Change History (8)

comment:1 Changed 9 years ago by rpuls@…

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Resolution set to duplicate
  • Status changed from new to closed

This is a duplicate of #3549 (which is newer, but has a better description).

comment:2 Changed 8 years ago by anonymous

  • Resolution duplicate deleted
  • Status changed from closed to reopened

This is not duplicate of #3549. #3549 says that 'reset' try to delete the same table more that once.

This ticket shows that generic_relation deletes tables from others applications. For example:

I have application 'comments' with model 'Comment' and generic foreign key. I have two other aplications with models 'Article' and 'Event'. They both have generic relation to Comment.

When I reset application 'articles' then table with comments will be droped, but isn't recreated. In that case I lost all comments for events and will have got errors that comments_comment table is not found. To fix that I need too syncdb.

I found that durning creating tables phase there is chechking that m2m table is not instance of GenericRelation - tables from generic realtion aren't created, but durning deleteing tables phase that is not checked and all m2m tables (including generic relations tables are dropped).

comment:3 Changed 8 years ago by anonymous

This is a how it can be fixed.

def get_sql_delete(app):
    ...
    # Output DROP TABLE statements for many-to-many tables.
    for model in app_models:
        opts = model._meta
        for f in opts.many_to_many:
            # check if is not a GenericRel
            if not isinstance(f.rel, generic.GenericRel):
                if cursor and table_name_converter(f.m2m_db_table()) in table_names:
                ...

ps. I know that I should submit a patch. Sorry for that.

Changed 8 years ago by jpd <jd@…>

Patch to check for GenericRel type at deletion (already done at creation)

comment:4 Changed 8 years ago by jpd <jd@…>

  • Has patch set

Added a patch along the lines of what anonymous suggested some months ago. WorksForMe.

This patch does in fact fix issue #3549. - the duplicate DROP statements will not be generated. Probably a more 'correct' fix than what has been suggested on #3549, too.

comment:5 Changed 8 years ago by durdinator

  • Needs tests set
  • Triage Stage changed from Unreviewed to Accepted

The patch looks good to me, definitely better than that from #3549. Could you add a regression test for this bug?

comment:6 Changed 8 years ago by esaj

I just got bitten by this: I reset an app and inadvertently deleted all the comments, even the ones for other apps...

comment:7 Changed 8 years ago by mtredinnick

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

(In [6921]) Generic relations should not try to drop their related table in "sqlreset".

Fixed #3480.

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