Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#30727 closed Bug (fixed)

Pickling a QuerySet evaluates the querysets given to Subquery in annotate.

Reported by: Andrew Brown Owned by: Andrew Brown
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I wrote a test case for tests/queryset_pickle/tests.py modeled after the test from bug #27499 which is very similar.

    def test_pickle_subquery_queryset_not_evaluated(self):
        """
        Verifies that querysets passed into Subquery expressions
        are not evaluated when pickled
        """
        groups = Group.objects.annotate(
            has_event=models.Exists(Event.objects.filter(group_id=models.OuterRef('id')))
        )
        with self.assertNumQueries(0):
            pickle.loads(pickle.dumps(groups.query))

The Subquery class (which is the base for Exists) only stores the underlying query object and throws the QuerySet away (as of this commit, although I don't think it worked before that). So in theory it shouldn't be pickling the QuerySet.

However, the QuerySet is still stored on the instance within the _constructor_args attribute added by the @deconstructible decorator on the BaseExpression base class. So when the inner query object gets pickled, so does the QuerySet, which attempts to evaluate it. In this case, it gets the error "ValueError: This queryset contains a reference to an outer query and may only be used in a subquery." since the inner queryset is being evaluated independently when called from pickle: it calls QuerySet.__getstate__().

I'm not sure what the best solution is here. I made a patch that adds the following override to __getstate__ to the SubQuery class, which fixes the problem and passes all other Django tests on my machine. I'll submit a PR shortly, but welcome any better approaches since I'm not sure what else that may effect.

class Subquery(Expression):
    ...
    def __getstate__(self):
        obj_dict = super().__getstate__()
        obj_dict.pop('_constructor_args', None)
        return obj_dict

Change History (8)

comment:2 by Andrew Brown, 5 years ago

Type: UncategorizedBug

comment:3 by Mariusz Felisiak, 5 years ago

Owner: changed from nobody to Andrew Brown
Status: newassigned
Summary: Pickling a Query object evaluates querysets in Subquery expressionsPickling a QuerySet evaluates the querysets given to Subquery in annotate.
Triage Stage: UnreviewedAccepted
Version: 2.2master

comment:4 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

Resolution: fixed
Status: assignedclosed

In 691def1:

Fixed #30727 -- Made Subquery pickle without evaluating their QuerySet.

Subquery expression objects, when pickled, were evaluating the QuerySet
objects saved in its _constructor_args attribute.

comment:5 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

In adfbf653:

Fixed #31568 -- Fixed alias reference when aggregating over multiple subqueries.

691def10a0197d83d2d108bd9043b0916d0f09b4 made all Subquery() instances
equal to each other which broke aggregation subquery pushdown which
relied on object equality to determine which alias it should select.

Subquery.eq() will be fixed in an another commit but
Query.rewrite_cols() should haved used object identity from the start.

Refs #30727, #30188.

Thanks Makina Corpus for the report.

comment:6 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

In 3913acdb:

[3.1.x] Fixed #31568 -- Fixed alias reference when aggregating over multiple subqueries.

691def10a0197d83d2d108bd9043b0916d0f09b4 made all Subquery() instances
equal to each other which broke aggregation subquery pushdown which
relied on object equality to determine which alias it should select.

Subquery.eq() will be fixed in an another commit but
Query.rewrite_cols() should haved used object identity from the start.

Refs #30727, #30188.

Thanks Makina Corpus for the report.

Backport of adfbf653dc1c1d0e0dacc4ed46602d22ba28b004 from master

comment:7 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

In 49bbf657:

[3.0.x] Fixed #31568 -- Fixed alias reference when aggregating over multiple subqueries.

691def10a0197d83d2d108bd9043b0916d0f09b4 made all Subquery() instances
equal to each other which broke aggregation subquery pushdown which
relied on object equality to determine which alias it should select.

Subquery.eq() will be fixed in an another commit but
Query.rewrite_cols() should haved used object identity from the start.

Refs #30727, #30188.

Thanks Makina Corpus for the report.

Backport of adfbf653dc1c1d0e0dacc4ed46602d22ba28b004 from master

comment:8 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

In b739f2e9:

Refs #30727 -- Added tests for Subquery with queryset in kwargs pickle without evaluating it.

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