Skip to content

Replace deepcopy of the Q object #543

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion polymorphic/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ def __new__(self, model_name, bases, attrs, **kwargs):
# determine the name of the primary key field and store it into the class variable
# polymorphic_primary_key_name (it is needed by query.py)
for f in new_class._meta.fields:
if f.primary_key and type(f) != models.OneToOneField:
if f.primary_key and type(f) is not models.OneToOneField:
new_class.polymorphic_primary_key_name = f.name
break

Expand Down
2 changes: 1 addition & 1 deletion polymorphic/query.py
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ def tree_node_test___lookup(my_model, node):
for i in range(len(node.children)):
child = node.children[i]

if type(child) == tuple:
if type(child) is tuple:
# this Q object child is a tuple => a kwarg like Q( instance_of=ModelB )
assert "___" not in child[0], ___lookup_assert_msg
else:
Expand Down
33 changes: 31 additions & 2 deletions polymorphic/query_translate.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
PolymorphicQuerySet support functions
"""

import copy
from collections import deque

from django.apps import apps
Expand Down Expand Up @@ -80,6 +79,35 @@ def tree_node_correct_field_specs(my_model, node):
return potential_q_object


def _deepcopy_q_object(q):
"""
Make a deepcopy of a Q-object.
"""

def _copy_child(child):
if isinstance(child, tuple):
return child # tuples are immutable, no need to make a copy.
elif isinstance(child, Q):
return _deepcopy_q_object(child)
else:
raise RuntimeError("Unknown child type: %s", type(child))

children = [_copy_child(c) for c in q.children]

if hasattr(q, "copy"): # Django 4.2+
obj = q.copy()
# This assignment of obj.children from children (created above)
# is required for test_query_filter_exclude_is_immutable to pass
# because somehow the capitalization changed for q_to_reuse?
#
# assert q_to_reuse.children == untouched_q_object.children
# AssertionError: assert [('model2b__f... 'something')] == [('Model2B___... 'something')]
obj.children = children
else:
obj = Q(*children, _connector=q.connector, _negated=q.negated)
return obj


def translate_polymorphic_filter_definitions_in_args(queryset_model, args, using=DEFAULT_DB_ALIAS):
"""
Translate the non-keyword argument list for PolymorphicQuerySet.filter()
Expand All @@ -92,7 +120,8 @@ def translate_polymorphic_filter_definitions_in_args(queryset_model, args, using
Returns: modified Q objects
"""
return [
translate_polymorphic_Q_object(queryset_model, copy.deepcopy(q), using=using) for q in args
translate_polymorphic_Q_object(queryset_model, _deepcopy_q_object(q), using=using)
for q in args
]


Expand Down
2 changes: 1 addition & 1 deletion polymorphic/showfields.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ def _showfields_add_regular_fields(self, parts):
out = field.name

# if this is the standard primary key named "id", print it as we did with older versions of django_polymorphic
if field.primary_key and field.name == "id" and type(field) == models.AutoField:
if field.primary_key and field.name == "id" and type(field) is models.AutoField:
out += f" {getattr(self, field.name)}"

# otherwise, display it just like all other fields (with correct type, shortened content etc.)
Expand Down
52 changes: 52 additions & 0 deletions polymorphic/tests/test_query_translate.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import copy
import tempfile
import pickle
import threading

from django.db.models import Q
from django.test import TestCase

from polymorphic.tests.models import Bottom, Middle, Top
from polymorphic.query_translate import translate_polymorphic_filter_definitions_in_args


class QueryTranslateTests(TestCase):
def test_translate_with_not_pickleable_query(self):
"""
In some cases, Django may attacha _thread object to the query and we
will get the following when we try to deepcopy inside of
translate_polymorphic_filter_definitions_in_args:

TypeError: cannot pickle '_thread.lock' object


For this to trigger, we need to somehoe go down this path:

File "/perfdash/.venv/lib64/python3.12/site-packages/polymorphic/query_translate.py", line 95, in translate_polymorphic_filter_definitions_in_args
translate_polymorphic_Q_object(queryset_model, copy.deepcopy(q), using=using) for q in args
^^^^^^^^^^^^^^^^
File "/usr/lib64/python3.12/copy.py", line 143, in deepcopy
y = copier(memo)
^^^^^^^^^^^^
File "/perfdash/.venv/lib64/python3.12/site-packages/django/utils/tree.py", line 53, in __deepcopy__
obj.children = copy.deepcopy(self.children, memodict)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib64/python3.12/copy.py", line 136, in deepcopy
y = copier(x, memo)
^^^^^^^^^^^^^^^

Internals in Django, somehow we must trigger this tree.py code in django via
the deepcopy in order to trigger this.

"""

with tempfile.TemporaryFile() as fd:
# verify this is definitely not pickleable
with self.assertRaises(TypeError):
pickle.dumps(threading.Lock())

# I know this doesn't make sense to pass as a Q(), but
# I haven't found another way to trigger the copy.deepcopy failing.
q = Q(blog__info="blog info") | Q(blog__info=threading.Lock())

translate_polymorphic_filter_definitions_in_args(Bottom, args=[q])