Opened 3 years ago

Closed 3 years ago

#19190 closed Cleanup/optimization (fixed)

Refactor sql.query.select and select_fields

Reported by: akaariai Owned by: nobody
Component: Database layer (models, ORM) Version: master
Severity: Normal Keywords:
Cc: 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

I introduced some major gis regressions in commit [f64a5ef404cb6fd28e008a01039a3beea2fa8e1b] - a fix for #19102. The reason for the regressions was that Query.select and select_fields must be in sync, but this applies only to gis backend. The commit did:

qs.select = [something]

while it should have done:

qs.select, qs.select_fields = [something], [None]

The qs.select and select_fields must always be edited in sync for gis to work. There are not even comments explaining how select and select_fields work currently.

I have implemented a refactor for this which removes select_fields and collapses it into qs.select. This way it will be impossible to do the same mistake I did in future.

It seems similar refactoring should be done for related_select_cols/related_select_fields.

Change History (5)

comment:1 Changed 3 years ago by akaariai

I have pushed a commit to the branch to do similar collapsing for related_select_cols and related_select_fields.

comment:2 Changed 3 years ago by aaugustin

  • Triage Stage changed from Unreviewed to Accepted

comment:3 Changed 3 years ago by akaariai

  • Triage Stage changed from Accepted to Ready for checkin

The branch has been rebased, and is now RFC.

comment:4 Changed 3 years ago by Alex

django/contrib/gis/db/models/sql/compiler.py: line 88, remove the zip

otherwise LGTM, please merge

comment:5 Changed 3 years ago by Anssi Kääriäinen <akaariai@…>

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

In 11699ac4b5f98ec11dba02b356a8fd4ab6b4b889:

Fixed #19190 -- Refactored Query select clause attributes

The Query.select and Query.select_fields were collapsed into one list
because the attributes had to be always in sync. Now that they are in
one attribute it is impossible to edit them out of sync.

Similar collapse was done for Query.related_select_cols and
Query.related_select_fields.

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