Skip to content
Merged
2 changes: 2 additions & 0 deletions .claude/skills/slayer-query.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ A `SlayerQuery` is a JSON/dict object. The same shape works across the REST API,

`order[].column` is the short alias (`count`, `revenue_sum`) — not the colon form.

**Ordering by something you don't project.** `order` may name an undeclared column/aggregate ("top-N by X, show only Y, Z"). An **aggregate** (`amount:sum`, `customers.revenue:sum`) is computed hidden, sorted on, and stripped from the result. A **raw row column** is orderable only in a raw-rows query (`distinct_dimension_values: false`); in a grouped/dedup query it's rejected (HTTP 400 — add it to `dimensions` or order by an aggregate of it). A **joined** row column and an inline **transform/composite** (`change(amount:sum)`) are also rejected — project it, or declare it as a measure and order by that.

**Dim-only queries deduplicate.** A query with no measures and at least one dimension or time-dimension auto-emits `GROUP BY <dim/td aliases>` and returns the distinct combinations. The `GROUP BY` is applied before `LIMIT`, so a row cap can't silently drop unique tuples. To opt out, set `"distinct_dimension_values": false` on the query — emits raw rows (no top-level `GROUP BY`), with WHERE / ORDER BY / LIMIT applied as usual. Any measure reference in `measures` / `filters` / `order` raises `DistinctDimensionValuesError` in this mode.

## Measures — colon aggregation
Expand Down
1 change: 1 addition & 0 deletions DECISIONS.md
Original file line number Diff line number Diff line change
Expand Up @@ -79,3 +79,4 @@ implementation detail. Include issue refs when known.
- 2026-08-03 — CTE-name collision detection stays case-sensitive for now (DEV-1713 Codex review, deferred to DEV-1726): `AliasAllocator` / `assert_unique_cte_names` compare CTE names by exact string, but generated CTE names are emitted unquoted and case-fold on Postgres/Snowflake/Redshift — so two user measure aliases differing only in case, both generating CTEs, can still collide there. Deferred rather than fixed in Stage 9 because it is pre-existing (pre-DEV-1713 the names weren't deduped at all), an edge case, and the correct fix is dialect-aware (fold only for case-folding dialects; a blanket case-insensitive dedup would wrongly merge genuinely-distinct names on case-sensitive BigQuery / quoted SQLite). Tracked in DEV-1726.
- 2026-08-03 — Merge resolution, Stage 4 × Stage 9 (DEV-1708 × DEV-1713): the user-approved derived-shared-grain raise (DEV-1708) wins over Stage 9's combined test vehicle — D3's dotted final-stage keys do NOT by themselves make a plain derived joined dim legal as cross-model shared grain (the CTE join-back rendering is still unbuilt; full support remains DEV-1495). The two Stage-9 Codex-F6 alias/key-agreement tests split their derived-dim and cross-model-aggregate coverage into separate queries.
- 2026-08-03 — Dialect-aware CTE-name case-folding (DEV-1726): `AliasAllocator` gains `folds_case` (resolved via `naming.dialect_folds_case`, threaded by the single `SQLGenerator._new_allocator` factory — the only construction site, test-pinned) and `assert_unique_cte_names` folds per dialect. Comparison-only folding with `str.lower()` (not `casefold`; sqlglot `normalize_identifier` parity) — allocated names keep original case, so output changes only when a genuine fold-collision forces a `_2` walk. Fold set = every registry dialect EXCEPT ClickHouse; unknown strings stay exact. Issue-text corrections (GoogleSQL docs + sqlglot + empirics): BigQuery FOLDS (CTE names are query aliases, case-insensitive — only real table names are CS) and SQLite/DuckDB fold even quoted names; MySQL/T-SQL fold deliberately despite config-dependence (folding is rename-only-safe, not folding leaves the bug live on majority configs). The belt folds regardless of quoting — over-strict by design on allocator-sanitized output, never a general SQL validator. Public result keys are reserved, never allocator-minted, so folding can never rename them.
- 2026-08-03 — Order-only hidden slots + plan-time order/partition validations (DEV-1712, DEV-1703 Stage 8). An ORDER BY ref that is not a declared dimension/measure is classified at **plan time** in `stage_planner.plan_query` (right after `_bucket_slots`): an **aggregate** (local or cross-model) always materialises hidden and orders — never rejected; a **local row column** is allowed only in a raw-rows query (`distinct_dimension_values=False`, no grouping) and emits a SPLIT `orders.<col>` reference in the generator's `_apply_order_limit_from_planned` (the old `NotImplementedError` becomes a defensive internal assertion — the plan-time pass guarantees only that shape reaches it); a **grouped** local row column raises `ValueError` (not in GROUP BY — add it to dims or order by an aggregate of it); a **joined** row column raises `UnresolvableOrderColumnError`; an inline **transform/composite** (`change(amount:sum)`, only reachable as `raw_formula` — composite arithmetic is unexpressible via `OrderItem.column`, Pydantic rejects it) raises `ValueError` pointing at "declare it as a measure" — full support deferred to **DEV-1733** with strict-xfail future tests + a matching worktree. The grouping predicate is planner-semantic: `bool(agg_slots) or (dims/tds present and distinct_dimension_values)` — a hidden order aggregate that induces grouping counts, so a row column in that same query is correctly rejected. **Hidden cross-model aggregate trim** (DEV-1495 bug 2): an order-only CMA gets `hidden=True`/`public_alias=None` from the planner; the generator's combined-projection loop skips it (`public_aliases=[]`) **only when there is no transform chain** — a hidden CMA feeding a `cumsum(...)` step must stay projected for the step CTE to consume (the transform outer-wrap does the public-vs-hidden trim there). Its ORDER BY term is CTE-qualified (`_cm_*."<alias>"`) since the bare alias is no longer projected. The DEV-1495-bug-1 malformed-alias half (`orders.customers._sum`) was already fixed by Stage 9's naming module — only the projection leak remained. **partition_by grain guard** (DEV-1497): a pre-intern pass (`planning.rewrite_rank_partition_keys`, mirroring `lower_sugar_transforms`' identity-preserving rebuild) validates every rank-family (`rank`/`dense_rank`/`percent_rank`/`ntile`) `partition_by` key resolves to a query dimension/time-dimension by **exact ValueKey membership** — the typed binder resolves `partition_by` to a ValueKey before validation, so the legacy string-matching ambiguity can't arise. A time-dimension **source column** is rewritten to its `TimeTruncKey` so `PARTITION BY` uses the truncated bucket (not the raw timestamp, which had silently widened the GROUP BY grain and emitted a duplicate alias); a non-dimension raises `ValueError` naming the transform + column + available dims (the legacy `enrichment._resolve_rank_partition` message, restored). The 7 DEV-1645 Flavor-A ORDER-BY unit pins (split-not-composite for unprojected sort keys; `UnresolvableOrderColumnError` for joined sort keys) were made green by porting main's legacy `_OrderColRef`/`_order_split_sql`/`_resolve_order_column` fix (lost in the Stage-0 merge) into the legacy generator — throwaway parity, deleted with the legacy stack in Stage 11 — so `tests/parity_xfails.py` is now empty (the DEV-1485 end-state). Deliberate divergence from main: the typed pipeline rejects a grouped raw-row order at plan time (HTTP 400) instead of emitting SQL the database rejects at execution. A follow-up review sweep (PR #274) extended the same joined-ORDER-BY host-local guard into the legacy `_resolve_order_column` and rejected an unprojected sort key in the CTE-wrapped `_apply_pagination_to_sql`; added a partition ambiguity guard for a time column carried at two granularities.
2 changes: 1 addition & 1 deletion docs/concepts/formulas.md
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ Combine with a filter to get "top N":

**Ranking within a partition (`partition_by=`):**

To rank within groups instead of across the whole result set, pass `partition_by=` referencing one or more **query dimensions** (or time dimensions). The columns must already be grouped on — partitioning by a column that's not a dimension errors at enrichment time.
To rank within groups instead of across the whole result set, pass `partition_by=` referencing one or more **query dimensions** (or time dimensions). The columns must already be grouped on — partitioning by a column that's not a dimension errors at plan time (HTTP 400). Naming a query time-dimension partitions by its truncated bucket, not the raw timestamp.

```json
{
Expand Down
21 changes: 21 additions & 0 deletions docs/concepts/queries.md
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,27 @@ A sort specification: `column` is the short alias (`status`, `revenue_sum`, `*:c

Via MCP: `{"column": "*:count", "direction": "desc"}`

### Ordering by something you don't project

`order` may reference a column or aggregate that is **not** declared as a dimension/measure — the classic "top-N by metric X, display only Y, Z" pattern:

```json
{"source_model": "orders", "dimensions": ["status"], "measures": [{"formula": "*:count"}],
"order": [{"column": "amount:sum", "direction": "desc"}], "limit": 10}
```

The `amount:sum` aggregate is computed as a hidden column, sorted on, and **stripped from the result** — the response projects only `status` and `_count`. This works for local aggregates, cross-model aggregates (`customers.revenue:sum`), and inner-stage columns re-aggregated in a later DAG stage (`customers__revenue_sum:max`).

What each shape of an *undeclared* order target does:

| Order target | Behavior |
| --- | --- |
| An aggregate (`amount:sum`, `customers.revenue:sum`) | Computed hidden, sorted on, stripped from the result. Always allowed. |
| A raw row column, in a **raw-rows** query (`distinct_dimension_values: false`, no measures) | Sorted on directly (`ORDER BY orders.created_at`). |
| A raw row column, in an **aggregated / dedup** query | Rejected (HTTP 400): it isn't in the `GROUP BY`. Add it to `dimensions`, or order by an aggregate of it (`created_at:max`). |
| A **joined** row column (`customers.regions.name`) not projected | Rejected (HTTP 400): project it (add to `dimensions`) or order by a projected field. |
| An inline **transform / composite** (`change(amount:sum)`, `revenue:sum / cnt:sum`) not declared | Rejected (HTTP 400): declare it as a measure (optionally with a `name`) and order by that. |

## Response

Query results are returned as a `SlayerResponse`:
Expand Down
60 changes: 59 additions & 1 deletion slayer/engine/planning.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@

from __future__ import annotations

from typing import Dict, FrozenSet, List, Optional
from typing import Callable, Dict, FrozenSet, List, Optional

from pydantic import BaseModel, ConfigDict, Field

Expand Down Expand Up @@ -54,6 +54,7 @@
column_path,
normalize_scalar,
)
from slayer.core.formula import RANK_FAMILY_TRANSFORMS
from slayer.core.refs import agg_kwarg_canonical_str, canonical_agg_name
from slayer.engine.binding import BoundExpr, BoundFilter
from slayer.engine.planned import SlotId, ValueSlot
Expand Down Expand Up @@ -401,6 +402,63 @@ def lower_sugar_transforms(key: ValueKey) -> ValueKey:
return key


def rewrite_rank_partition_keys( # NOSONAR(S3776) — sequential isinstance dispatch over the closed ValueKey union; each branch is the per-type identity-preserving rebuild contract, mirroring lower_sugar_transforms. Extracting per-type helpers would scatter the contract across the module.
key: ValueKey, *, rewrite_fn: Callable[[TransformKey], FrozenSet],
) -> ValueKey:
"""Walk ``key``; for every rank-family ``TransformKey`` carrying an
explicit ``partition_by`` (non-empty ``partition_keys``), replace those
keys with ``rewrite_fn(transform_key)`` (DEV-1497).

``rewrite_fn`` receives the whole ``TransformKey`` and returns a new
``frozenset`` of partition keys — validating that each resolves to a query
dimension / time-dimension and rewriting a time-dimension source column to
its ``TimeTruncKey`` bucket. It may raise ``ValueError`` for a partition
column that is not a query dimension.

Identity-preserving (mirrors :func:`lower_sugar_transforms`): parents are
rebuilt only where a child changed, so this runs BEFORE interning without
churning unrelated slots. Reaches rank transforms nested in composite
measures (``ArithmeticKey`` / ``ScalarCallKey``) and in filter predicates
(comparisons are ``ArithmeticKey``).
"""
def _rec(k: ValueKey) -> ValueKey:
return rewrite_rank_partition_keys(key=k, rewrite_fn=rewrite_fn)

if isinstance(key, TransformKey):
new_input = _rec(key.input)
new_pk = key.partition_keys
if key.op in RANK_FAMILY_TRANSFORMS and key.partition_keys:
new_pk = rewrite_fn(key)
if new_input is key.input and new_pk == key.partition_keys:
return key
return key.model_copy(update={"input": new_input, "partition_keys": new_pk})
if isinstance(key, ArithmeticKey):
new_ops = tuple(_rec(op) for op in key.operands)
unchanged = all(a is b for a, b in zip(new_ops, key.operands))
return key if unchanged else ArithmeticKey(op=key.op, operands=new_ops)
if isinstance(key, ScalarCallKey):
rewritable = _SLOTTABLE_KIND + (ArithmeticKey, ScalarCallKey, BetweenKey)
new_args = tuple(
_rec(a) if isinstance(a, rewritable) else a for a in key.args
)
unchanged = all(a is b for a, b in zip(new_args, key.args))
return key if unchanged else ScalarCallKey(name=key.name, args=new_args)
if isinstance(key, BetweenKey):
new_col, new_low, new_high = _rec(key.column), _rec(key.low), _rec(key.high)
unchanged = (
new_col is key.column and new_low is key.low and new_high is key.high
)
return key if unchanged else BetweenKey(
column=new_col, low=new_low, high=new_high,
)
if isinstance(key, InKey):
new_col = _rec(key.column)
return key if new_col is key.column else InKey(
column=new_col, values=key.values, negated=key.negated,
)
return key


def desugar_change_pct(key: TransformKey) -> ArithmeticKey:
"""``change_pct(x)`` → ``(x - time_shift(x, periods=-1)) /
NULLIF(time_shift(x, periods=-1), 0)``.
Expand Down
Loading
Loading