Invalid SQL statement generated when deleting rows in Meta ordered tables (found using Oracle)
|Reported by:||dylan.klomparens@…||Owned by:||nobody|
|Component:||Database layer (models, ORM)||Version:||1.5-beta-1|
|Severity:||Release blocker||Keywords:||ORM SQL ordering Meta|
|Has patch:||yes||Needs documentation:||no|
|Needs tests:||no||Patch needs improvement:||no|
Given the following example model, invalid SQL deletes are composed in Django's ORM. This breaks Django's interaction with Oracle, producing error 'ORA-00907'.
from django.db import models class OrderedModel(models.Model): name = models.CharField(max_length=100) class Meta: ordering = ['name'] def __unicode__(self): return unicode(self.name)
Django's source code comments state that a QuerySet delete is broken into two SQL queries: one to find the items to delete, and the second to perform the deletion. The general form of this is:
DELETE FROM "TABLE" WHERE "ID" IN (SEARCH_STATEMENT)
Thus, there is no reason to order SEARCH_STATEMENT. However, Django provides an ORDER BY clause in the SEARCH_STATEMENT when Meta ordering is specified. Using the example model shown above, deleting an object from the admin interface produces this SQL SEARCH_STATEMENT:
SELECT "Test_orderedmodel"."id", "Test_orderedmodel"."name" FROM "Test_orderedmodel" WHERE "Test_orderedmodel"."id" IN (5, 6) ORDER BY "Test_orderedmodel"."name" ASC
This bug can be remedied by patching function delete() in class QuerySet in file $DJANGO_HOME/db/models/query.py, line 525.
The buggy line is: del_query.query.clear_ordering()
And it should be: del_query.query.clear_ordering(force_empty=True)
After the code is patched the example SQL statement becomes:
SELECT "Test_orderedmodel"."id", "Test_orderedmodel"."name" FROM "Test_orderedmodel" WHERE "Test_orderedmodel"."id" IN (5, 6)
Further testing reveals that this is an isolated bug.
I looked for other uses of the clear_ordering() function in Django's source code that might cause similar bugs. I found two other usage of the function in $DJANGO_HOME/db/models/query.py: one in the latest() function and the other in the order_by() function. After extensive testing I concluded that Meta ordering does not adversely affect these functions. That said, it may be prudent to make the force_empty parameter of the clear_ordering() function an explicit parameter instead of providing a default of False. All other uses of the clear_ordering() function that I found specify force_empty=True.
I am 99% confident this bug does not adversely affect any other portions of Django's code.
Change History (4)
comment:1 Changed 2 years ago by akaariai
- Needs documentation unset
- Needs tests unset
- Patch needs improvement unset
- Severity changed from Normal to Release blocker
- Triage Stage changed from Unreviewed to Accepted
comment:2 Changed 2 years ago by Anssi Kääriäinen <akaariai@…>
- Resolution set to fixed
- Status changed from new to closed