From 9dff316be41847c505ebf397e4a52a0a71e0acc4 Mon Sep 17 00:00:00 2001 From: Nick Pope Date: Tue, 21 Sep 2021 00:49:16 +0100 Subject: [PATCH] Refs #32948, Refs #32946 -- Used Q.create() internally for dynamic Q() objects. Node.create() which has a compatible signature with Node.__init__() takes in a single `children` argument rather than relying in unpacking *args in Q.__init__() which calls Node.__init__(). In addition, we were often needing to unpack iterables into *args and can instead pass a list direct to Node.create(). --- django/contrib/admin/filters.py | 2 +- django/contrib/admin/options.py | 8 ++++---- django/contrib/contenttypes/fields.py | 14 ++++++++------ django/db/models/base.py | 4 ++-- django/db/models/deletion.py | 6 +++--- django/db/models/fields/related.py | 9 +++++---- django/db/models/fields/related_descriptors.py | 12 +++++++----- django/utils/tree.py | 2 +- tests/queries/test_q.py | 9 +++++++++ 9 files changed, 40 insertions(+), 26 deletions(-) diff --git a/django/contrib/admin/filters.py b/django/contrib/admin/filters.py index ec97605f14..dd6954a065 100644 --- a/django/contrib/admin/filters.py +++ b/django/contrib/admin/filters.py @@ -527,7 +527,7 @@ class EmptyFieldListFilter(FieldListFilter): lookup_conditions.append((self.field_path, "")) if self.field.null: lookup_conditions.append((f"{self.field_path}__isnull", True)) - lookup_condition = models.Q(*lookup_conditions, _connector=models.Q.OR) + lookup_condition = models.Q.create(lookup_conditions, connector=models.Q.OR) if self.lookup_val == "1": return queryset.filter(lookup_condition) return queryset.exclude(lookup_condition) diff --git a/django/contrib/admin/options.py b/django/contrib/admin/options.py index cb77048fe1..8ccacd6213 100644 --- a/django/contrib/admin/options.py +++ b/django/contrib/admin/options.py @@ -1146,12 +1146,12 @@ class ModelAdmin(BaseModelAdmin): for bit in smart_split(search_term): if bit.startswith(('"', "'")) and bit[0] == bit[-1]: bit = unescape_string_literal(bit) - or_queries = models.Q( - *((orm_lookup, bit) for orm_lookup in orm_lookups), - _connector=models.Q.OR, + or_queries = models.Q.create( + [(orm_lookup, bit) for orm_lookup in orm_lookups], + connector=models.Q.OR, ) term_queries.append(or_queries) - queryset = queryset.filter(models.Q(*term_queries)) + queryset = queryset.filter(models.Q.create(term_queries)) may_have_duplicates |= any( lookup_spawns_duplicates(self.opts, search_spec) for search_spec in orm_lookups diff --git a/django/contrib/contenttypes/fields.py b/django/contrib/contenttypes/fields.py index 193c2f2687..5b327b6ae4 100644 --- a/django/contrib/contenttypes/fields.py +++ b/django/contrib/contenttypes/fields.py @@ -627,17 +627,19 @@ def create_generic_related_manager(superclass, rel): queryset._add_hints(instance=instances[0]) queryset = queryset.using(queryset._db or self._db) # Group instances by content types. - content_type_queries = ( - models.Q( - (f"{self.content_type_field_name}__pk", content_type_id), - (f"{self.object_id_field_name}__in", {obj.pk for obj in objs}), + content_type_queries = [ + models.Q.create( + [ + (f"{self.content_type_field_name}__pk", content_type_id), + (f"{self.object_id_field_name}__in", {obj.pk for obj in objs}), + ] ) for content_type_id, objs in itertools.groupby( sorted(instances, key=lambda obj: self.get_content_type(obj).pk), lambda obj: self.get_content_type(obj).pk, ) - ) - query = models.Q(*content_type_queries, _connector=models.Q.OR) + ] + query = models.Q.create(content_type_queries, connector=models.Q.OR) # We (possibly) need to convert object IDs to the type of the # instances' PK in order to match up instances: object_id_converter = instances[0]._meta.pk.to_python diff --git a/django/db/models/base.py b/django/db/models/base.py index 04f045d923..1f7e5f84aa 100644 --- a/django/db/models/base.py +++ b/django/db/models/base.py @@ -1152,8 +1152,8 @@ class Model(metaclass=ModelBase): op = "gt" if is_next else "lt" order = "" if is_next else "-" param = getattr(self, field.attname) - q = Q((field.name, param), (f"pk__{op}", self.pk), _connector=Q.AND) - q = Q(q, (f"{field.name}__{op}", param), _connector=Q.OR) + q = Q.create([(field.name, param), (f"pk__{op}", self.pk)], connector=Q.AND) + q = Q.create([q, (f"{field.name}__{op}", param)], connector=Q.OR) qs = ( self.__class__._default_manager.using(self._state.db) .filter(**kwargs) diff --git a/django/db/models/deletion.py b/django/db/models/deletion.py index 6912b49498..2cb3c88444 100644 --- a/django/db/models/deletion.py +++ b/django/db/models/deletion.py @@ -399,9 +399,9 @@ class Collector: """ Get a QuerySet of the related model to objs via related fields. """ - predicate = query_utils.Q( - *((f"{related_field.name}__in", objs) for related_field in related_fields), - _connector=query_utils.Q.OR, + predicate = query_utils.Q.create( + [(f"{related_field.name}__in", objs) for related_field in related_fields], + connector=query_utils.Q.OR, ) return related_model._base_manager.using(self.using).filter(predicate) diff --git a/django/db/models/fields/related.py b/django/db/models/fields/related.py index bb4e065e37..32085daf18 100644 --- a/django/db/models/fields/related.py +++ b/django/db/models/fields/related.py @@ -407,12 +407,13 @@ class RelatedField(FieldCacheMixin, Field): select all instances of self.related_field.model related through this field to obj. obj is an instance of self.model. """ - base_filter = ( - (rh_field.attname, getattr(obj, lh_field.attname)) - for lh_field, rh_field in self.related_fields + base_q = Q.create( + [ + (rh_field.attname, getattr(obj, lh_field.attname)) + for lh_field, rh_field in self.related_fields + ] ) descriptor_filter = self.get_extra_descriptor_filter(obj) - base_q = Q(*base_filter) if isinstance(descriptor_filter, dict): return base_q & Q(**descriptor_filter) elif descriptor_filter: diff --git a/django/db/models/fields/related_descriptors.py b/django/db/models/fields/related_descriptors.py index f62a9170c0..b192df4fbf 100644 --- a/django/db/models/fields/related_descriptors.py +++ b/django/db/models/fields/related_descriptors.py @@ -1002,19 +1002,21 @@ def create_forward_many_to_many_manager(superclass, rel, reverse): do_not_call_in_templates = True def _build_remove_filters(self, removed_vals): - filters = Q((self.source_field_name, self.related_val)) + filters = Q.create([(self.source_field_name, self.related_val)]) # No need to add a subquery condition if removed_vals is a QuerySet without # filters. removed_vals_filters = ( not isinstance(removed_vals, QuerySet) or removed_vals._has_filters() ) if removed_vals_filters: - filters &= Q((f"{self.target_field_name}__in", removed_vals)) + filters &= Q.create([(f"{self.target_field_name}__in", removed_vals)]) if self.symmetrical: - symmetrical_filters = Q((self.target_field_name, self.related_val)) + symmetrical_filters = Q.create( + [(self.target_field_name, self.related_val)] + ) if removed_vals_filters: - symmetrical_filters &= Q( - (f"{self.source_field_name}__in", removed_vals) + symmetrical_filters &= Q.create( + [(f"{self.source_field_name}__in", removed_vals)] ) filters |= symmetrical_filters return filters diff --git a/django/utils/tree.py b/django/utils/tree.py index d897cbeacd..443e996700 100644 --- a/django/utils/tree.py +++ b/django/utils/tree.py @@ -33,7 +33,7 @@ class Node: __init__() with a signature that conflicts with the one defined in Node.__init__(). """ - obj = Node(children, connector, negated) + obj = Node(children, connector or cls.default, negated) obj.__class__ = cls return obj diff --git a/tests/queries/test_q.py b/tests/queries/test_q.py index 289305d33f..923846b5a3 100644 --- a/tests/queries/test_q.py +++ b/tests/queries/test_q.py @@ -216,6 +216,15 @@ class QTests(SimpleTestCase): flatten = list(q.flatten()) self.assertEqual(len(flatten), 7) + def test_create_helper(self): + items = [("a", 1), ("b", 2), ("c", 3)] + for connector in [Q.AND, Q.OR, Q.XOR]: + with self.subTest(connector=connector): + self.assertEqual( + Q.create(items, connector=connector), + Q(*items, _connector=connector), + ) + class QCheckTests(TestCase): def test_basic(self):