[Django] #30727: Pickling a Query object evaluates querysets in Subquery expressions
Django
-------------------------------------+-------------------------------------
Reporter: Andrew | Owner: nobody
Brown |
Type: | Status: new
Uncategorized |
Component: Database | Version: 2.2
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
I wrote a test case for `tests/queryset_pickle/tests.py` modeled after the
test from bug #27499 which is very similar.
{{{#!python
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
[https://github.com/django/django/commit/96b6ad94d9ebbd57b77b44e185ee215b5b899ac8
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.
{{{#!python
class Subquery(Expression):
...
def __getstate__(self):
obj_dict = super().__getstate__()
obj_dict.pop('_constructor_args', None)
return obj_dict
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30727>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Type: Uncategorized | Status: new
Component: Database layer | Version: 2.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
Comment:
PR: https://github.com/django/django/pull/11707
--
Ticket URL: <https://code.djangoproject.com/ticket/30727#comment:1>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 2.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* type: Uncategorized => Bug
--
Ticket URL: <https://code.djangoproject.com/ticket/30727#comment:2>
Django
-------------------------------------+-------------------------------------
Reporter: Andrew Brown | Owner: Andrew
| Brown
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => assigned
* owner: nobody => Andrew Brown
* version: 2.2 => master
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/30727#comment:3>
Django
-------------------------------------+-------------------------------------
Reporter: Andrew Brown | Owner: Andrew
| Brown
Component: Database layer | Version: master
(models, ORM) |
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"691def10a0197d83d2d108bd9043b0916d0f09b4" 691def1]:
{{{
#!CommitTicketReference repository=""
revision="691def10a0197d83d2d108bd9043b0916d0f09b4"
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.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30727#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: Andrew Brown | Owner: Andrew
| Brown
Type: Bug | Status: closed
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"adfbf653dc1c1d0e0dacc4ed46602d22ba28b004" adfbf653]:
{{{
#!CommitTicketReference repository=""
revision="adfbf653dc1c1d0e0dacc4ed46602d22ba28b004"
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.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30727#comment:5>
Django
-------------------------------------+-------------------------------------
Reporter: Andrew Brown | Owner: Andrew
| Brown
Type: Bug | Status: closed
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"3913acdb29d09109ec82ce789b592e6281aa7be9" 3913acdb]:
{{{
#!CommitTicketReference repository=""
revision="3913acdb29d09109ec82ce789b592e6281aa7be9"
[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
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30727#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: Andrew Brown | Owner: Andrew
| Brown
Type: Bug | Status: closed
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"49bbf6570d9f0880b836f741d79e4cdb6e061ea2" 49bbf657]:
{{{
#!CommitTicketReference repository=""
revision="49bbf6570d9f0880b836f741d79e4cdb6e061ea2"
[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
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30727#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: Andrew Brown | Owner: Andrew
| Brown
Type: Bug | Status: closed
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"b739f2e91d68aa3d4c5009127a43af037796225a" b739f2e9]:
{{{
#!CommitTicketReference repository=""
revision="b739f2e91d68aa3d4c5009127a43af037796225a"
Refs #30727 -- Added tests for Subquery with queryset in kwargs pickle
without evaluating it.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30727#comment:8>