Opened 10 years ago

Last modified 7 years ago

#24254 closed Bug

Passing a query to "pk__in" of another query may result in invalid SQL — at Version 2

Reported by: Torsten Bronger Owned by: nobody
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords:
Cc: bronger@…, Simon Charette Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Torsten Bronger)

Consider the following model:

from django.db import models

class MyModel(models.Model):
    name = models.CharField(max_length=200)

    class Meta:
        ordering = ("name",)

Then, the following query:

MyModel.objects.filter(pk__in=MyModel.objects.all().distinct()[:10])

results in an invalid SQL query. SQLite complains with "only a single result allowed for a SELECT that is part of an expression".

Change History (2)

comment:1 by Peter Inglesby, 10 years ago

Have had a little dig into this.

Consider an __in lookup with the following on the RHS: M.objects.order_by(sort_field).distinct()[m:n].

This generates a query that returns both M's primary key and sort_field. Because it returns two fields, it cannot be used as a subquery (at least in SQLite) and so we hit the reported error.

I think this behaviour is related to a warning in the docs: "Any fields used in an order_by() call are included in the SQL SELECT columns" (although from reading the code for SQLCompiler.get_extra_select I think that the fields used in an order_by() call are only included in the SELECT columns when the query has distinct set).

It's not obvious to me why any fields used in an order_by() call must always be included in the SELECT columns, so I'd like to propose a fix whereby SQLCompiler.get_extra_select returns an empty list if it's dealing with a subquery. I've made that change locally, and nothing fails. However, I don't know enough about the ORM to know whether this is a good idea! I hope this is useful.

comment:2 by Torsten Bronger, 10 years ago

Description: modified (diff)

You're right, my example was not minimal. I changed it so that only one model is defined.

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