Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#19426 closed Bug (fixed)

EmptyQuerySet.distinct does not have the same signature as QuerySet.distinct()

Reported by: anonymous Owned by: claudep
Component: Database layer (models, ORM) Version: master
Severity: Normal Keywords:
Cc: bdauvergne, hongshuning@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

QuerySet.distinct is:

    def distinct(self, *field_names):

but EmptyQuerySet.distinct is :

    def distinct(self, fields=None):

when called like this:

    qs = qs.distinct('field1', 'field2')

if qs is empty then it raises:

TypeError: distinct() takes at most 2 arguments (3 given)

Attachments (1)

19426.diff (1.1 KB) - added by hongshuning 2 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 Changed 2 years ago by bdauvergne

  • Cc bdauvergne added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 2 years ago by ptone

  • Component changed from Uncategorized to Database layer (models, ORM)
  • Easy pickings set
  • Triage Stage changed from Unreviewed to Accepted
  • Type changed from Uncategorized to Cleanup/optimization

Easy enough to keep the fields kwarg while allowing *args as this is essentially a noop for EmptyQuerySet

comment:3 Changed 2 years ago by claudep

  • Owner changed from nobody to claudep
  • Type changed from Cleanup/optimization to Bug

bdauvergne, you are the reporter, right?

Changed 2 years ago by hongshuning

comment:4 Changed 2 years ago by hongshuning

  • Cc hongshuning@… added
  • Has patch set

I have fixed this bug, please check.

comment:5 Changed 2 years ago by charettes

  • Triage Stage changed from Accepted to Ready for checkin

@claudep, looking at #6422 it seems the chosen approach was ignoring the fields kwarg altogether. I think we should stick to that here also.

All queries tests pass on Python 2.7.3 SQlite so I'm marking as RFC.

comment:6 Changed 2 years ago by akaariai

There is another ticket which does a larger refactor of the emptyqs class, see https://code.djangoproject.com/ticket/19184

comment:7 Changed 2 years ago by Claude Paroz <claude@…>

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

In a893ee3315d6a134b46ff8c36c44a8efbbe34886:

Fixed #19426 -- Adapted EmptyQuerySet.distinct signature

1.5-only change, as EmptyQuerySet will be refactored in 1.6.
Thanks hongshuning@… for the patch.

comment:8 Changed 2 years ago by claudep

I committed the fix without the test, as #19184 will probably remove this part anyway.

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

In 69a46c5ca7d4e6819096af88cd8d51174efd46df:

Tests for various emptyqs tickets

The tickets are either about different signature between qs.none() and
qs or problems with subclass types (either EmptyQS overrided the custom
qs class, or EmptyQS was overridden by another class - values() did
this).

Fixed #15959, fixed #17271, fixed #17712, fixed #19426

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