[Django] #12192: Don't execute sql query with limit 0

0 views
Skip to first unread message

Django

unread,
Nov 9, 2009, 2:48:57 AM11/9/09
to djang...@holovaty.com, django-...@googlegroups.com
#12192: Don't execute sql query with limit 0
------------------------------------------+---------------------------------
Reporter: Suor | Owner: nobody
Status: new | Milestone:
Component: Database layer (models, ORM) | Version: 1.1
Keywords: | Stage: Unreviewed
Has_patch: 0 |
------------------------------------------+---------------------------------
When QuerySet.query has low_mark == high_mark (happens in pagination for
example) ORM executes a query like:
{{{
select ... from ... limit 0
}}}

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

Django

unread,
Dec 10, 2009, 6:27:56 AM12/10/09
to djang...@holovaty.com, django-...@googlegroups.com
#12192: Don't execute sql query with limit 0
---------------------------------------------------+------------------------
Reporter: Suor | Owner: ramiro
Status: new | Milestone:
Component: Database layer (models, ORM) | Version: 1.1
Resolution: | Keywords:
Stage: Design decision needed | Has_patch: 1
Needs_docs: 0 | Needs_tests: 0
Needs_better_patch: 0 |
---------------------------------------------------+------------------------
Changes (by ramiro):

* needs_better_patch: => 0
* needs_tests: => 0
* owner: nobody => ramiro
* needs_docs: => 0
* has_patch: 0 => 1
* stage: Unreviewed => Design decision needed

Comment:

I've attached two patches that implement detection of the `high_mark ==
low_mark` condition and avoid going to the DB with such a query, the
second patch could be even slightly more efficient because it shortcuts
execution of `Query.as_sql()` early completely avoiding the setup and
generation of the SQL query text.

One thing to review and decide is that isn't clear from the docstrings
and comments what `Query.as_sql()` is supposed to return to signal no SQL
query text was generated. The patches currently return an empty string but
I don't know now if it should be None.

Tests that exercise this already exist in `modeltests/basic` and
`regressiontests/queries`.

--
Ticket URL: <http://code.djangoproject.com/ticket/12192#comment:1>

Django

unread,
Jan 10, 2010, 11:09:28 AM1/10/10
to djang...@holovaty.com, django-...@googlegroups.com
#12192: Don't execute sql query with limit 0
---------------------------------------------------+------------------------
Reporter: Suor | Owner: ramiro
Status: new | Milestone:
Component: Database layer (models, ORM) | Version: SVN
Resolution: | Keywords:
Stage: Design decision needed | Has_patch: 1
Needs_docs: 0 | Needs_tests: 0
Needs_better_patch: 0 |
---------------------------------------------------+------------------------
Changes (by ramiro):

* version: 1.1 => SVN

Comment:

I've updated the patches for this ticket to trunk status after the multi-
db merge.

If/once one of the are chosen and applied to trunk, then the similarly
named chosen among `12192-patch1.diff` and `12192-patch2.diff` could be
applied to `1.1.x` as they should still apply correctly there.

--
Ticket URL: <http://code.djangoproject.com/ticket/12192#comment:2>

Django

unread,
Feb 23, 2010, 8:55:17 AM2/23/10
to djang...@holovaty.com, django-...@googlegroups.com
#12192: Don't execute sql query with limit 0
---------------------------------------------------+------------------------
Reporter: Suor | Owner: ramiro
Status: new | Milestone:
Component: Database layer (models, ORM) | Version: SVN
Resolution: | Keywords:
Stage: Design decision needed | Has_patch: 1
Needs_docs: 0 | Needs_tests: 0
Needs_better_patch: 0 |
---------------------------------------------------+------------------------
Comment (by ramiro):

Patches were fixed and updated to r12492. Both of them were tested by
running the Django test suite against MySQL, PostgreSQL and sqlite3 and it
was verified none of them introduced errors.

--
Ticket URL: <http://code.djangoproject.com/ticket/12192#comment:3>
Reply all
Reply to author
Forward
0 new messages