[Django] #29503: Order of parameters pass to __in lookup affected by de-duplication.

6 views
Skip to first unread message

Django

unread,
Jun 18, 2018, 10:16:13 AM6/18/18
to django-...@googlegroups.com
#29503: Order of parameters pass to __in lookup affected by de-duplication.
-------------------------------------+-------------------------------------
Reporter: Nick Pope | Owner: Nick Pope
Type: Bug | Status: assigned
Component: Database | Version: 1.11
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
After the fix for ticket #25544, the order of parameters passed to the
`__in` lookup is affected by the de-duplication using `set()`.

This results in the generated query being unstable between different runs
of the python interpreter due to hash seed randomisation.

--
Ticket URL: <https://code.djangoproject.com/ticket/29503>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Jun 18, 2018, 10:23:06 AM6/18/18
to django-...@googlegroups.com
#29503: Order of parameters pass to __in lookup affected by de-duplication.
-------------------------------------+-------------------------------------
Reporter: Nick Pope | Owner: Nick Pope
Type: Bug | Status: assigned
Component: Database layer | Version: 1.11
(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 Nick Pope):

* has_patch: 0 => 1


Comment:

[https://github.com/django/django/pull/10060 PR]

--
Ticket URL: <https://code.djangoproject.com/ticket/29503#comment:1>

Django

unread,
Jun 18, 2018, 10:27:58 AM6/18/18
to django-...@googlegroups.com
#29503: Order of parameters pass to __in lookup affected by de-duplication.
-------------------------------------+-------------------------------------
Reporter: Nick Pope | Owner: Nick Pope
Type: Bug | Status: assigned
Component: Database layer | Version: 1.11
(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
-------------------------------------+-------------------------------------

Comment (by Tim Graham):

What practical problem did you encounter? I'm surprised that the order of
items in an `IN` clause would matter much.

--
Ticket URL: <https://code.djangoproject.com/ticket/29503#comment:2>

Django

unread,
Jun 18, 2018, 11:33:19 AM6/18/18
to django-...@googlegroups.com
#29503: Order of parameters pass to __in lookup affected by de-duplication.
-------------------------------------+-------------------------------------
Reporter: Nick Pope | Owner: Nick Pope
Type: Bug | Status: assigned
Component: Database layer | Version: 1.11
(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
-------------------------------------+-------------------------------------

Comment (by Nick Pope):

Replying to [comment:2 Tim Graham]:


> What practical problem did you encounter? I'm surprised that the order
of items in an `IN` clause would matter much.

Heh. I get the feeling that I'm going to get shot down here (and I know
and understand the reasons why).

I've inherited some code that currently and for the foreseeable future
depends on the output of the query generated by `Query.sql_with_params()`.
It executes it via `pandas.DataFrame.read_sql_query()` and also uses this
to generate a cache key. Because the parameter ordering is unstable, this
affects the key generation.

Regardless of the horrors of this, the issue looks like a regression to me
- the previous behaviour changed unexpectedly. I can understand if you'd
like to only apply this to master, however.

--
Ticket URL: <https://code.djangoproject.com/ticket/29503#comment:3>

Django

unread,
Jun 18, 2018, 7:52:44 PM6/18/18
to django-...@googlegroups.com
#29503: Order of parameters pass to __in lookup affected by de-duplication.
-------------------------------------+-------------------------------------
Reporter: Nick Pope | Owner: Nick Pope
Type: Bug | Status: assigned
Component: Database layer | Version: 1.11
(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 Ian Foote):

* cc: Ian Foote (added)


--
Ticket URL: <https://code.djangoproject.com/ticket/29503#comment:4>

Django

unread,
Jun 20, 2018, 3:26:54 AM6/20/18
to django-...@googlegroups.com
#29503: Order of parameters pass to __in lookup affected by de-duplication.
-------------------------------------+-------------------------------------
Reporter: Nick Pope | Owner: Nick Pope
Type: Bug | Status: assigned
Component: Database layer | Version: 1.11
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Carlton Gibson):

* cc: Carlton Gibson (added)
* stage: Unreviewed => Ready for checkin


Comment:

I'd be happy with the use of `OrderedSet` here. Semantically `in` uses a
set but we (always/mostly?) pass it a list, which is ordered, and it's
reasonable enough to maintain that. (Beyond Nick's usage, it could be
handy somewhere.)

Is this a regression? Was it always part of the contract? Not sure. Pass.
:)

--
Ticket URL: <https://code.djangoproject.com/ticket/29503#comment:5>

Django

unread,
Jun 20, 2018, 9:53:44 AM6/20/18
to django-...@googlegroups.com
#29503: Order of parameters pass to __in lookup affected by de-duplication.
-------------------------------------+-------------------------------------
Reporter: Nick Pope | Owner: Nick Pope
Type: Bug | Status: closed

Component: Database layer | Version: 1.11
(models, ORM) |
Severity: Normal | Resolution: fixed

Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Tim Graham <timograham@…>):

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


Comment:

In [changeset:"b0fbfae09334554124e1dccb7559d10f36e2f84c" b0fbfae0]:
{{{
#!CommitTicketReference repository=""
revision="b0fbfae09334554124e1dccb7559d10f36e2f84c"
Fixed #29503 -- Made __in lookup keep order of values in query.

Regression in 86eccdc8b67728d84440a46e5bf62c78f2eddf6d.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/29503#comment:6>

Django

unread,
Jun 20, 2018, 9:53:55 AM6/20/18
to django-...@googlegroups.com
#29503: Order of parameters pass to __in lookup affected by de-duplication.
-------------------------------------+-------------------------------------
Reporter: Nick Pope | Owner: Nick Pope
Type: Bug | Status: closed
Component: Database layer | Version: 1.11
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Tim Graham <timograham@…>):

In [changeset:"16e691d568a06873445171025ba44b5f00920e7e" 16e691d]:
{{{
#!CommitTicketReference repository=""
revision="16e691d568a06873445171025ba44b5f00920e7e"
[2.1.x] Fixed #29503 -- Made __in lookup keep order of values in query.

Regression in 86eccdc8b67728d84440a46e5bf62c78f2eddf6d.

Backport of b0fbfae09334554124e1dccb7559d10f36e2f84c from master
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/29503#comment:7>

Reply all
Reply to author
Forward
0 new messages