#24431: Combining extra(), annotate() or distinct() and count() can generate
invalid SQL
-------------------------------------+-------------------------------------
Reporter: briankrznarich | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.8beta1
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: extra distinct | Triage Stage: Accepted
aggregate count annotate |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by briankrznarich):
Thanks for the quick response. I've been poking at this trying to
understand completely what's going on, and now I'm afraid that the
examples I've given may simply have undefined results (which happened to
work out in 1.7, but not 1.8).
In both of my examples, I relied on select_related() to ensure that the
tables I needed for the extra() call would be available. However,
select_related() does not make that guarantee. I've found an even
simpler example that fails(and others have historically noted similar
problems):
{{{#!python
Permission.objects.all().select_related('content_type').extra(select={'foo':"django_content_type.app_label"}).order_by().values('foo')
}}}
This fails with a SQL generation error, since values() drops the contents
of select_related. The empty .order_by() is needed to remove the default
ordering of the Permission class, which references ContentType explicitly:
{{{#!python
class Meta:
ordering = ('content_type__app_label', 'content_type__model',
'codename')
}}}
If I leave the default ordering in, the command succeeds, since the
ordering forces the join to occur.
Here the query works until values() is called. The ValuesQuerySet class
clearly references bug #3358 right before discarding select_related. Bug
#3358 says it's fixed, but the source code does not agree.
{{{#!python
class ValuesQuerySet(QuerySet):
def __init__(self, *args, **kwargs):
super(ValuesQuerySet, self).__init__(*args, **kwargs)
# select_related isn't supported in values(). (FIXME -#3358)
self.query.select_related = False
}}}
Commenting out the line simply leads to other problems.
It seems the real issue is that it isn't obvious how to ensure that a
required JOIN will actually occur for extra() if your queryset is subject
to modification by code you did not write. In our case, tastypie is free
to append .count(), .distinct(), and .values() to whatever we might
construct. The one technique that seems to work is to apply a filter()
against the needed table with a clause that is always true. For example,
in the Permission example above:
{{{#!python
Permission.objects.all().filter(content_type__app_label__isnull=False).select_related('content_type').extra(select={'foo':"django_content_type.app_label"}).order_by().values('foo')
}}}
Here I've verified that a guaranteed NOT NULL field is, in fact, not null.
Any decent database should manage that rather quickly. This is the
technique I have applied to our source code as a workaround.
.order_by() is not even reliable for this purpose, as a distinct() applied
later will discard the order_by clause.
Counting works on this query:
{{{#!python
Permission.objects.all().extra(select={'foo':"django_content_type.app_label"}).order_by('content_type__app_label').count()
}}}
Distinct works, no problem
{{{#!python
Permission.objects.all().extra(select={'foo':"django_content_type.app_label"}).order_by('content_type__app_label').distinct()
}}}
However, distinct()+count() fails. The distinct() is performed in a
subselect with the ORDER BY clause discarded. This is in the same block
of code as my original ticket description, but the fix is not so straight
forward. PostgreSQL has no problem with ORDER BY in subselects, but other
DBs certainly do (SQL Server, for instance).
{{{#!python
Permission.objects.all().extra(select={'foo':"django_content_type.app_label"}).order_by('content_type__app_label').distinct().count()
}}}
I think the correct fix in the long term(if it is fixed, I know extra() is
a little hacky to begin with) is for extra() to have a mechanism to
specify the simple JOINs it requires. That's not easy pickings anymore.
In the interim, the filter() trick seems like the safe bet. Perhaps the
documentation could use some comments on the unreliability of order_by and
select_related with regards to extra().
--
Ticket URL: <https://code.djangoproject.com/ticket/24431#comment:5>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/072.80981a6739ff133c61c88e6a904a04e4%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
