Re: [Django] #24254: Passing a query to "pk__in" of another query may result in invalid SQL

[Django]

#24254: Passing a query to "pk__in" of another query may result in invalid SQL
-+-
 Reporter:  bronger  |Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  master
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-
Changes (by charettes):

 * cc: charettes (added)
 * version:  1.7 => master


Comment:

 We should be able to avoid unconditional tuple wrapping as it only seems
 to be required when `DISTINCT` is used with `ORDER BY`.

--
Ticket URL: 
Django 
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 django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/065.c5a910e3e81b65aa4fc77c570a59a1c6%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #24254: Passing a query to "pk__in" of another query may result in invalid SQL

[Django]

#24254: Passing a query to "pk__in" of another query may result in invalid SQL
-+-
 Reporter:  bronger  |Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  1.7
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-

Comment (by rtpg):

 I've hit this issue as well, reaching an identical situation of `__in` +
 ordering + distinct + slicing.

 Typing this out, I realised that if you have ordering and slicing, then
 you'll need the values in the `SELECT` (see the Postgres error mentioned
 before).

 So I would propose a "tuple unwrapping" in that case. So the `pk__in=qs`
 property would expand to SQL like `SELECT U1."id" FROM (SELECT DISTINCT
 ... ) U1`


 {{{
 In [61]: print
 MyModel.objects.filter(pk__in=MyModel.objects.all().distinct()[:10]).query
 # invalid syntax due to missing tuple unwrapping but includes proper
 ordering
 SELECT "test_mymodel"."id", "test_mymodel"."name" FROM "test_mymodel"
  WHERE "test_mymodel"."id" IN
   (SELECT DISTINCT "test_mymodel"."id", "test_mymodel"."name" FROM
 "test_mymodel" ORDER BY "test_mymodel"."name" ASC LIMIT 10)
 ORDER BY "test_mymodel"."name" ASC

 # proposed solution with tuple unwrapping
 SELECT "test_mymodel"."id", "test_mymodel"."name" FROM "test_mymodel"
 WHERE "test_mymodel"."id" IN
 (SELECT U0."id" FROM
   (SELECT DISTINCT "test_mymodel"."id", "test_mymodel"."name" FROM
 "test_mymodel" ORDER BY "test_mymodel"."name" ASC LIMIT 10) U0
 )
  ORDER BY "test_mymodel"."name" ASC
 }}}


 I think we could always do wrapping and unwrapping of `__in` subqueries.
 The cost is pretty negligible in Postgres (an extra select node), and
 avoids having to figure out the innards of the subquery select.

--
Ticket URL: 
Django 
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 django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/065.8c17a540762f7cb7acdc2545e58db886%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #24254: Passing a query to "pk__in" of another query may result in invalid SQL

[Django]

#24254: Passing a query to "pk__in" of another query may result in invalid SQL
-+-
 Reporter:  bronger  |Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  1.7
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-

Comment (by inglesp):

 The proposed patch passes on SQLite, but fails on Postgres, with:

 {{{
 for SELECT DISTINCT, ORDER BY expressions must appear in select list LINE
 1: ...eries_person"."id" FROM "queries_person" ORDER BY "queries_p...
 }}}

 and on MySQL, with:

 {{{
 This version of MySQL doesn't yet support 'LIMIT & IN/ALL/ANY/SOME
 subquery'
 }}}

 I'm afraid I don't know enough about either database to know what the
 right fix is.

--
Ticket URL: 
Django 
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 django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/065.ec40a3244dc28669f6737d0b943572a9%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #24254: Passing a query to "pk__in" of another query may result in invalid SQL

[Django]

#24254: Passing a query to "pk__in" of another query may result in invalid SQL
-+-
 Reporter:  bronger  |Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  1.7
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-
Changes (by timgraham):

 * needs_better_patch:  0 => 1


--
Ticket URL: 
Django 
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 django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/065.44499fb4a370e19eeba2264be4f1f0de%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #24254: Passing a query to "pk__in" of another query may result in invalid SQL

[Django]

#24254: Passing a query to "pk__in" of another query may result in invalid SQL
-+-
 Reporter:  bronger  |Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  1.7
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by inglesp):

 * stage:  Unreviewed => Accepted


--
Ticket URL: 
Django 
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 django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/065.b411e2e51b208343de5bace6aaeae753%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #24254: Passing a query to "pk__in" of another query may result in invalid SQL

[Django]

#24254: Passing a query to "pk__in" of another query may result in invalid SQL
-+-
 Reporter:  bronger  |Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  1.7
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:
 |  Unreviewed
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by inglesp):

 * has_patch:  0 => 1


Comment:

 I've added a patch with a fix and a test at
 https://github.com/django/django/pull/4078.

--
Ticket URL: 
Django 
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 django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/065.c4ad6da8207e9916b63d316d507851af%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #24254: Passing a query to "pk__in" of another query may result in invalid SQL

[Django]

#24254: Passing a query to "pk__in" of another query may result in invalid SQL
-+-
 Reporter:  bronger  |Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  1.7
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:
 |  Unreviewed
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Old description:

> Consider the following models:
>
> {{{
> from django.db import models
>

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

> class Son(Father):
>
> class Meta:
> ordering = ("name",)
> }}}
>
> Then, the following query:
>
> {{{
> Son.objects.filter(pk__in=Son.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".

New description:

 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".

--

Comment (by bronger):

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

--
Ticket URL: 
Django 
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 django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/065.e9acd5d40d2837458e48fbffb401cb87%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #24254: Passing a query to "pk__in" of another query may result in invalid SQL

[Django]

#24254: Passing a query to "pk__in" of another query may result in invalid SQL
-+-
 Reporter:  bronger  |Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  1.7
  (models, ORM)  |
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:
 |  Unreviewed
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by inglesp):

 * needs_better_patch:   => 0
 * needs_tests:   => 0
 * needs_docs:   => 0


Comment:

 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
 
[https://docs.djangoproject.com/en/dev/ref/models/querysets/#django.db.models.query.QuerySet.distinct
 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.

--
Ticket URL: 
Django 
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 django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/065.63b93f9b930942b1868bbb5d292b4606%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

[Django] #24254: Passing a query to "pk__in" of another query may result in invalid SQL

[Django]

#24254: Passing a query to "pk__in" of another query may result in invalid SQL
--+
 Reporter:  bronger   |  Owner:  nobody
 Type:  Bug   | Status:  new
Component:  Database layer (models, ORM)  |Version:  1.7
 Severity:  Normal|   Keywords:
 Triage Stage:  Unreviewed|  Has patch:  0
Easy pickings:  0 |  UI/UX:  0
--+
 Consider the following models:

 {{{
 from django.db import models


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


 class Son(Father):

 class Meta:
 ordering = ("name",)
 }}}

 Then, the following query:

 {{{
 Son.objects.filter(pk__in=Son.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".

--
Ticket URL: 
Django 
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 django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/050.c71660a649e7dff8d8fc340890b73693%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.