Code

Opened 6 years ago

Closed 6 years ago

#7056 closed (fixed)

Query.as_sql() has side effects for select_related queries

Reported by: ikelly 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")',
 ())

Attachments (0)

Change History (3)

comment:1 Changed 6 years ago by mtredinnick

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

comment:2 Changed 6 years ago by mtredinnick

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 6 years ago by mtredinnick

  • Resolution set to fixed
  • Status changed from new to closed

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

Fixed #7056

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.