[Django] #30242: Double spaces before limit/offset clause in as_sql() of SQLCompiler

5 views
Skip to first unread message

Django

unread,
Mar 7, 2019, 11:47:45 PM3/7/19
to django-...@googlegroups.com
#30242: Double spaces before limit/offset clause in as_sql() of SQLCompiler
-------------------------------------+-------------------------------------
Reporter: hangpark | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Database | Version: master
layer (models, ORM) | Keywords: sql
Severity: Normal | databaseoperations sqlcompiler
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 1
UI/UX: 0 |
-------------------------------------+-------------------------------------
`SQLCompiler` has `as_sql()` method to create the SQL for a given query.
Specifically, end-users can get the SQL string of their `QuerySet` by this
method like:

{{{
User.objects.values('id').query.__str__()
}}}

where `User` is a basic user model.

Absolutely, this returns a SQL string `SELECT "auth_user"."id" FROM
"auth_user"`, and note that every separator between tokens is all single
space.

However, any query which has limit/offset clause produces double spaces
before limit/offset clause. For example:

{{{
User.objects.values('id')[1:2].query.__str__()
}}}

returns
{{{
SELECT "auth_user"."id" FROM "auth_user" LIMIT 1 OFFSET 1
}}}
not
{{{
SELECT "auth_user"."id" FROM "auth_user" LIMIT 1 OFFSET 1
}}}

These queries are executed as well as expected but seem not a good
practice for formatting SQL as a string.

--
Ticket URL: <https://code.djangoproject.com/ticket/30242>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Mar 7, 2019, 11:48:05 PM3/7/19
to django-...@googlegroups.com
#30242: Double spaces before limit/offset clause in as_sql() of SQLCompiler
-------------------------------------+-------------------------------------
Reporter: hangpark | Owner: hangpark
Type: | Status: assigned
Cleanup/optimization |
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: sql | Triage Stage:
databaseoperations sqlcompiler | Unreviewed
Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by hangpark):

* owner: nobody => hangpark
* status: new => assigned


--
Ticket URL: <https://code.djangoproject.com/ticket/30242#comment:1>

Django

unread,
Mar 8, 2019, 1:10:55 AM3/8/19
to django-...@googlegroups.com
#30242: Double spaces before limit/offset clause in as_sql() of SQLCompiler
-------------------------------------+-------------------------------------
Reporter: hangpark | Owner: hangpark
Type: | Status: assigned
Cleanup/optimization |
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: sql | Triage Stage:
databaseoperations sqlcompiler | Unreviewed
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by hangpark):

* has_patch: 0 => 1


Comment:

PR open at https://github.com/django/django/pull/11061

--
Ticket URL: <https://code.djangoproject.com/ticket/30242#comment:2>

Django

unread,
Mar 8, 2019, 6:30:26 AM3/8/19
to django-...@googlegroups.com
#30242: Double spaces before limit/offset clause in as_sql() of SQLCompiler
-------------------------------------+-------------------------------------
Reporter: hangpark | Owner: hangpark
Type: | Status: assigned
Cleanup/optimization |
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: sql | Triage Stage: Ready for
databaseoperations sqlcompiler | checkin
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Carlton Gibson):

* stage: Unreviewed => Ready for checkin


Comment:

OK, given that you've patched this striaght-away, very happy to accept.
Looks good.
(Welcome aboard!)

--
Ticket URL: <https://code.djangoproject.com/ticket/30242#comment:3>

Django

unread,
Mar 9, 2019, 3:33:25 PM3/9/19
to django-...@googlegroups.com
#30242: Double spaces before limit/offset clause in as_sql() of SQLCompiler
-------------------------------------+-------------------------------------
Reporter: Hang Park | Owner: Hang Park
Type: | Status: closed

Cleanup/optimization |
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution: fixed

Keywords: sql | Triage Stage: Ready for
databaseoperations sqlcompiler | checkin
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak <felisiak.mariusz@…>):

* status: assigned => closed
* resolution: => fixed


Comment:

In [changeset:"142e1ead76fd452dc9bca0ab0f12bad56a116fb5" 142e1ead]:
{{{
#!CommitTicketReference repository=""
revision="142e1ead76fd452dc9bca0ab0f12bad56a116fb5"
Fixed #30242 -- Removed extra space before LIMIT/OFFSET SQL.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/30242#comment:4>

Reply all
Reply to author
Forward
0 new messages