Opened 18 years ago

Closed 14 years ago

Last modified 12 years ago

#2493 closed defect (wontfix)

Error "reset"ing app with self-referencing ForeignKey

Reported by: pfh@… Owned by: nobody
Component: Database layer (models, ORM) Version: dev
Severity: normal Keywords: sprintdec01
Cc: Triage Stage: Design decision needed
Has patch: no Needs documentation: no
Needs tests: yes Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I am trying to define a foreign key relating a model to itself, to create a tree structure, ie

class Category(Model):

parent = ForeignKey("Category")

"python manage.py reset <app>" fails with the error:

ERROR: constraint "parent_id_referencing_event_category_id" does not exist

ALTER TABLE "event_category" DROP CONSTRAINT "parent_id_referencing_event_category_id";

I am using PostgreSQL.

A similar error occurs for some ForeignKey relations, but not others. It seems to happen whenever the SQL for the reset command includes an "ALTER TABLE ... DROP CONSTRAINT" command. This command is sometimes included and sometimes not.

Change History (14)

comment:1 by James Bennett, 18 years ago

Resolution: invalid
Status: newclosed

The code which handles constraint name generation was just refactored in a way which can cause problems trying to reset an application from current trunk when the constraints were generated by an older revision; there's a note on the BackwardsIncompatibleChanges page about it: http://code.djangoproject.com/wiki/BackwardsIncompatibleChanges#Databaseconstraintnameschanged

comment:2 by Malcolm Tredinnick, 18 years ago

Resolution: invalid
Status: closedreopened

The previous comment isn't correct (you can tell because the name of the constraint if the "old" format). However, ...

This one is going to be a little painful to fix, if I'm guessing right. If we don't create the constraint via an "ALTER TABLE" -- that is, if we create it as a "REFERENCES ..." addition on the column definition -- it won't have the name we expect.

We need to do introspection (which is database-specific) to find out the constraint names.

comment:3 by Philippe Raoult, 17 years ago

Needs tests: set

that sounds related to #4193 and #4930. The constraints are randomly not created, so they might not exist which would explain why trying to drop them would fail.

comment:4 by Michael Radziej, 17 years ago

Couldn't we simply use "ALTER TABLE" in both cases?

comment:5 by empty <mtrier@…>, 17 years ago

Owner: changed from nobody to empty
Status: reopenednew

comment:6 by empty <mtrier@…>, 17 years ago

Keywords: sprintdec01 added

comment:7 by empty <mtrier@…>, 17 years ago

Owner: changed from empty to anonymous
Status: newassigned

Confirmed that it's a problem but for the reason mtredinnick stated. The issue is that the constraint name is built up by doing a hash on the two related table names. If the constraint name was changed manually, or otherwise it the drop statement will be invalid. The best solution is to use introspection as mtredinnick suggested.

Some of the odd behavior may also have been related to the #4193 issue, but that's covered there.

comment:8 by empty <mtrier@…>, 17 years ago

Owner: changed from anonymous to empty
Status: assignednew

The issue of the ticket is not really the reason for this comment, but rather the larger issue of understanding the design goals of the sqlreset / reset commands. To me the best approach when constructing the drop statements is to use introspection solely to figure out what needs to be dropped. This includes not only for the foreign key
relationships but also for the tables themselves.

With the current design the table and foreign key information is
pulled together from the model information itself. The problem with
that is that if someone manually changes the name of an foreign key
constraint or removes the constraint entirely then the constructed
information will differ from what is in the database and therefore the
drop statements will fail.

In addition, somewhat related to this, if a user attempts to change
the name of a model and then does a reset it will fail, because the
constraint names and the table name itself will no longer match up to
what is being constructed by the sql_delete function.

If introspection is used entirely for the constructing the drop
statements then it would solve all of the above issues.

comment:9 by empty <mtrier@…>, 17 years ago

Triage Stage: UnreviewedDesign decision needed

Somewhat related to this ticket is the larger issue of understanding the design goals of the sqlreset / reset commands. To me the best approach when constructing the drop statements is to use introspection solely to figure out what needs to be dropped. This includes not only for the foreign key relationships but also for the tables themselves.

With the current design the table and foreign key information is pulled together from the model information itself. The problem with that is that if someone manually changes the name of an foreign key constraint or removes the constraint entirely then the constructed information will differ from what is in the database and therefore the drop statements will fail.

In addition, somewhat related to this, if a user attempts to change the name of a model and then does a reset it will fail, because the constraint names and the table name itself will no longer match up to what is being constructed by the sql_delete function.

If introspection is used entirely for the constructing the drop statements then it would solve all of the above issues.

That said, there are some complex issues to resolve with using introspection. For instance the db_table attribute can be set which will cause the table name to be inconsistent with the standard format so just using the appname as a prefix will not work.

comment:10 by Malcolm Tredinnick, 17 years ago

My concern here is that if introspection is used to construct any "drop table" constructs because it doesn't match what the model tells us, are there cases where a manually altered table will be dropped or somehow corrupted accidentally. We must fail safe here: if there's any doubt, don't drop the table.

If you want to come up with a patch that fixes the issue in this ticket, give it a shot. I'm not sure how hard it will be. However, broad introspection looks like it might be very dangerous, so if you go that far, we'll have to be careful when reviewing it (not saying it won't go in; more noting for myself and other reviewers that we'll need to use our brains at that time).

comment:11 by empty, 17 years ago

Owner: changed from empty to nobody

comment:13 by ingo86, 14 years ago

I suggest another way to do the fix.
In place of changing the create table sql, why don't change the drop table statements?
I use with success the following command:

python manage.py sqlreset myapp | sed 's/DROP TABLE \(.*\);/DROP TABLE \1 CASCADE;/g' | psql --username myusername mydbname

This enable me to add "CASCADE" after each drop table, that makes the trick.

comment:14 by Malcolm Tredinnick, 14 years ago

Resolution: wontfix
Status: newclosed

We are going to deprecate and phase out reset and sqlreset after our experiences with all the fragility of it. Django isn't a good DBA substitute here, unfortunately.

comment:15 by Alper Cugun, 12 years ago

Easy pickings: unset
UI/UX: unset

What's the suggested alternative? A framework that insulates a developer from its database would be preferable.

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