Opened 8 years ago

Closed 8 years ago

#7056 closed (fixed)

Query.as_sql() has side effects for select_related queries

Reported by: Ian Kelly Owned by: nobody
Component: Database layer (models, ORM) Version: queryset-refactor
Severity: Keywords: qs-rf
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

Each time Query.as_sql() is called on a select_related query, the list of select columns grows again. Presumably Query.pre_sql_setup() should only get called once? For example:

In [39]: qs = Species.objects.all().select_related(depth=1)

In [40]: qs.query.as_sql()
Out[40]:
('SELECT "TEST_SPECIES"."ID", "TEST_SPECIES"."NAME", "TEST_SPECIES"."GENUS_ID", "TEST_GENUS"."ID", "TEST_GENUS"."NAME", "TEST_GENUS"."FAMILY_ID" FROM "TEST_SPECIES" INNER JOIN "TEST_GENUS" ON ("TEST_SPECIES"."GENUS_ID" = "TEST_GENUS"."ID")',
 ())

In [41]: qs.query.as_sql()
Out[41]:
('SELECT "TEST_SPECIES"."ID", "TEST_SPECIES"."NAME", "TEST_SPECIES"."GENUS_ID", "TEST_GENUS"."ID", "TEST_GENUS"."NAME", "TEST_GENUS"."FAMILY_ID", "TEST_SPECIES"."ID", "TEST_SPECIES"."NAME", "TEST_SPECIES"."GENUS_ID", "TEST_GENUS"."ID", "TEST_GENUS"."NAME", "TEST_GENUS"."FAMILY_ID" FROM "TEST_SPECIES" INNER JOIN "TEST_GENUS" ON ("TEST_SPECIES"."GENUS_ID" = "TEST_GENUS"."ID")',
 ())

Change History (3)

comment:1 Changed 8 years ago by Malcolm Tredinnick

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset
Triage Stage: UnreviewedAccepted

comment:2 Changed 8 years ago by Malcolm Tredinnick

We can't really arrange to call pre_sql_setup() just once, because if we did that and then added something to the query set, it could change the required select columns and we wouldn't know how to undo the previous select modifications from pre_sql_setup(). So the sequence

qs = ....
qs.query.as_sql()
qs = qs.select_related(depth=2)
qs.query.as_sql()

and similar usage patterns wouldn't be correct.

I've fixed this by keeping the extra select columns separate until we construct the query in as_sql().

comment:3 Changed 8 years ago by Malcolm Tredinnick

Resolution: fixed
Status: newclosed

(In [7445]) queryset-refactor: Removed an unwanted side-effect from the Query.as_sql() method.

Fixed #7056

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