Another tricky bug. Now we should be good.

This commit is contained in:
Yaro Kasear 2025-09-25 13:38:48 -05:00
parent 803eb83ca5
commit ea8e8a9df7

View file

@ -41,6 +41,45 @@ class _CRUDModelProto(_HasID, _HasTable, _HasADict, Protocol):
T = TypeVar("T", bound=_CRUDModelProto) T = TypeVar("T", bound=_CRUDModelProto)
def _unwrap_ob(ob):
"""Return (col, is_desc) from an ORDER BY element (handles .asc()/.desc())."""
col = getattr(ob, "element", None)
if col is None:
col = ob
is_desc = False
dir_attr = getattr(ob, "_direction", None)
if dir_attr is not None:
is_desc = (dir_attr is operators.desc_op) or (getattr(op, "name", "").upper() == "DESC")
elif isinstance(ob, UnaryExpression):
op = getattr(ob, "operator", None)
is_desc = (op is operators.desc_op) or (getattr(op, "name", "").upper() == "DESC")
return col, bool(is_desc)
def _order_identity(col: ColumnElement):
"""
Build a stable identity for a column suitable for deduping.
We ignore direction here. Duplicates are duplicates regardless of ASC/DESC.
"""
table = getattr(col, "table", None)
table_key = getattr(table, "key", None) or id(table)
col_key = getattr(col, "key", None) or getattr(col, "name", None)
return (table_key, col_key)
def _dedupe_order_by(order_by):
"""Remove duplicate ORDER BY entries (by column identity, ignoring direction)."""
if not order_by:
return []
seen = set()
out = []
for ob in order_by:
col, _ = _unwrap_ob(ob)
ident = _order_identity(col)
if ident in seen:
continue
seen.add(ident)
out.append(ob)
return out
def _hops_from_sort(params: dict | None) -> set[str]: def _hops_from_sort(params: dict | None) -> set[str]:
"""Extract first-hop relationship names from a sort spec like 'owner.first_name,-brand.name'.""" """Extract first-hop relationship names from a sort spec like 'owner.first_name,-brand.name'."""
if not params: if not params:
@ -406,20 +445,27 @@ class CRUDService(Generic[T]):
""" """
Ensure deterministic ordering by appending PK columns as tiebreakers. Ensure deterministic ordering by appending PK columns as tiebreakers.
If no order is provided, fall back to default primary-key order. If no order is provided, fall back to default primary-key order.
Never duplicates columns in ORDER BY (SQL Server requires uniqueness).
""" """
order_by = list(given_order_by or []) order_by = list(given_order_by or [])
if not order_by: if not order_by:
return self._default_order_by(root_alias) return _dedupe_order_by(self._default_order_by(root_alias))
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))
pk_cols = [] present = {_order_identity(_unwrap_ob(ob)[0]) for ob in order_by}
for col in mapper.primary_key:
try:
pk_cols.append(getattr(root_alias, col.key))
except AttributeError:
pk_cols.append(col)
return [*order_by, *pk_cols] for pk in mapper.primary_key:
try:
pk_col = getattr(root_alias, pk.key)
except AttributeError:
pk_col = pk
if _order_identity(pk_col) not in present:
order_by.append(pk_col.asc())
present.add(_order_identity(pk_col))
return order_by
def get(self, id: int, params=None) -> T | None: def get(self, id: int, params=None) -> T | None:
""" """
@ -551,8 +597,11 @@ class CRUDService(Generic[T]):
paginating = (limit is not None) or (offset is not None and offset != 0) paginating = (limit is not None) or (offset is not None and offset != 0)
if paginating and not order_by and self.backend.requires_order_by_for_offset: if paginating and not order_by and self.backend.requires_order_by_for_offset:
order_by = self._default_order_by(root_alias) order_by = self._default_order_by(root_alias)
order_by = _dedupe_order_by(order_by)
if order_by: if order_by:
query = query.order_by(*self._stable_order_by(root_alias, order_by)) query = query.order_by(*order_by)
# Offset/limit # Offset/limit
if offset is not None and offset != 0: if offset is not None and offset != 0: