More attempts to fix a bug.
This commit is contained in:
parent
bd2daf921a
commit
dd863dba99
3 changed files with 102 additions and 21 deletions
|
|
@ -145,16 +145,18 @@ def _split_field_tokens(fields: Iterable[str]) -> Tuple[List[str], Dict[str, Lis
|
||||||
def _deep_get_loaded(obj: Any, dotted: str) -> Any:
|
def _deep_get_loaded(obj: Any, dotted: str) -> Any:
|
||||||
"""
|
"""
|
||||||
Deep get with no lazy loads:
|
Deep get with no lazy loads:
|
||||||
- Walk intermediate hops via _safe_get_loaded_attr only.
|
- Intermediate hops via _safe_get_loaded_attr only.
|
||||||
- Final hop: prefer _safe_get_loaded_attr; if that returns an ORM object or a collection of ORM objects,
|
- Final hop:
|
||||||
serialize to simple dicts; else return the plain value.
|
* If relationship and not loaded: return None
|
||||||
|
* else allow getattr for non-relationship attrs (hybrids/properties) that compute from already-loaded data.
|
||||||
|
* Serialize ORM objects at the lead into dicts
|
||||||
"""
|
"""
|
||||||
parts = dotted.split(".")
|
parts = dotted.split(".")
|
||||||
if not parts:
|
if not parts:
|
||||||
return None
|
return None
|
||||||
|
|
||||||
cur = obj
|
cur = obj
|
||||||
# Traverse up to parent of last token without lazy-loads
|
# Traverse up to the parent of the last token safely
|
||||||
for part in parts[:-1]:
|
for part in parts[:-1]:
|
||||||
if cur is None:
|
if cur is None:
|
||||||
return None
|
return None
|
||||||
|
|
@ -163,24 +165,36 @@ def _deep_get_loaded(obj: Any, dotted: str) -> Any:
|
||||||
return None
|
return None
|
||||||
|
|
||||||
last = parts[-1]
|
last = parts[-1]
|
||||||
|
|
||||||
|
# If last is an ORM relationship, only return it if already loaded.
|
||||||
|
m = _sa_mapper(cur)
|
||||||
|
if m is not None and last in m.relationships:
|
||||||
|
val = _safe_get_loaded_attr(cur, last)
|
||||||
|
if val is None:
|
||||||
|
return None
|
||||||
|
# serialize relationship value
|
||||||
|
if _sa_mapper(val) is not None:
|
||||||
|
return _serialize_simple_obj(val)
|
||||||
|
if isinstance(val, (list, tuple)):
|
||||||
|
out = []
|
||||||
|
for v in val:
|
||||||
|
out.append(_serialize_simple_obj(v) if _sa_mapper(v) is not None else v)
|
||||||
|
return out
|
||||||
|
return val
|
||||||
|
|
||||||
|
# Not a relationship: try loaded value, else safe getattr
|
||||||
val = _safe_get_loaded_attr(cur, last)
|
val = _safe_get_loaded_attr(cur, last)
|
||||||
if val is None:
|
if val is None:
|
||||||
# Do NOT lazy load. If it isn't loaded, treat as absent.
|
try:
|
||||||
return None
|
# getattr here will not lazy-load relationships because we already gated those
|
||||||
|
val = getattr(cur, last, None)
|
||||||
|
except Exception:
|
||||||
|
val = None
|
||||||
|
|
||||||
# If the final hop is an ORM object or collection, serialize it
|
|
||||||
if _sa_mapper(val) is not None:
|
if _sa_mapper(val) is not None:
|
||||||
return _serialize_simple_obj(val)
|
return _serialize_simple_obj(val)
|
||||||
if isinstance(val, (list, tuple)):
|
if isinstance(val, (list, tuple)):
|
||||||
out = []
|
return [_serialize_simple_obj(v) if _sa_mapper(v) is not None else v for v in val]
|
||||||
for v in val:
|
|
||||||
if _sa_mapper(v) is not None:
|
|
||||||
out.append(_serialize_simple_obj(v))
|
|
||||||
else:
|
|
||||||
out.append(v)
|
|
||||||
return out
|
|
||||||
|
|
||||||
# Plain scalar/computed value
|
|
||||||
return val
|
return val
|
||||||
|
|
||||||
def _serialize_leaf(obj: Any) -> Any:
|
def _serialize_leaf(obj: Any) -> Any:
|
||||||
|
|
|
||||||
|
|
@ -23,4 +23,4 @@ def rel_map(model: type) -> Dict[str, RelInfo]:
|
||||||
return mapper_info(model)["rels"]
|
return mapper_info(model)["rels"]
|
||||||
|
|
||||||
def column_names_for_model(model: type) -> Tuple[str, ...]:
|
def column_names_for_model(model: type) -> Tuple[str, ...]:
|
||||||
return mapper_info(model)["col"]
|
return mapper_info(model)["cols"]
|
||||||
|
|
|
||||||
|
|
@ -12,7 +12,7 @@ from sqlalchemy.sql.elements import UnaryExpression, ColumnElement
|
||||||
|
|
||||||
from crudkit.core import to_jsonable, deep_diff, diff_to_patch, filter_to_columns, normalize_payload
|
from crudkit.core import to_jsonable, deep_diff, diff_to_patch, filter_to_columns, normalize_payload
|
||||||
from crudkit.core.base import Version
|
from crudkit.core.base import Version
|
||||||
from crudkit.core.meta import rel_map
|
from crudkit.core.meta import rel_map, column_names_for_model
|
||||||
from crudkit.core.params import is_truthy, normalize_fields_param
|
from crudkit.core.params import is_truthy, normalize_fields_param
|
||||||
from crudkit.core.spec import CRUDSpec, CollPred
|
from crudkit.core.spec import CRUDSpec, CollPred
|
||||||
from crudkit.core.types import OrderSpec, SeekWindow
|
from crudkit.core.types import OrderSpec, SeekWindow
|
||||||
|
|
@ -281,7 +281,10 @@ class CRUDService(Generic[T]):
|
||||||
def _stable_order_by(self, root_alias, given_order_by):
|
def _stable_order_by(self, root_alias, given_order_by):
|
||||||
order_by = list(given_order_by or [])
|
order_by = list(given_order_by or [])
|
||||||
if not order_by:
|
if not order_by:
|
||||||
|
# Safe default: primary key(s) only. No unlabeled expressions.
|
||||||
return _dedupe_order_by(self._default_order_by(root_alias))
|
return _dedupe_order_by(self._default_order_by(root_alias))
|
||||||
|
|
||||||
|
# Dedupe what the user gave us, then ensure PK tie-breakers exist
|
||||||
order_by = _dedupe_order_by(order_by)
|
order_by = _dedupe_order_by(order_by)
|
||||||
mapper: Mapper[Any] = cast(Mapper[Any], inspect(self.model))
|
mapper: Mapper[Any] = cast(Mapper[Any], inspect(self.model))
|
||||||
present = {_order_identity(_unwrap_ob(ob)[0]) for ob in order_by}
|
present = {_order_identity(_unwrap_ob(ob)[0]) for ob in order_by}
|
||||||
|
|
@ -355,7 +358,16 @@ class CRUDService(Generic[T]):
|
||||||
join_paths = tuple(spec.get_join_paths())
|
join_paths = tuple(spec.get_join_paths())
|
||||||
filter_tables = _collect_tables_from_filters(filters)
|
filter_tables = _collect_tables_from_filters(filters)
|
||||||
fkeys = set()
|
fkeys = set()
|
||||||
_, proj_opts = compile_projection(self.model, req_fields) if req_fields else ([], [])
|
# Build projection opts only if there are true scalar columns requested.
|
||||||
|
# Bare relationship fields like "owner" should not force root column pruning.
|
||||||
|
column_names = set(column_names_for_model(self.model))
|
||||||
|
has_scalar_column_tokens = any(
|
||||||
|
(("." not in f) and (f in column_names))
|
||||||
|
for f in (req_fields or [])
|
||||||
|
)
|
||||||
|
_, proj_opts = (compile_projection(self.model, req_fields)
|
||||||
|
if (req_fields and has_scalar_column_tokens)
|
||||||
|
else ([], []))
|
||||||
|
|
||||||
# filter_tables = ()
|
# filter_tables = ()
|
||||||
# fkeys = set()
|
# fkeys = set()
|
||||||
|
|
@ -374,8 +386,25 @@ class CRUDService(Generic[T]):
|
||||||
|
|
||||||
def _apply_firsthop_strategies(self, query, root_alias, plan: _Plan):
|
def _apply_firsthop_strategies(self, query, root_alias, plan: _Plan):
|
||||||
joined_rel_keys: set[str] = set()
|
joined_rel_keys: set[str] = set()
|
||||||
rels = rel_map(self.model)
|
rels = rel_map(self.model) # {name: RelInfo}
|
||||||
|
|
||||||
|
# Eager join to-one relationships requested as bare fields (e.g., fields=owner)
|
||||||
|
requested_scalars = set(plan.root_field_names or [])
|
||||||
|
for key in requested_scalars:
|
||||||
|
info = rels.get(key)
|
||||||
|
if info and not info.uselist and key not in joined_rel_keys:
|
||||||
|
query = query.join(getattr(root_alias, key), isouter=True)
|
||||||
|
joined_rel_keys.add(key)
|
||||||
|
|
||||||
|
# 1) Join to-one relationships explicitly requested as bare fields
|
||||||
|
requested_scalars = set(plan.root_field_names or []) # names like "owner", "supervisor"
|
||||||
|
for key in requested_scalars:
|
||||||
|
info = rels.get(key)
|
||||||
|
if info and not info.uselist and key not in joined_rel_keys:
|
||||||
|
query = query.join(getattr(root_alias, key), isouter=True)
|
||||||
|
joined_rel_keys.add(key)
|
||||||
|
|
||||||
|
# 2) Join to-one relationships from parsed join_paths
|
||||||
for base_alias, rel_attr, target_alias in plan.join_paths:
|
for base_alias, rel_attr, target_alias in plan.join_paths:
|
||||||
if base_alias is not root_alias:
|
if base_alias is not root_alias:
|
||||||
continue
|
continue
|
||||||
|
|
@ -384,6 +413,7 @@ class CRUDService(Generic[T]):
|
||||||
query = query.join(target_alias, rel_attr.of_type(target_alias), isouter=True)
|
query = query.join(target_alias, rel_attr.of_type(target_alias), isouter=True)
|
||||||
joined_rel_keys.add(prop.key if prop is not None else rel_attr.key)
|
joined_rel_keys.add(prop.key if prop is not None else rel_attr.key)
|
||||||
|
|
||||||
|
# 3) Ensure to-one touched by filters is joined
|
||||||
if plan.filter_tables:
|
if plan.filter_tables:
|
||||||
for key, info in rels.items():
|
for key, info in rels.items():
|
||||||
if info.uselist or not info.target_cls:
|
if info.uselist or not info.target_cls:
|
||||||
|
|
@ -393,6 +423,7 @@ class CRUDService(Generic[T]):
|
||||||
query = query.join(getattr(root_alias, key), isouter=True)
|
query = query.join(getattr(root_alias, key), isouter=True)
|
||||||
joined_rel_keys.add(key)
|
joined_rel_keys.add(key)
|
||||||
|
|
||||||
|
# 4) Collections via selectinload, optionally load_only for requested child columns
|
||||||
for base_alias, rel_attr, _target_alias in plan.join_paths:
|
for base_alias, rel_attr, _target_alias in plan.join_paths:
|
||||||
if base_alias is not root_alias:
|
if base_alias is not root_alias:
|
||||||
continue
|
continue
|
||||||
|
|
@ -408,10 +439,46 @@ class CRUDService(Generic[T]):
|
||||||
if cols:
|
if cols:
|
||||||
opt = opt.load_only(*cols)
|
opt = opt.load_only(*cols)
|
||||||
query = query.options(opt)
|
query = query.options(opt)
|
||||||
|
|
||||||
|
for path, _names in (plan.rel_field_names or {}).items():
|
||||||
|
if not path:
|
||||||
|
continue
|
||||||
|
# Build a chained selectinload for each relationship segment in the path
|
||||||
|
first = path[0]
|
||||||
|
info = rels.get(first)
|
||||||
|
if not info or info.target_cls is None:
|
||||||
|
continue
|
||||||
|
|
||||||
|
# Start with selectinload on the first hop
|
||||||
|
opt = selectinload(getattr(root_alias, first))
|
||||||
|
|
||||||
|
# Walk deeper segments
|
||||||
|
cur_cls = info.target_cls
|
||||||
|
for seg in path[1:]:
|
||||||
|
sub = rel_map(cur_cls).get(seg)
|
||||||
|
if not sub or sub.target_cls is None:
|
||||||
|
# if segment isn't a relationship, we stop the chain
|
||||||
|
break
|
||||||
|
opt = opt.selectinload(getattr(cur_cls, seg))
|
||||||
|
cur_cls = sub.target_cls
|
||||||
|
|
||||||
|
query = query.options(opt)
|
||||||
|
|
||||||
return query
|
return query
|
||||||
|
|
||||||
|
|
||||||
def _apply_proj_opts(self, query, plan: _Plan):
|
def _apply_proj_opts(self, query, plan: _Plan):
|
||||||
return query.options(*plan.proj_opts) if plan.proj_opts else query
|
if not plan.proj_opts:
|
||||||
|
return query
|
||||||
|
try:
|
||||||
|
return query.options(*plan.proj_opts)
|
||||||
|
except KeyError as e:
|
||||||
|
# Seen "KeyError: 'col'" when alias-column remapping meets unlabeled exprs.
|
||||||
|
log.debug("Projection options disabled due to %r; proceeding without them.", e)
|
||||||
|
return query
|
||||||
|
except Exception as e:
|
||||||
|
log.debug("Projection options failed (%r); proceeding without them.", e)
|
||||||
|
return query
|
||||||
|
|
||||||
def _projection_meta(self, plan: _Plan):
|
def _projection_meta(self, plan: _Plan):
|
||||||
if plan.req_fields:
|
if plan.req_fields:
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue