Opened 8 years ago

Closed 7 years ago

#23622 closed Bug (fixed)

Subquery doesn't respect order when not bound to a __pk__ field.

Reported by: ris Owned by:
Component: Database layer (models, ORM) Version: 1.7
Severity: Normal Keywords: subquery order distinct
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Fix for bug #20600 doesn't seem to have covered all cases.

Using django git fa4b6482df08d308fe88044b8c8bf981c6225fb8 (stable/1.7.x), postgres backend.

Example models.py:

class ModelA ( models.Model ):
	pass

class ModelB ( models.Model ):
	modela_fk = models.ForeignKey ( ModelA )
	
	field_b0 = models.IntegerField ( null = True )
	field_b1 = models.DateField ()

Enter some data:

import datetime
a1 = ModelA ()
a1.save ()
a2 = ModelA ()
a2.save ()
ModelB ( modela_fk = a1 , field_b0 = 123 , field_b1 = datetime.date ( 2013 , 1 , 6 ) ).save ()
ModelB ( modela_fk = a1 , field_b0 = 23 , field_b1 = datetime.date ( 2011 , 6 , 6 ) ).save ()
ModelB ( modela_fk = a1 , field_b0 = 234 , field_b1 = datetime.date ( 2011 , 9 , 2 ) ).save ()
ModelB ( modela_fk = a1 , field_b0 = 12 , field_b1 = datetime.date ( 2012 , 9 , 15 ) ).save ()
ModelB ( modela_fk = a2 , field_b0 = 567 , field_b1 = datetime.date ( 2014 , 3 , 1 ) ).save ()
ModelB ( modela_fk = a2 , field_b0 = 76 , field_b1 = datetime.date ( 2011 , 3 , 3 ) ).save ()
ModelB ( modela_fk = a2 , field_b0 = 7 , field_b1 = datetime.date ( 2012 , 10 , 20 ) ).save ()
ModelB ( modela_fk = a2 , field_b0 = 56 , field_b1 = datetime.date ( 2011 , 1 , 27 ) ).save ()

Some queries:

qx = (
	Q ( modelb__pk__in = ModelB.objects.order_by ( "modela_fk" , "-field_b1" ).distinct ( "modela_fk" ) )
	& Q ( modelb__field_b0__gte = 50 )
)

qy = (
	Q ( modelb__in = ModelB.objects.order_by ( "modela_fk" , "-field_b1" ).distinct ( "modela_fk" ) )
	& Q ( modelb__field_b0__gte = 50 )
)

(only difference being modelb__pk__in vs modelb__in)

and...

>>> frozenset ( ModelA.objects.filter ( qx ).values_list ( "pk" , flat = True ) ) == frozenset ( ModelA.objects.filter ( qy ).values_list ( "pk" , flat = True ) )
False

We see this is because

>>> str ( ModelA.objects.filter ( qx ).query )
'SELECT "dummy_modela"."id" FROM "dummy_modela" INNER JOIN "dummy_modelb" ON ( "dummy_modela"."id" = "dummy_modelb"."modela_fk_id" ) WHERE ("dummy_modelb"."id" IN (SELECT DISTINCT ON ("dummy_modelb"."modela_fk_id") "dummy_modelb"."id" FROM "dummy_modelb" ORDER BY "dummy_modelb"."modela_fk_id" ASC, "dummy_modelb"."field_b1" DESC) AND "dummy_modelb"."field_b0" >= 50)'
>>> str ( ModelA.objects.filter ( qy ).query )
'SELECT "dummy_modela"."id" FROM "dummy_modela" INNER JOIN "dummy_modelb" ON ( "dummy_modela"."id" = "dummy_modelb"."modela_fk_id" ) WHERE ("dummy_modelb"."id" IN (SELECT DISTINCT ON ("dummy_modelb"."modela_fk_id") "dummy_modelb"."id" FROM "dummy_modelb") AND "dummy_modelb"."field_b0" >= 50)'

qy's SQL is missing the ORDER BY clause. This is a bit of an unexpected gotcha.

Change History (5)

comment:1 Changed 8 years ago by Nadja Deininger

Owner: changed from nobody to Nadja Deininger
Status: newassigned
Triage Stage: UnreviewedAccepted

I've been able to reproduce this on my machine. Looks like a bug to me, and I'll take a look at the code and try to fix it.

comment:2 Changed 8 years ago by Nadja Deininger

Owner: Nadja Deininger deleted
Status: assignednew

Unassigning myself for now as I'm a bit busier than anticipated and am not sure I'll be able to fix it in a timely fashion.

comment:3 Changed 8 years ago by ris

I've also just realized (in #23791) that adding a .values ( "pk" ) also removes the ORDER BY clause of the subquery

comment:4 Changed 7 years ago by Tim Graham <timograham@…>

In baa732a:

Refs #23622 -- Added tests to ensure ordering is retained for distinct on fields subqueries.

The ticket was already fixed by
b68212f539f206679580afbfd008e7d329c9cd31.

Thanks to Beauhurst for commissioning the work on this ticket.

comment:5 Changed 7 years ago by Tim Graham

Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.
Back to Top