Refs #35356 -- Clarified select related with masked field logic.

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.
This commit is contained in:
Simon Charette 2024-04-05 23:20:41 -04:00 committed by nessita
parent 83f5478225
commit 195d885ca0
3 changed files with 49 additions and 51 deletions

View File

@ -340,38 +340,37 @@ class RegisterLookupMixin:
_unregister_class_lookup = classmethod(_unregister_class_lookup) _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 Return whether `field` should be used to descend deeper for
select_related() purposes. Used by both the query construction code `select_related()` purposes.
(compiler.get_related_selections()) and the model instance creation code
(compiler.klass_info).
Arguments: Arguments:
* field - the field to be checked * `field` - the field to be checked. Can be either a `Field` or
* restricted - a boolean field, indicating if the field list has been `ForeignObjectRel` instance.
manually restricted using a requested clause) * `restricted` - a boolean field, indicating if the field list has been
* requested - The select_related() dictionary. manually restricted using a select_related() clause.
* select_mask - the dictionary of selected fields. * `requested` - the select_related() dictionary.
* reverse - boolean, True if we are checking a reverse select related * `select_mask` - the dictionary of selected fields.
""" """
# Only relationships can be descended.
if not field.remote_field: if not field.remote_field:
return False 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 return False
if restricted: # When `select_related()` is used without a `*requested` mask all
if reverse and field.related_query_name() not in requested: # 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 return False
if not reverse and field.name not in requested: # Prevent invalid usages of `select_related()` and `only()`/`defer()` such
return False # as `select_related("a").only("b")` and `select_related("a").defer("a")`.
if not restricted and field.null: if select_mask and field not in select_mask:
return False
if (
restricted
and select_mask
and field.name in requested
and field not in select_mask
):
raise FieldError( raise FieldError(
f"Field {field.model._meta.object_name}.{field.name} cannot be both " f"Field {field.model._meta.object_name}.{field.name} cannot be both "
"deferred and traversed using select_related at the same time." "deferred and traversed using select_related at the same time."

View File

@ -1259,11 +1259,10 @@ class SQLCompiler:
] ]
for related_object, related_field, model in related_fields: for related_object, related_field, model in related_fields:
if not select_related_descend( if not select_related_descend(
related_field, related_object,
restricted, restricted,
requested, requested,
select_mask, select_mask,
reverse=True,
): ):
continue continue
@ -1280,7 +1279,7 @@ class SQLCompiler:
"model": model, "model": model,
"field": related_field, "field": related_field,
"reverse": True, "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, "remote_setter": related_field.set_cached_value,
"from_parent": from_parent, "from_parent": from_parent,
} }
@ -1296,7 +1295,7 @@ class SQLCompiler:
select_fields.append(len(select)) select_fields.append(len(select))
select.append((col, None)) select.append((col, None))
klass_info["select_fields"] = select_fields 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( next_klass_infos = self.get_related_selections(
select, select,
related_select_mask, related_select_mask,

View File

@ -791,44 +791,44 @@ class Query(BaseExpression):
if select_mask is None: if select_mask is None:
select_mask = {} select_mask = {}
select_mask[opts.pk] = {} select_mask[opts.pk] = {}
# All concrete fields that are not part of the defer mask must be # All concrete fields and related objects that are not part of the
# loaded. If a relational field is encountered it gets added to the # defer mask must be included. If a relational field is encountered it
# mask for it be considered if `select_related` and the cycle continues # gets added to the mask for it be considered if `select_related` and
# by recursively calling this function. # the cycle continues by recursively calling this function.
for field in opts.concrete_fields: for field in opts.concrete_fields + opts.related_objects:
field_mask = mask.pop(field.name, None) 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: if field_mask is None and field_att_mask is None:
select_mask.setdefault(field, {}) select_mask.setdefault(field, {})
elif field_mask: elif field_mask:
if not field.is_relation: if not field.is_relation:
raise FieldError(next(iter(field_mask))) 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, {}) 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( self._get_defer_select_mask(
related_model._meta, field_mask, field_select_mask related_model._meta, field_mask, field_select_mask
) )
# Remaining defer entries must be references to reverse relationships. # Remaining defer entries must be references to filtered relations
# The following code is expected to raise FieldError if it encounters # otherwise they are surfaced as missing field errors.
# a malformed defer entry.
for field_name, field_mask in mask.items(): for field_name, field_mask in mask.items():
if filtered_relation := self._filtered_relations.get(field_name): if filtered_relation := self._filtered_relations.get(field_name):
relation = opts.get_field(filtered_relation.relation_name) relation = opts.get_field(filtered_relation.relation_name)
field_select_mask = select_mask.setdefault((field_name, relation), {}) field_select_mask = select_mask.setdefault((field_name, relation), {})
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 related_model = relation.related_model._meta.concrete_model
self._get_defer_select_mask( self._get_defer_select_mask(
related_model._meta, field_mask, field_select_mask related_model._meta, field_mask, field_select_mask
) )
else:
opts.get_field(field_name)
return select_mask return select_mask
def _get_only_select_mask(self, opts, mask, select_mask=None): def _get_only_select_mask(self, opts, mask, select_mask=None):