Opened 9 years ago

Last modified 3 months ago

#24638 new New feature

Support adding an SQL comment in queries

Reported by: Adam Johnson Owned by:
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords: queryset, query, comment, optimizer hint
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

This feature gives QuerySet a .comment(text) method that inserts text into a comment in the output SQL query, right after the SELECT [DISTINCT] part. This has two uses:

  1. For all database backends, this can be used for feature-to-query tracking. For example, you might override def queryset on your admin classes to use comment() to insert the name of the admin class generating the query, making query analysis (processlist, slow query logs, etc.) easier to track back to the point in the code generating the queries. This is not trivial.

I've seen Facebook have nice tools that automatically insert file/line number summaries into queries as comments to make analysis easy. Adding comment() would making this easy for custom QuerySet subclasses to do this however they want, with e.g. caller inspection at __init__ time.

  1. For the MySQL and Oracle backends, flags and optimizer hints can be added. Both have a number of options for queries that are otherwise unsupported on django, and they can be inserted with special comments.

This is my current motivation - I had a pretty complex 3-join/5-subquery MySQL query today that could only be made good on the ORM by adding MySQL's "STRAIGHT_JOIN" hint, and the only way I found to do this in Django at current was to monkey-patch the MySQL backend CursorWrapper's execute() to rewrite the generated query with regexes... :( And it couldn't use raw() since it was being passed to the admin with the ability to filter it.

I couldn't find any historical tickets on optimizer hints.

Change History (15)

comment:1 by Adam Johnson, 9 years ago

Ticket https://github.com/django/django/pull/4495

No documentation yet, don't know if it will be accepted...

comment:2 by Steve McConville, 9 years ago

Type: UncategorizedNew feature

comment:3 by Anssi Kääriäinen, 9 years ago

Triage Stage: UnreviewedAccepted

comment:4 by Tim Graham, 9 years ago

Has patch: set
Needs documentation: set

comment:5 by Adam Johnson, 9 years ago

For reference (checking I did get the right place for Oracle and MySQL query hints):

  1. MySQL hints are listed in its syntax https://dev.mysql.com/doc/refman/5.5/en/select.html (STRAIGHT_JOIN, SQL_NO_CACHE, etc.). They can be inserted as comments by starting the comment with an exclamation mark, which pretty much means "this is not a comment" (the reason this exists is that if you follow the exclamation mark with an encoded version number, that SQL will only be executed on versions of the server at that version or above). Also newer more powerful hints are coming in 5.7 with comments that start with a + : https://dev.mysql.com/worklog/task/?id=8017
  1. Oracle has extensive documentation on its hints which appear only in conditional comments, although the page is lacking in examples: http://docs.oracle.com/cd/B19306_01/server.102/b14211/hintsref.htm . However the syntax for SELECT does show hints going *before* DISTINCT: http://docs.oracle.com/cd/B28359_01/server.111/b28286/statements_10002.htm , however this blog post shows them after: http://www.dba-oracle.com/t_index_fast_full_scan.htm .

comment:6 by Anssi Kääriäinen, 9 years ago

It might be a bit ugly if we have to support multiple places for the comment. What about MSSQL, DB2 and other non-core backends? I'm not sure what is the best way to alter the generated SQL if we need support for injecting hints in arbitrary places.

Also, are we ok with support for read queries only, or should we have it for update/delete/insert queries, too? The delete queries might be especially hard for cascades. Maybe we should aim for "user generated select queries only" for the first implementation?

comment:7 by Adam Johnson, 9 years ago

MSSQL - it appears all hints come in an OPTION clause after HAVING: https://msdn.microsoft.com/en-us/library/ms189499.aspx
DB2 - it appears hints come in a %_HINTS clause after WHERE: http://scn.sap.com/thread/1573275

So yes, unfortunately this is not general-purpose enough for adding query hints.

I did try an implementation with the comments injected on DeleteSQLCompiler but the cascade did lead to it pretty broken since the query hint for delete was carried to the selects. User generated selects only sounds good - the current patch does update too, but it's not often you want to add hints on them anyway as there are no joins.

comment:8 by Adam Johnson, 9 years ago

For the record, here's the first open-source version of the regex-based SQL-rewriting code I have that adds MySQL's STRAIGHT_JOIN: https://github.com/adamchainz/django-mysql/commit/cf69c24962c4a22cc2fac5b29319777ee2a44fb0 . If we can't add .comment(), this is the probably the best solution possible.

comment:9 by Hannes Ljungberg, 22 months ago

Owner: changed from nobody to Hannes Ljungberg
Status: newassigned

comment:10 by Hannes Ljungberg, 22 months ago

Needs documentation: unset
Version: 1.8dev

comment:11 by Mariusz Felisiak, 22 months ago

Patch needs improvement: set
Summary: New feature: support adding an SQL comment in queriesSupport adding an SQL comment in queries

comment:12 by Hannes Ljungberg, 22 months ago

Patch needs improvement: unset

comment:13 by Mariusz Felisiak, 22 months ago

Needs tests: set
Patch needs improvement: set

comment:14 by Mariusz Felisiak, 14 months ago

Owner: Hannes Ljungberg removed
Status: assignednew

comment:15 by Michael Warkentin, 3 months ago

I've recently come across https://github.com/ossc-db/pg_hint_plan which also provides support for plan hints in Postgres (which I'd like to use, and the reason I've found this issue). Just thought that would be worth considering as well during implementation if this work goes through.

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