[Django] #18414: queryset.exists() returns False when queryset is distinct and sliced
Django
----------------------------------------------+--------------------
Reporter: bitrut | Owner: nobody
Type: Bug | Status: new
Component: Database layer (models, ORM) | Version: 1.4
Severity: Normal | Keywords: orm
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------------------+--------------------
Consider this:
{{{
User.objects.count()
3
}}}
This queryset
{{{
User.objects.all()[1:].exists()
True
}}}
produces SQL
{{{
SELECT (1) AS "a" FROM "user" LIMIT 1 OFFSET 1
}}}
so the result is correct.
But when you add `distinct()`:
{{{
User.objects.distinct()[1:].exists()
False
}}}
the SQL is
{{{
SELECT DISTINCT (1) AS "a" FROM "user" LIMIT 1 OFFSET 1
}}}
which returns nothing, so the `exists()` returns `False` which is
incorrect.
`DISTINCT` narrows results to just one '''1''' and `OFFSET` omits the
first result and goes to the next one which does not exist.
It's because `DISTINCT` has the higher priority than `OFFSET` in SQL.
I don't know the perfect solution. Maybe in the case of using
`distinct()`, slicing and `exists()` together an exception should be
thrown. Or the `exists()` function should notice this case and call
`count()`, but this would lead to unexpected DB usage.
--
Ticket URL: <https://code.djangoproject.com/ticket/18414>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Type: Bug | Status: new
(models, ORM) | Resolution:
Severity: Normal | Triage Stage:
Keywords: orm | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Changes (by paluho@…):
* cc: paluho@… (added)
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/18414#comment:1>
Django
Type: Bug | Status: new
(models, ORM) | Resolution:
Keywords: orm | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by lukeplant):
* stage: Unreviewed => Accepted
Comment:
`QuerySet.exists()` should throw an exception for this case, and possibly
for any case involving slicing - I don't think it was designed to be
correct in the presence of LIMIT/OFFSET. However, for the moment probably
best to limit to the distinct()+slicing()+exists() combination.
--
Ticket URL: <https://code.djangoproject.com/ticket/18414#comment:2>
Django
Type: Bug | Status: new
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: orm | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
* cc: bitrut@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/18414#comment:3>
Django
Reporter: bitrut | Owner: err
Type: Bug | Status: assigned
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: orm | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
* owner: nobody => err
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/18414#comment:4>
Django
Reporter: bitrut | Owner: err
Type: Bug | Status: assigned
Component: Database layer | Version: 1.4
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: orm | Needs documentation: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by err):
* has_patch: 0 => 1
Comment:
Here is my pull request: https://github.com/django/django/pull/161
--
Ticket URL: <https://code.djangoproject.com/ticket/18414#comment:5>
Django
Reporter: bitrut | Owner: err
Type: Bug | Status: assigned
Component: Database layer | Version: 1.4
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: orm | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by akaariai):
The pull request doesn't seem to do what Luke asked - it disallows the
usage in any filtered situation, not just in conjunction with .distinct().
It seems likely there are users relying on this currently.
As for fixing the distinct() + limit + .exists() - it should be done by
not doing select distinct(1) but instead select distinct {normal select
fields} instead. I wonder if this would be straightforward to do?
--
Ticket URL: <https://code.djangoproject.com/ticket/18414#comment:6>
Django
Reporter: bitrut | Owner: err
Type: Bug | Status: assigned
Component: Database layer | Version: 1.4
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: orm | Needs documentation: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/18414#comment:7>
Django
Reporter: bitrut | Owner: err
Type: Bug | Status: assigned
Component: Database layer | Version: 1.4
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: orm | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 1
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by akaariai):
It seems this will be relatively easy to fix, just do
{{{
if not q.distinct:
q.clear_select_clause()
}}}
instead of always doing clear_select_clause(). I'll test this & commit if
things seem to work OK.
--
Ticket URL: <https://code.djangoproject.com/ticket/18414#comment:8>
Django
Reporter: bitrut | Owner: err
Component: Database layer | Version: 1.4
Severity: Normal | Triage Stage: Accepted
Keywords: orm | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 1
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"93cc6dcdac6fc3e506640fa38dd1798c3cd61cff"]:
{{{
#!CommitTicketReference repository=""
revision="93cc6dcdac6fc3e506640fa38dd1798c3cd61cff"
Fixed #18414 -- qs.exists() for sliced distinct queries
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/18414#comment:9>
Django
Reporter: bitrut | Owner: err
Type: Bug | Status: closed
Component: Database layer | Version: 1.4
(models, ORM) |
Keywords: orm | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"d2263b7b87a2af5dcd6964937b157b7e1cf2881f" d2263b7b]:
{{{
#!CommitTicketReference repository=""
revision="d2263b7b87a2af5dcd6964937b157b7e1cf2881f"
Refs #18414 -- Added tests for selected columns of sliced distinct
querysets.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/18414#comment:10>