Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#29503 closed Bug (fixed)

Order of parameters pass to __in lookup affected by de-duplication.

Reported by: Nick Pope Owned by: Nick Pope
Component: Database layer (models, ORM) Version: 1.11
Severity: Normal Keywords:
Cc: Ian Foote, Carlton Gibson 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

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.

Change History (7)

comment:1 by Nick Pope, 6 years ago

Has patch: set

comment:2 by Tim Graham, 6 years ago

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

in reply to:  2 comment:3 by Nick Pope, 6 years ago

Replying to 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.

comment:4 by Ian Foote, 6 years ago

Cc: Ian Foote added

comment:5 by Carlton Gibson, 6 years ago

Cc: Carlton Gibson added
Triage Stage: UnreviewedReady for checkin

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. :)

comment:6 by Tim Graham <timograham@…>, 6 years ago

Resolution: fixed
Status: assignedclosed

In b0fbfae0:

Fixed #29503 -- Made in lookup keep order of values in query.

Regression in 86eccdc8b67728d84440a46e5bf62c78f2eddf6d.

comment:7 by Tim Graham <timograham@…>, 6 years ago

In 16e691d:

[2.1.x] Fixed #29503 -- Made in lookup keep order of values in query.

Regression in 86eccdc8b67728d84440a46e5bf62c78f2eddf6d.

Backport of b0fbfae09334554124e1dccb7559d10f36e2f84c from master

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