Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#29118 closed Bug (fixed)

QuerySet.order_by(Exists(...)) crashes

Reported by: Raphael Gaschignard Owned by: nobody
Component: Database layer (models, ORM) Version: 2.0
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

In BaseExpression we have the following:

    def resolve_expression(self, query=None, allow_joins=True, reuse=None, summarize=False, for_save=False):
          ...

Here we have a couple default arguments. A decent number of callsites for this method only provide positional arguments. The Exists subclass assumes keyword-only arguments:

    def resolve_expression(self, query=None, **kwargs):
        # As a performance optimization, remove ordering since EXISTS doesn't
        # care about it, just whether or not a row matches.
        self.queryset = self.queryset.order_by()
        return super(Exists, self).resolve_expression(query, **kwargs)

I've submitted a patch (https://github.com/django/django/pull/9671) that adds support for positional arguments for Exists.resolve_expression, but perhaps the intention was for the base method to be keyword-only.

Change History (3)

comment:1 by Tim Graham, 6 years ago

Has patch: set
Summary: Uncertainty in proper signature for resolve_expression methodQuerySet.order_by(Exists(...)) crashes
Triage Stage: UnreviewedReady for checkin

comment:2 by Tim Graham <timograham@…>, 6 years ago

Resolution: fixed
Status: newclosed

In bf26f66:

Fixed #29118 -- Fixed crash with QuerySet.order_by(Exists(...)).

comment:3 by Tim Graham <timograham@…>, 6 years ago

In 9b5ba216:

[2.0.x] Fixed #29118 -- Fixed crash with QuerySet.order_by(Exists(...)).

Backport of bf26f66029bca94b007a2452679ac004598364a6 from master

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