Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#19426 closed Bug (fixed)

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

Reported by: anonymous Owned by: Claude Paroz
Component: Database layer (models, ORM) Version: master
Severity: Normal Keywords:
Cc: Benjamin Dauvergne, 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 Hong Shuning 4 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 Changed 4 years ago by Benjamin Dauvergne

Cc: Benjamin Dauvergne added
Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset

comment:2 Changed 4 years ago by Preston Holmes

Component: UncategorizedDatabase layer (models, ORM)
Easy pickings: set
Triage Stage: UnreviewedAccepted
Type: UncategorizedCleanup/optimization

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

comment:3 Changed 4 years ago by Claude Paroz

Owner: changed from nobody to Claude Paroz
Type: Cleanup/optimizationBug

bdauvergne, you are the reporter, right?

Changed 4 years ago by Hong Shuning

Attachment: 19426.diff added

comment:4 Changed 4 years ago by Hong Shuning

Cc: hongshuning@… added
Has patch: set

I have fixed this bug, please check.

comment:5 Changed 4 years ago by Simon Charette

Triage Stage: AcceptedReady 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 4 years ago by Anssi Kääriäinen

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

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

Resolution: fixed
Status: newclosed

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 4 years ago by Claude Paroz

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

comment:9 Changed 4 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