Opened 3 years ago

Closed 2 years ago

#18414 closed Bug (fixed)

queryset.exists() returns False when queryset is distinct and sliced

Reported by: bitrut Owned by: err
Component: Database layer (models, ORM) Version: 1.4
Severity: Normal Keywords: orm
Cc: paluho@…, bitrut@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

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.

Change History (9)

comment:1 Changed 3 years ago by paluho@…

  • Cc paluho@… added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 3 years ago by lukeplant

  • Triage Stage changed from Unreviewed to Accepted

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.

comment:3 Changed 3 years ago by bitrut@…

  • Cc bitrut@… added

comment:4 Changed 3 years ago by err

  • Owner changed from nobody to err
  • Status changed from new to assigned

comment:5 Changed 3 years ago by err

  • Has patch set

comment:6 Changed 3 years ago 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?

comment:7 Changed 2 years ago by timo

  • Patch needs improvement set

comment:8 Changed 2 years ago 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.

comment:9 Changed 2 years ago by Anssi Kääriäinen <akaariai@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

In 93cc6dcdac6fc3e506640fa38dd1798c3cd61cff:

Fixed #18414 -- qs.exists() for sliced distinct queries

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