Opened 6 years ago

Closed 5 years ago

#10829 closed (duplicate)

QuerySet.delete() attempts to delete unmanaged models.

Reported by: justinlilly Owned by: nobody
Component: Database layer (models, ORM) Version: 1.1-beta
Severity: Keywords: unmanaged
Cc: Triage Stage: Design decision needed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

When there is an unmanaged model which has a ForeignKey to an object that you're attempting to delete, the ORM attempts to delete it. In my case, this is a database view which doesn't support deletion.

I'll update this with a bit more information and a method for reproducing the error later today, just want to get it on the radar.

I'm also assigning this to the 1.1 milestone as not being able to delete an object associated with a managed model is kind of a big deal. Feel free to override me on it though.

Attachments (1)

django.db.models.query.py.patch (2.0 KB) - added by philroche 6 years ago.
Patch for MySQL unmanaged view (discussed in comment#7)

Download all attachments as: .zip

Change History (11)

comment:1 follow-up: Changed 6 years ago by Alex

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Hrm, this will require some thought, as an unmanaged model *could* indicate something like a db view that you can't delete from, however that semantic is in no way implied by the documentation.

comment:2 Changed 6 years ago by Alex

  • Triage Stage changed from Unreviewed to Design decision needed

comment:3 Changed 6 years ago by anonymous

I would say that the proper way to handle this would be to have a kwarg for the delete method. QuerySet.delete(cascade=False) Seems reasonable, at least.

comment:4 Changed 6 years ago by mtredinnick

Urgh. This is messy. The idea in comment:3 isn't a solution, since this isn't a property of the delete() call -- it's a property of the model definition (and cascading delete behaviour is the subject of another ticket and unresolved as yet because it's a hard problem to cover all use-cases). The current behaviour is correct (unmanaged refers to syncdb and other SQL-altering statements), but we should be able to handle Justin's case, too.

Would be nice to have a solution for 1.1, so I'll give this some thought.

comment:5 Changed 6 years ago by justinlilly

For any looking for a stop-gap, I've gotten past it by writing a delete() method which invalidates the FK used to populate my view which then allows it to delete cleanly. Code is:

    def delete(self):
        baseid = self.baseobject_ptr_id
        BaseRelation.objects.filter(models.Q(base_obj=baseid) | models.Q(related_obj=baseid)).delete()
        super(Video, self).delete()

comment:6 Changed 6 years ago by jacob

  • milestone 1.1 deleted

This is probably a subset of the "deletes always cascade" bug, but even if not this won't be fixed in the 1.1 timeframe.

comment:7 Changed 6 years ago by philroche

This is a show stopper for us so I have a small patch

line 1002 django\db\models\query.py

perform_delete = True

if cls._meta.managed == False:
    check_view = "select count(TABLE_NAME) from INFORMATION_SCHEMA.VIEWS WHERE TABLE_NAME = '%s';" % cls._meta.db_table
    cursor = connection.cursor()
    cursor.execute(check_view, [])
    if not cursor.rowcount == 0:
        perform_delete = False
    
if perform_delete:
   ...... the rest of the deletion of code

It's ugly as hell I know but a start.

Changed 6 years ago by philroche

Patch for MySQL unmanaged view (discussed in comment#7)

comment:8 in reply to: ↑ 1 Changed 6 years ago by dvainsencher

Replying to Alex:

Hrm, this will require some thought, as an unmanaged model *could* indicate something like a db view that you can't delete from, however that semantic is in no way implied by the documentation.

I think that if you create a structure that can represent a view it's API should know how to deal with database changes operations. It's look like a bug to me.

comment:9 Changed 5 years ago by ticcky

Yet another dirty solution... =)

--- django/db/models/.svn/text-base/query.py.svn-base	2010-06-21 13:58:01.000000000 +0200
+++ django/db/models/query.py	2010-07-13 13:38:44.000000000 +0200
@@ -1304,6 +1304,8 @@
     obj_pairs = {}
     try:
         for cls in ordered_classes:
+            if cls._meta.managed == False:
+                continue
             items = seen_objs[cls].items()
             items.sort()
             obj_pairs[cls] = items
@@ -1327,6 +1329,8 @@
 
         # Now delete the actual data.
         for cls in ordered_classes:
+            if cls._meta.managed == False:
+                continue
             items = obj_pairs[cls]
             items.reverse()
 

comment:10 Changed 5 years ago by carljm

  • Resolution set to duplicate
  • Status changed from new to closed

Marking as duplicate of #7539; being able to turn off cascade per-FK is the correct solution here, since unmanaged does not imply unmodifiable.

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