If need be, I don't mind working on my own hack of the delete function
for the admin interface for now. My questions:
Is there are news on how this issue is likely to be handled?
Can anyone tell me where in the django code base to look for the admin
interface delete function so I can override it?
Is there a way to simply override the admins logical behaviour in the
manner that one can override its templates?
Thanks
Iain
Patches would be gratefully reviewed.
> Is there are news on how this issue is likely to be handled?
Not that I'm aware of. Personally, I think we've got bigger fish to
fry, especially since the cascading delete performed by the admin
*should* be a familiar behavior for those who've worked with databases
before.
> Can anyone tell me where in the django code base to look for the admin
> interface delete function so I can override it?
delete_stage and _get_deleted_objects in
django/contrib/admin/views/main.py would be the place to look.
> Is there a way to simply override the admins logical behaviour in the
> manner that one can override its templates?
No.
Now, on to the philosophical aspect of the discussion...
SQL, of course, provides us the option (in theory) of specifying at
the database level what should happen when something which is referred
to by a foreign key in another table is to be deleted or changed in a
way which would invalidate the reference. But actual *support* for
this is, um, varied.
MySQL, when both of the tables involved are using InnoDB, supports the
standard range of options for ON DELETE. The default, so far as I can
tell, is NO ACTION (or RESTRiCT; since InnoDB does not support
deferred checks they are functionally equivalent in MySQL). When using
any other table type, MySQL apparently defaults to deleting the
referenced row while retaining -- intact -- rows which referenced it
(or at least that's what it did when I tried it on a test DB I had
hanging around).
Postgres supports the full range of options all the time and, per its
online manual, defaults to NO ACTION.
SQLite is confusing as hell. Per their online docs, they use 'ON
CONFLICT' as a generic catch-all for any behavior which would violate
a constraint, and the default is 'ABORT', which appears to be
functionally equivalent to NO ACTION. However, the SQLite docs also
state that "FOREIGN KEY constraints are parsed but are not enforced."
I haven't had an opportunity to find out if that means what it seems
to mean (i.e., that SQLite, even when you specify ON CONFLICT ABORT,
would proceed to delete a referenced row on account of not enforcing
the foreign key).
So we've got some options:
1. Have the Django admin app, by default, throw an error and refuse to
delete an object which is referenced by foreign keys on other objects;
even if they don't always support it, this is what all three databases
aspire to even if they don't all manage to achieve that. This would
likely involve a lot of work. Implementing a full range of options in
model definitions for specifying ON DELETE and ON UPDATE behavior, as
I'd half-heartedly suggested in ticket #2288, would likely involve a
metric assload of work. But it'd be awfully nice to have.
2. We could keep things as they are, and document in big flashing
letters that foreign keys in the admin will be subject to cascading
delete. And for people who don't want that behavior and who are using
databases which support this, we could explain how to modify Django's
CREATE TABLE statements (or issue ALTER TABLE statements later on) to
hard-wire NO ACTION or RESTRICT at the database level and do something
with the exception that gets raised.
3. As a compromise between the two, we could implement something like
class MyModel(models.Model):
...
other_model = models.ForeignKey(OtherModel, on_delete='cascade')
and have that change the initial table-creation statements to specify
the appropriate ON DELETE action for Postgres, and force InnoDB with
ON DELETE clause for MySQL; the admin app would then need to recognize
and deal with the exception coming back from the database adapter on a
failed delete. This wouldn't be as much work as the first option and
would provide more flexibility then the second, but would leave SQLite
users out in the cold.
So.
What should we do here?
--
"May the forces of evil become confused on the way to your house."
-- George Carlin
I agree, it makes sense from a database user/designer perspective.
Problem is, it doesn't make sense from an uneducated end user
perspective. I can totally understand why it is the default behaviour,
but there's no way I can have that in my average client admin interface,
and I'd really like to be able to just alter the stock admin rather than
build my own.
> So we've got some options:
>
> 1. Have the Django admin app, by default, throw an error and refuse to
> delete an object which is referenced by foreign keys on other objects;
> even if they don't always support it, this is what all three databases
> aspire to even if they don't all manage to achieve that. This would
> likely involve a lot of work. Implementing a full range of options in
> model definitions for specifying ON DELETE and ON UPDATE behavior, as
> I'd half-heartedly suggested in ticket #2288, would likely involve a
> metric assload of work. But it'd be awfully nice to have.
Agreed, though maybe a long run solution. To me at least, this is an
important issue when making a CMS.
> 2. We could keep things as they are, and document in big flashing
> letters that foreign keys in the admin will be subject to cascading
> delete. And for people who don't want that behavior and who are using
> databases which support this, we could explain how to modify Django's
> CREATE TABLE statements (or issue ALTER TABLE statements later on) to
> hard-wire NO ACTION or RESTRICT at the database level and do something
> with the exception that gets raised.
This seems to me like a "pedal backwards and then correct" approach. I
don't personally like the idea that changing a behaviour that is
un-friendly to a newbie site admin should require such advanced hacking
and database knowledge for a new Django site builder. I could see this
being an issue that could perhaps lower adoption.
> 3. As a compromise between the two, we could implement something like
>
> class MyModel(models.Model):
> ...
> other_model = models.ForeignKey(OtherModel, on_delete='cascade')
>
> and have that change the initial table-creation statements to specify
> the appropriate ON DELETE action for Postgres, and force InnoDB with
> ON DELETE clause for MySQL; the admin app would then need to recognize
> and deal with the exception coming back from the database adapter on a
> failed delete. This wouldn't be as much work as the first option and
> would provide more flexibility then the second, but would leave SQLite
> users out in the cold.
>
> So.
>
> What should we do here?
Throwing out an idea, what if there is a Django settings variable that
allows one to chose the behaviour of a delete, and delete gets recoded
to accomplish this behaviour manually for the various DMBSs. I.E. the
settings might be variants of:
- cascade and kill 'em all ( as is now )
- cascade, reverting to null if possible, killing if the table doesn't
allow converting to null
- as above, but not deleting at all if conversion to null is illegal (
raise exception I suppose )
- etc
Perhaps this is kind of what you're getting at with option 3. I imagine
this would be some work however. I think I shall try out something like
the above on a small scale for my db only, but I'm a ways from having
the python and database knowledge to have a comprehensive solution and
to be making patches that will solve anyone's problems in a hurry. I
would be happy to help with whatever we get though.
Thoughts?
Iain
the cascading delete came to surprise for me, too. I'm quite familiar
with a range of relational databases, and none of them does cascading
by default. They usually forbid it by default. That's why I did not
expect it at all, and of course it happened during a demonstration,
nearly wiping out the test database, which was not what the audience
liked to see ... they'd rather liked to change the default behaviour
to 'restrict'.
But, yes, it is often very useful.
As far as I can see, the cascade happens in the Model.delete()
function, and not really in the admin. Therefore all deletes are
cascading unless you carefully set the foreign keys to Null before
you delete. You cannot change this behaviour with an ALTER TABLE,
since the cascading does not rely on the database.
If you allow Null, this is in my opinion a strong indication that
deleting the other end should not cascade but set Null.
I'm not totally sure myself, but what do the others think about this
is a general rule, i.e. cascading only for keys that do not allow
NULL, else setting the foreign key field to NULL?
Michael
Whatever the default ends up being, whether it stays cascading or
moves to restricting, assuming that allowing NULL on a foreign key
means we should set it NULL when the referenced object goes away seems
to be both dangerous and a violation of "explicit is better than
implicit".
Makes sense to me.
> I'm not totally sure myself, but what do the others think about this
> is a general rule, i.e. cascading only for keys that do not allow
> NULL, else setting the foreign key field to NULL?
That's what I'm after. But I see no reason why we could accomodate more
options! =)
Iain
Valid point!
Michael
For the admin interface, probably your best option is to override the
delete view. This is very simple, actually, because all actions are
directed by the url and can be overridden by urls.py.
So, in your urls.py file, above include('admin/', ...), type in
something like:
(r'(\w+)/(\w+)/(\d+)/delete/$', 'my.delete.view'),
In that view, you can unset the relationship and delete the object in
question, using the project, model and object id args. If you just need
it for one particular model, then hard-code the project and models and
the rest will fall through to the included admin actions.
-rob
Thanks for the tips Rob, that sounds like the best bet for me in the
meantime.
Iain
Anyone have tips on how to add an icon to the admin page to call a
custom admin callback that way? And to overide the existing icon? Then I
could have "delete all related" and "delete, set realted to null" icons.
Iain
-rob