This introduces an uglt regression.

This commit is contained in:
Yaro Kasear 2025-10-20 15:20:52 -05:00
parent ce7d092be4
commit bd2daf921a
5 changed files with 136 additions and 128 deletions

View file

@ -1,18 +1,19 @@
from __future__ import annotations
from collections.abc import Iterable
from dataclasses import dataclass
from flask import current_app
from typing import Any, Callable, Type, TypeVar, Generic, Optional, Protocol, runtime_checkable, cast
from sqlalchemy import and_, func, inspect, or_, text, select, literal
from sqlalchemy import and_, func, inspect, or_, text
from sqlalchemy.engine import Engine, Connection
from sqlalchemy.orm import Load, Session, with_polymorphic, Mapper, selectinload, with_loader_criteria, aliased, with_parent
from sqlalchemy.orm import Load, Session, with_polymorphic, Mapper, selectinload, with_loader_criteria
from sqlalchemy.orm.attributes import InstrumentedAttribute
from sqlalchemy.sql import operators, visitors
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.base import Version
from crudkit.core.meta import rel_map
from crudkit.core.params import is_truthy, normalize_fields_param
from crudkit.core.spec import CRUDSpec, CollPred
from crudkit.core.types import OrderSpec, SeekWindow
from crudkit.backend import BackendInfo, make_backend_info
@ -139,23 +140,6 @@ def _dedupe_order_by(order_by):
out.append(ob)
return out
def _is_truthy(val):
return str(val).lower() in ('1', 'true', 'yes', 'on')
def _normalize_fields_param(params: dict | None) -> list[str]:
if not params:
return []
raw = params.get("fields")
if isinstance(raw, (list, tuple)):
out: list[str] = []
for item in raw:
if isinstance(item, str):
out.extend([p for p in (s.strip() for s in item.split(",")) if p])
return out
if isinstance(raw, str):
return [p for p in (s.strip() for s in raw.split(",")) if p]
return []
# ---------------------------- CRUD service ----------------------------
class CRUDService(Generic[T]):
@ -200,7 +184,7 @@ class CRUDService(Generic[T]):
def _apply_soft_delete_criteria_for_children(self, query, plan: "CRUDService._Plan", params):
# Skip if caller explicitly asked for deleted
if _is_truthy((params or {}).get("include_deleted")):
if is_truthy((params or {}).get("include_deleted")):
return query
seen = set()
@ -218,7 +202,8 @@ class CRUDService(Generic[T]):
with_loader_criteria(
target_cls,
lambda cls: cls.is_deleted == False,
include_aliases=True
include_aliases=True,
propagate_to_loaders=True,
)
)
return query
@ -248,21 +233,18 @@ class CRUDService(Generic[T]):
order_spec = self._extract_order_spec(root_alias, plan.order_by)
# Inner subquery must be ordered exactly like the real query
# Inner subquery ordered exactly like the real query
inner = query.order_by(*self._order_clauses(order_spec, invert=False))
# IMPORTANT: Build subquery that actually exposes the order-by columns
# under predictable names, then select FROM that and reference subq.c[...]
subq = inner.with_entities(*order_spec.cols).subquery()
# Label each order-by column with deterministic, unique names
labeled_cols = []
for idx, col in enumerate(order_spec.cols):
base = getattr(col, "key", None) or getattr(col, "name", None)
name = f"ord_{idx}_{base}" if base else f"ord_{idx}"
labeled_cols.append(col.label(name))
# Map the order columns to the subquery columns by key/name
cols_on_subq = []
for col in order_spec.cols:
key = getattr(col, "key", None) or getattr(col, "name", None)
if not key:
# Fallback, but frankly your order cols should have names
raise ValueError("Order-by column is missing a key/name")
cols_on_subq.append(getattr(subq.c, key))
subq = inner.with_entities(*labeled_cols).subquery()
cols_on_subq = [getattr(subq.c, c.key) for c in labeled_cols]
# Now the outer anchor query orders and offsets on the subquery columns
anchor_q = (
@ -282,7 +264,7 @@ class CRUDService(Generic[T]):
return list(row) # tuple-like -> list for _key_predicate
def _apply_not_deleted(self, query, root_alias, params):
if self.supports_soft_delete and not _is_truthy((params or {}).get("include_deleted")):
if self.supports_soft_delete and not is_truthy((params or {}).get("include_deleted")):
return query.filter(getattr(root_alias, "is_deleted") == False)
return query
@ -362,7 +344,7 @@ class CRUDService(Generic[T]):
proj_opts: Any
def _plan(self, params, root_alias) -> _Plan:
req_fields = _normalize_fields_param(params)
req_fields = normalize_fields_param(params)
spec = CRUDSpec(self.model, params or {}, root_alias)
filters = spec.parse_filters()
@ -391,62 +373,41 @@ class CRUDService(Generic[T]):
return query.options(Load(root_alias).load_only(*only_cols)) if only_cols else query
def _apply_firsthop_strategies(self, query, root_alias, plan: _Plan):
nested_first_hops = { p[0] for p in (plan.rel_field_names or {}).keys() if len(p) > 1 }
joined_rel_keys = set()
joined_rel_keys: set[str] = set()
rels = rel_map(self.model)
# Existing behavior: join everything in join_paths (to-one), selectinload collections
for base_alias, rel_attr, target_alias in plan.join_paths:
if base_alias is not root_alias:
continue
prop = getattr(rel_attr, "property", None)
is_collection = bool(getattr(prop, "uselist", False))
if not is_collection:
if prop and not prop.uselist:
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)
else:
opt = selectinload(rel_attr)
child_names = (plan.collection_field_names or {}).get(rel_attr.key, [])
if child_names:
target_cls = prop.mapper.class_
cols = [getattr(target_cls, n, None) for n in child_names]
cols = [c for c in cols if isinstance(c, InstrumentedAttribute)]
if cols:
opt = opt.load_only(*cols)
query = query.options(opt)
# NEW: if a first-hop to-one relationships target table is present in filter expressions,
# make sure we actually JOIN it (outer) so filters dont create a cartesian product.
if plan.filter_tables:
mapper: Mapper[Any] = cast(Mapper[Any], inspect(self.model))
for rel in mapper.relationships:
if rel.uselist:
continue # only first-hop to-one here
target_tbl = getattr(rel.mapper.class_, "__table__", None)
if target_tbl is None:
for key, info in rels.items():
if info.uselist or not info.target_cls:
continue
if target_tbl in plan.filter_tables:
if rel.key in joined_rel_keys:
continue # already joined via join_paths
query = query.join(getattr(root_alias, rel.key), isouter=True)
joined_rel_keys.add(rel.key)
if log.isEnabledFor(logging.DEBUG):
info = []
for base_alias, rel_attr, target_alias in plan.join_paths:
if base_alias is not root_alias:
continue
prop = getattr(rel_attr, "property", None)
sel = getattr(target_alias, "selectable", None)
info.append({
"rel": (getattr(prop, "key", getattr(rel_attr, "key", "?"))),
"collection": bool(getattr(prop, "uselist", False)),
"target_keys": list(_selectable_keys(sel)) if sel is not None else [],
"joined": (getattr(prop, "key", None) in joined_rel_keys),
})
log.debug("FIRSTHOP: %s.%s first-hop paths: %s",
self.model.__name__, getattr(root_alias, "__table__", type(root_alias)).key,
info)
target_tbl = getattr(info.target_cls, "__table__", None)
if target_tbl is not None and target_tbl in plan.filter_tables and key not in joined_rel_keys:
query = query.join(getattr(root_alias, key), isouter=True)
joined_rel_keys.add(key)
for base_alias, rel_attr, _target_alias in plan.join_paths:
if base_alias is not root_alias:
continue
prop = getattr(rel_attr, "property", None)
if not prop or not prop.uselist:
continue
opt = selectinload(rel_attr)
child_names = (plan.collection_field_names or {}).get(prop.key, [])
if child_names:
target_cls = prop.mapper.class_
cols = [getattr(target_cls, n, None) for n in child_names]
cols = [c for c in cols if isinstance(c, InstrumentedAttribute)]
if cols:
opt = opt.load_only(*cols)
query = query.options(opt)
return query
def _apply_proj_opts(self, query, plan: _Plan):