Skip to content

Commit

Permalink
Refs #35356 -- Clarified select related with masked field logic.
Browse files Browse the repository at this point in the history
By always including related objects in the select mask via adjusting the
defer logic (_get_defer_select_mask()), it becomes possible for
select_related_descend() to treat forward and reverse relationships
indistinctively.

This work also simplifies and adds comments to
select_related_descend() to make it easier to understand.
  • Loading branch information
charettes authored and nessita committed Apr 23, 2024
1 parent 83f5478 commit 195d885
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 51 deletions.
47 changes: 23 additions & 24 deletions django/db/models/query_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -340,38 +340,37 @@ def _unregister_instance_lookup(self, lookup, lookup_name=None):
_unregister_class_lookup = classmethod(_unregister_class_lookup)


def select_related_descend(field, restricted, requested, select_mask, reverse=False):
def select_related_descend(field, restricted, requested, select_mask):
"""
Return True if this field should be used to descend deeper for
select_related() purposes. Used by both the query construction code
(compiler.get_related_selections()) and the model instance creation code
(compiler.klass_info).
Return whether `field` should be used to descend deeper for
`select_related()` purposes.
Arguments:
* field - the field to be checked
* restricted - a boolean field, indicating if the field list has been
manually restricted using a requested clause)
* requested - The select_related() dictionary.
* select_mask - the dictionary of selected fields.
* reverse - boolean, True if we are checking a reverse select related
* `field` - the field to be checked. Can be either a `Field` or
`ForeignObjectRel` instance.
* `restricted` - a boolean field, indicating if the field list has been
manually restricted using a select_related() clause.
* `requested` - the select_related() dictionary.
* `select_mask` - the dictionary of selected fields.
"""
# Only relationships can be descended.
if not field.remote_field:
return False
if field.remote_field.parent_link and not reverse:
# Forward MTI parent links should not be explicitly descended as they are
# always JOIN'ed against (unless excluded by `select_mask`).
if getattr(field.remote_field, "parent_link", False):
return False
if restricted:
if reverse and field.related_query_name() not in requested:
return False
if not reverse and field.name not in requested:
return False
if not restricted and field.null:
# When `select_related()` is used without a `*requested` mask all
# relationships are descended unless they are nullable.
if not restricted:
return not field.null
# When `select_related(*requested)` is used only fields that are part of
# `requested` should be descended.
if field.name not in requested:
return False
if (
restricted
and select_mask
and field.name in requested
and field not in select_mask
):
# Prevent invalid usages of `select_related()` and `only()`/`defer()` such
# as `select_related("a").only("b")` and `select_related("a").defer("a")`.
if select_mask and field not in select_mask:
raise FieldError(
f"Field {field.model._meta.object_name}.{field.name} cannot be both "
"deferred and traversed using select_related at the same time."
Expand Down
7 changes: 3 additions & 4 deletions django/db/models/sql/compiler.py
Original file line number Diff line number Diff line change
Expand Up @@ -1259,11 +1259,10 @@ def get_related_klass_infos(klass_info, related_klass_infos):
]
for related_object, related_field, model in related_fields:
if not select_related_descend(
related_field,
related_object,
restricted,
requested,
select_mask,
reverse=True,
):
continue

Expand All @@ -1280,7 +1279,7 @@ def get_related_klass_infos(klass_info, related_klass_infos):
"model": model,
"field": related_field,
"reverse": True,
"local_setter": related_field.remote_field.set_cached_value,
"local_setter": related_object.set_cached_value,
"remote_setter": related_field.set_cached_value,
"from_parent": from_parent,
}
Expand All @@ -1296,7 +1295,7 @@ def get_related_klass_infos(klass_info, related_klass_infos):
select_fields.append(len(select))
select.append((col, None))
klass_info["select_fields"] = select_fields
next = requested.get(related_field.related_query_name(), {})
next = requested.get(related_field_name, {})
next_klass_infos = self.get_related_selections(
select,
related_select_mask,
Expand Down
46 changes: 23 additions & 23 deletions django/db/models/sql/query.py
Original file line number Diff line number Diff line change
Expand Up @@ -791,44 +791,44 @@ def _get_defer_select_mask(self, opts, mask, select_mask=None):
if select_mask is None:
select_mask = {}
select_mask[opts.pk] = {}
# All concrete fields that are not part of the defer mask must be
# loaded. If a relational field is encountered it gets added to the
# mask for it be considered if `select_related` and the cycle continues
# by recursively calling this function.
for field in opts.concrete_fields:
# All concrete fields and related objects that are not part of the
# defer mask must be included. If a relational field is encountered it
# gets added to the mask for it be considered if `select_related` and
# the cycle continues by recursively calling this function.
for field in opts.concrete_fields + opts.related_objects:
field_mask = mask.pop(field.name, None)
field_att_mask = mask.pop(field.attname, None)
field_att_mask = None
if field_attname := getattr(field, "attname", None):
field_att_mask = mask.pop(field_attname, None)
if field_mask is None and field_att_mask is None:
select_mask.setdefault(field, {})
elif field_mask:
if not field.is_relation:
raise FieldError(next(iter(field_mask)))
# Virtual fields such as many-to-many and generic foreign keys
# cannot be effectively deferred. Historically, they were
# allowed to be passed to QuerySet.defer(). Ignore such field
# references until a layer of validation at mask alteration
# time is eventually implemented.
if field.many_to_many:
continue
field_select_mask = select_mask.setdefault(field, {})
related_model = field.remote_field.model._meta.concrete_model
related_model = field.related_model._meta.concrete_model
self._get_defer_select_mask(
related_model._meta, field_mask, field_select_mask
)
# Remaining defer entries must be references to reverse relationships.
# The following code is expected to raise FieldError if it encounters
# a malformed defer entry.
# Remaining defer entries must be references to filtered relations
# otherwise they are surfaced as missing field errors.
for field_name, field_mask in mask.items():
if filtered_relation := self._filtered_relations.get(field_name):
relation = opts.get_field(filtered_relation.relation_name)
field_select_mask = select_mask.setdefault((field_name, relation), {})
related_model = relation.related_model._meta.concrete_model
self._get_defer_select_mask(
related_model._meta, field_mask, field_select_mask
)
else:
relation = opts.get_field(field_name)
# While virtual fields such as many-to-many and generic foreign
# keys cannot be effectively deferred we've historically
# allowed them to be passed to QuerySet.defer(). Ignore such
# field references until a layer of validation at mask
# alteration time will be implemented eventually.
if not hasattr(relation, "field"):
continue
field_select_mask = select_mask.setdefault(relation, {})
related_model = relation.related_model._meta.concrete_model
self._get_defer_select_mask(
related_model._meta, field_mask, field_select_mask
)
opts.get_field(field_name)
return select_mask

def _get_only_select_mask(self, opts, mask, select_mask=None):
Expand Down

0 comments on commit 195d885

Please sign in to comment.