Opened 11 months ago

Last modified 3 months ago

#27587 new Cleanup/optimization

Document str(QuerySet.query)

Reported by: Peter Inglesby Owned by: nobody
Component: Documentation Version: 1.10
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

As far as I can tell, the only place where the behaviour of str(QuerySet.query) is mentioned in the docs is as a throwaway comment in the aggregation topic, and in a code example in the GeoDjango tutorial.

This is very useful behaviour but it is not widely known. Additionally, its limitations (see ticket:12611, "Incorrect quoting in QuerySet.query.str()") are even less widely known.

I think it would be suitable to add this to section 2 of the tutorial, and to the Making queries topic guide.

Change History (7)

comment:1 Changed 11 months ago by Aymeric Augustin

Regarding the limitations, I don't think we should accept the current status quo as the best we can do:

  1. if it's reasonably easy to perform the escaping correctly, typically just by escaping arguments with a function provided by the database adapter prior to interpolation, then Django should do it.
  2. if there's no function for escaping arguments, but a well documented and not too complicated process do to so (replace " by "" then wrap in "...") then we should consider doing it as well
  3. if the escaping rules are unclear and there's no way to ask the database to do it, then we should return something that is *obviously* invalid, like QUERY = ... ; PARAMS = ...

I did that some time ago for last_executed_query. SQLite stayed at 3 for half a decade before moving to 2.

I believe the same logic should apply to QuerySet.query and similar methods.

comment:2 Changed 11 months ago by Tim Graham

Easy pickings: unset
Summary: str(QuerySet.query) should be documentedDocument str(QuerySet.query)
Triage Stage: UnreviewedAccepted
Type: UncategorizedCleanup/optimization

Another place to document it is docs/ref/models/querysets.txt.

comment:3 Changed 11 months ago by Mads Jensen

comment:4 Changed 11 months ago by Mads Jensen

Has patch: set

comment:5 Changed 9 months ago by Tim Graham

Patch needs improvement: set

The current limitations described in this ticket and in #18631 should also be mentioned.

comment:6 Changed 6 months ago by Mads Jensen

Patch needs improvement: unset

comment:7 Changed 3 months ago by Tim Graham

Patch needs improvement: set
Note: See TracTickets for help on using tickets.
Back to Top