Re: [Django] #23259: Query extra() select_params are inserted in the incorrect order in the overall query parameters with values_list

[Django]

#23259: Query extra() select_params are inserted in the incorrect order in the
overall query parameters with values_list
-+-
 Reporter:  rajivm   |Owner:  akaariai
 Type:  Bug  |   Status:  closed
Component:  Database layer   |  Version:  1.7-rc-2
  (models, ORM)  |   Resolution:  fixed
 Severity:  Release blocker  | Triage Stage:  Accepted
 Keywords:   |  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  1
  Needs tests:  1|UI/UX:  0
Easy pickings:  0|
-+-

Comment (by Anssi Kääriäinen ):

 In [changeset:"4ce5ced230481fc93288aeea922398bc36102d1e"]:
 {{{
 #!CommitTicketReference repository=""
 revision="4ce5ced230481fc93288aeea922398bc36102d1e"
 [1.7.x] Fixed #23259 -- Corrected insertion order of extra() select_params

 A regression caused queries to produce incorrect results for cases where
 extra(select) is excluded by values() but included by extra(order_by)

 The regression was caused by 2f35c6f10fcbae541691207fb0c0560a13b754fc.

 Backport of f0b358880a from master
 }}}

-- 
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/064.2a55d6f3e96d75c18e75f3f79d9684fe%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #23259: Query extra() select_params are inserted in the incorrect order in the overall query parameters with values_list

[Django]

#23259: Query extra() select_params are inserted in the incorrect order in the
overall query parameters with values_list
-+-
 Reporter:  rajivm   |Owner:  akaariai
 Type:  Bug  |   Status:  closed
Component:  Database layer   |  Version:  1.7-rc-2
  (models, ORM)  |   Resolution:  fixed
 Severity:  Release blocker  | Triage Stage:  Accepted
 Keywords:   |  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  1
  Needs tests:  1|UI/UX:  0
Easy pickings:  0|
-+-
Changes (by Anssi Kääriäinen ):

 * status:  assigned => closed
 * resolution:   => fixed


Comment:

 In [changeset:"f0b358880a6825d667c037757caac470bc526a1f"]:
 {{{
 #!CommitTicketReference repository=""
 revision="f0b358880a6825d667c037757caac470bc526a1f"
 Fixed #23259 -- Corrected insertion order of extra() select_params

 A regression caused queries to produce incorrect results for cases where
 extra(select) is excluded by values() but included by extra(order_by)

 The regression was caused by 2f35c6f10fcbae541691207fb0c0560a13b754fc.
 }}}

-- 
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/064.da2dd9e21bc191a9344b6e6c7437af18%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #23259: Query extra() select_params are inserted in the incorrect order in the overall query parameters with values_list

[Django]

#23259: Query extra() select_params are inserted in the incorrect order in the
overall query parameters with values_list
-+-
 Reporter:  rajivm   |Owner:  akaariai
 Type:  Bug  |   Status:  assigned
Component:  Database layer   |  Version:  1.7-rc-2
  (models, ORM)  |   Resolution:
 Severity:  Release blocker  | Triage Stage:  Accepted
 Keywords:   |  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  1
  Needs tests:  1|UI/UX:  0
Easy pickings:  0|
-+-
Changes (by akaariai):

 * owner:  nobody => akaariai
 * status:  new => assigned


Comment:

 I'll finish this one.

-- 
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/064.c466e89230395454dbe2a305d53d2269%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #23259: Query extra() select_params are inserted in the incorrect order in the overall query parameters with values_list

[Django]

#23259: Query extra() select_params are inserted in the incorrect order in the
overall query parameters with values_list
-+-
 Reporter:  rajivm   |Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  1.7-rc-2
  (models, ORM)  |   Resolution:
 Severity:  Release blocker  | Triage Stage:  Accepted
 Keywords:   |  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  1
  Needs tests:  1|UI/UX:  0
Easy pickings:  0|
-+-

Comment (by andrewgodwin):

 Can one of the other core devs or the patch author tell me what about this
 patch needs improvement? Or are we waiting for someone to review the
 tests?

-- 
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/064.4dee2fd3e9279c3b549634c1cbe1f648%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #23259: Query extra() select_params are inserted in the incorrect order in the overall query parameters with values_list

[Django]

#23259: Query extra() select_params are inserted in the incorrect order in the
overall query parameters with values_list
-+-
 Reporter:  rajivm   |Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  1.7-rc-2
  (models, ORM)  |   Resolution:
 Severity:  Release blocker  | Triage Stage:  Accepted
 Keywords:   |  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  1
  Needs tests:  1|UI/UX:  0
Easy pickings:  0|
-+-

Comment (by rajivm):

 I have added tests to the provided pull request that cover this specific
 issue. However, on a more general note, it seems like tests covering this
 area are lacking (the previously-referenced issue / commit that introduced
 this bug added more tests, but they only cover a very narrow case, mainly,
 there are many tests around ordering in values() w/ extras, but they don't
 actually test ordering it-self).

-- 
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/064.795cb46710fc29a432545abb984d7b9c%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #23259: Query extra() select_params are inserted in the incorrect order in the overall query parameters with values_list

[Django]

#23259: Query extra() select_params are inserted in the incorrect order in the
overall query parameters with values_list
-+-
 Reporter:  rajivm   |Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  1.7-rc-2
  (models, ORM)  |   Resolution:
 Severity:  Release blocker  | Triage Stage:  Accepted
 Keywords:   |  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  1
  Needs tests:  1|UI/UX:  0
Easy pickings:  0|
-+-
Changes (by charettes):

 * needs_better_patch:  0 => 1
 * has_patch:  0 => 1
 * needs_tests:  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/064.302b63bd3ef3ece3d8ed051cf6db52ed%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #23259: Query extra() select_params are inserted in the incorrect order in the overall query parameters with values_list (was: Query extra(select=) select_params are parameterized in the

[Django]

#23259: Query extra() select_params are inserted in the incorrect order in the
overall query parameters with values_list
-+-
 Reporter:  rajivm   |Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  1.7-rc-2
  (models, ORM)  |   Resolution:
 Severity:  Release blocker  | Triage Stage:  Accepted
 Keywords:   |  Needs documentation:  0
Has patch:  0|  Patch needs improvement:  0
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-
Changes (by akaariai):

 * severity:  Normal => Release blocker
 * stage:  Unreviewed => Accepted


Comment:

 I don't agree with security ramifications here. Yes, there could be some
 cases where this could lead to users seeing data they aren't supposed to.
 But, any bug in the ORM that produces wrong results qualify for that, and
 we definitely are not going to interpret all ORM bugs as security issues.

 However, this is a regression and thus a release blocker.

 I haven't tested this myself, but based on the research done this seems
 valid, so marking as 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/064.bf32c79fc7997cbd01eca0fd6696ed52%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #23259: Query extra() select_params are inserted in the incorrect order in the overall query parameters with values_list

[Django]

#23259: Query extra() select_params are inserted in the incorrect order in the
overall query parameters with values_list
-+-
 Reporter:  rajivm   |Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  1.7-rc-2
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:
 Keywords:   |  Unreviewed
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by rajivm):

 This bug seems to have been introduced at
 2f35c6f10fcbae541691207fb0c0560a13b754fc when trying to resolve ticket
 #14930.

 Based on some cursory research into the source of the bug, it looks like
 the parameters are simply being inserted too early by the sql compiler in
 this situation, and I have created a patch (sans tests) and created a
 pull-request: https://github.com/django/django/pull/3033. It's late now,
 but I will try to add tests to it tomorrow.

-- 
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/064.f28c65774ab3225c6abed6ea980435eb%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #23259: Query extra() select_params are inserted in the incorrect order in the overall query parameters with values_list

[Django]

#23259: Query extra() select_params are inserted in the incorrect order in the
overall query parameters with values_list
-+-
 Reporter:  rajivm   |Owner:  nobody
 Type:  Bug  |   Status:  new
Component:  Database layer   |  Version:  1.7-rc-2
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:
 Keywords:   |  Unreviewed
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by rajivm):

 * type:  Uncategorized => Bug


-- 
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/064.d5e68f8b92c19ae07867e85ea381affd%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #23259: Query extra() select_params are inserted in the incorrect order in the overall query parameters with values_list

[Django]

#23259: Query extra() select_params are inserted in the incorrect order in the
overall query parameters with values_list
-+-
 Reporter:  rajivm   |Owner:  nobody
 Type:  Uncategorized|   Status:  new
Component:  Database layer   |  Version:  1.7-rc-2
  (models, ORM)  |   Resolution:
 Severity:  Normal   | Triage Stage:
 Keywords:   |  Unreviewed
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by rajivm):

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


Comment:

 Also, imho, this could be a release blocker because it has potential
 security ramifications since the parameters are being mismatched.

-- 
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/064.03a0c198145cfcd2b006a55f55ddd0be%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

[Django] #23259: Query extra() select_params are inserted in the incorrect order in the overall query parameters with values_list

[Django]

#23259: Query extra() select_params are inserted in the incorrect order in the
overall query parameters with values_list
--+--
 Reporter:  rajivm|  Owner:  nobody
 Type:  Uncategorized | Status:  new
Component:  Database layer (models, ORM)  |Version:  1.7-rc-2
 Severity:  Normal|   Keywords:
 Triage Stage:  Unreviewed|  Has patch:  0
Easy pickings:  0 |  UI/UX:  0
--+--
 This is running the latest stable/1.7
 (267630ad50c69ebfe594de37a0636264aa5be7d6)

 {{{
 User.objects.extra(select=OrderedDict([('points', 'id + %s')]),
 
select_params=[5]).filter(id__gte=3).extra(order_by=['points']).values_list("id",
 flat=True)
 }}}

 {{{
 [DEBUG django.db.backends] (0.001) SELECT "auth_user"."id" FROM
 "auth_user" WHERE "auth_user"."id" >= 5 ORDER BY (id + 3) ASC LIMIT 21;
 args=(5, 3)
 }}}

 Whereas in Django 1.6 (the correct query):

 {{{
 [DEBUG django.db.backends] (0.003) SELECT "auth_user"."id", (id + 5) AS
 "points" FROM "auth_user" WHERE "auth_user"."id" >= 3  ORDER BY "points"
 ASC LIMIT 21; args=(5, 3)
 }}}

 In the second case, the extra select_param of 5 is correctly matched with
 the extra select "id + %s", resulting in id+5.
 However, in the first case, in Django 1.7, the extra select results in id
 + 3 (incorrectly inverting the parameters to the query).

 It seems like what is happening is the "select" of "points" (the extra
 select) is being optimized out because of the values_list, but is being
 kept by the extra(order_by) -- resulting in it being in a different part
 of the query.

 I tried to simplify the example as much as possible (I had a much crazier
 query this was destroying).

 Let me know if you have any questions.

-- 
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/049.65847aa099af5caaf480b67af959d8b2%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.