Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 17 additions & 41 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,8 @@ sql = "SELECT target / MEAN(target) OVER (PARTITION BY city) AS enc FROM __THIS_
- **Typed I/O**: Pydantic models for the input row and output, validated when the
transformer is built and again at call time. Output is a typed model; the input
schema is auto-synthesized or user-supplied.
- **Transformer references**: interpolate a fitted sklearn transformer directly
into a t-string and apply it to columns (see below).
- **Transform references**: interpolate another `SQLTransform` into a t-string to
compose transforms (see below).

See [docs/SQL_SUPPORT.md](docs/SQL_SUPPORT.md) for the feature-by-feature tracker.

Expand Down Expand Up @@ -104,57 +104,33 @@ query engine once; inference pays only for a lean interpreter walking a plan. Th
separation of **fit** (compute statistics) and **transform/infer** (apply them)
is the standard ML pattern — fit on training data, apply to training and serving.

## Referencing a fitted sklearn transformer
## Referencing another SQLTransform

Interpolate a fitted transformer into a t-string and apply it to columns:
Interpolate a `SQLTransform` into a t-string to apply it to a column. The
reference inlines to a scalar expression at `fit`, so it composes with window
aggregates freely:

```python
import pandas as pd
import pyarrow as pa
from sklearn.preprocessing import StandardScaler
from sql_transform import SQLTransform

train_df = pd.DataFrame(
{"age": [10.0, 20.0, 30.0, 40.0], "income": [1.0, 2.0, 3.0, 4.0]}
)
table = pa.Table.from_pandas(train_df)
table = pa.table({"age": [10.0, 20.0, 30.0, 40.0]})

sc = StandardScaler().fit(train_df) # fit on a DataFrame -> records feature_names_in_
t = SQLTransform(t"SELECT {sc}(age, income) AS scaled FROM __THIS__").fit(table)
```

**Output is a single Arrow struct column**, not one column per feature:

```python
t.transform(table).schema # scaled: struct<age: double, income: double>
t.transform(table).flatten().schema.names # ['scaled.age', 'scaled.income']
```

Call `.flatten()` to get flat columns for an sklearn handoff.

**Column binding** depends on how the transformer was fitted:

| fitted with | `feature_names_in_` | binding |
|---|---|---|
| `fit(DataFrame)` | recorded | by **name** — call order is free, and is validated against the names |
| `fit(ndarray)` | absent | by **position**, in call order — only the count is checked |

With positional binding, `{sc}(income, age)` against a transformer fitted as
`[age, income]` silently swaps the features. Fit on a DataFrame when you can.

Aggregating over a transformer's output (`AVG({sc}(age)) OVER ()`) is not
supported — it is inherently two-stage and needs a subquery. Aggregate over an
input column, or use a `SQLTransform` reference, which inlines to a scalar and
composes with aggregates freely:

```python
norm = SQLTransform("SELECT age / MEAN(age) OVER () AS a FROM __THIS__").fit(table)
t2 = SQLTransform(
t = SQLTransform(
t"SELECT AVG({norm.transform}(age)) OVER () AS m FROM __THIS__"
).fit(table)
t2.transform(table).column("m").to_pylist() # [1.0, 1.0, 1.0, 1.0]
t.transform(table).column("m").to_pylist() # [1.0, 1.0, 1.0, 1.0]
```

`{norm.transform}` reuses the referenced transform's already-frozen state. A bare
`{norm}` instead re-fits that transform's *definition* into the outer fit, leaving
the referenced object untouched. References nest — `{a}({b}(age))` — and resolve
innermost-first.

> **Note:** referencing a fitted **sklearn** transformer is not currently
> supported. That surface was removed in TASK-40 and may return later.

## Development

```bash
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ status: To Do
assignee:
- Ritchie
created_date: '2026-07-19 16:08'
updated_date: '2026-07-23 14:32'
updated_date: '2026-07-25 01:23'
labels:
- codegen
- transformer
Expand Down Expand Up @@ -77,4 +77,14 @@ created: 2026-07-23 04:41
---
PRE-AUTHORIZED (2026-07-23): AmirHossein green-lit dispatching TASK-34 to Ritchie automatically once TASK-29 lands. No further approval needed — PM dispatches on TASK-29 completion. (Dependency TASK-29 still In Progress.)
---

author: Claude (TASK-40)
created: 2026-07-25 01:23
---
OBSOLETED BY TASK-40 (2026-07-25), archiving. This ticket adds codegen support for the transformer-ref feature; TASK-40 removes that feature from both engines, so there is nothing left to port. The `__tfm_` UnsupportedInCodegen branch this ticket was meant to replace is deleted too.

Its standing PRE-AUTHORIZATION to auto-dispatch to Ritchie on TASK-29 completion is retired -- do NOT dispatch when TASK-29 lands.

Recoverable: archived, not deleted. If transformer support returns, write a fresh ticket against the new surface rather than un-archiving this one (its AC#2/#3 reference resolve_transformers and TASK-31's parity test, both of which are gone).
---
<!-- COMMENTS:END -->
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
---
id: TASK-3
title: Transformer-refs (Part-2 authoring surface) review follow-ups
status: In Progress
status: Done
assignee:
- Wren
created_date: '2026-07-18 13:44'
updated_date: '2026-07-23 14:32'
updated_date: '2026-07-25 01:23'
labels:
- python
- transformer-refs
Expand Down Expand Up @@ -69,4 +69,24 @@ created: 2026-07-23 13:09
---
Dispatched to Wren (2026-07-23) per the standing pre-authorization, on TASK-2 completion. 6 ACs — mostly DX/guardrails on the shipped authoring surface. Note AC#6 is a README/docs item and AC#4 is self-described low value; Wren to flag if any AC looks not-worth-it rather than padding it out.
---

author: Claude (TASK-40)
created: 2026-07-25 01:23
---
CLOSED BY TASK-40 (2026-07-25). The five fixes that were completed landed on master (commits d47d2df..a83b742: bind names on both ref paths, probe in CALL order, nested refs declare the struct they receive, probe in fitted order + engine tolerance, shared _rows_equal).

ACs #1-#6 are now VOID, not deferred: TASK-40 removes the {transformer}(col) authoring surface entirely, so the double-.transform() probe, the feature_names_in_ footgun, the aggregate-over-output error, and the struct-output README note all describe code that no longer exists. Re-open a fresh ticket against the new surface if transformer support returns.

Important: this ticket carried a standing PRE-AUTHORIZATION to dispatch to Wren. Closing it retires that authorization -- do not dispatch.
---
<!-- COMMENTS:END -->

## Final Summary

<!-- SECTION:FINAL_SUMMARY:BEGIN -->
Partially completed, then superseded by TASK-40 (remove transformer support).

LANDED (on master): probe/binding correctness fixes for the transformer-ref path -- bind names on both ref paths with an actionable error when unsettable; probe in CALL order so the UDF signature matches the named_struct the SQL builds; nested refs declare the struct they actually receive; probe in fitted order and compare engines with a tolerance; test cleanup to the shared _rows_equal helper.

NOT DONE, AND NOW VOID: ACs #1-#6. All six target the {transformer}(col) authoring surface, which TASK-40 deletes from both the Python front-end and the native interpreter. They are not deferred work -- the code they describe is gone.
<!-- SECTION:FINAL_SUMMARY:END -->
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ status: Done
assignee:
- Ritchie
created_date: '2026-07-19 15:35'
updated_date: '2026-07-23 00:53'
updated_date: '2026-07-25 01:24'
labels:
- test
- sql-surface
Expand Down Expand Up @@ -46,6 +46,12 @@ created: 2026-07-19 15:53
---
Done per Ritchie's merge cb8c350. Mutation-verified (broke the recursion, saw the test go red on non-first branches, restored) — meets the 'must catch the regression' bar exactly. Answering Ritchie's codegen question: your call is correct — transformers are native-only, so a native parity test + a codegen defers-loudly (UnsupportedInCodegen) assertion is the right shape of 'both engines'. Nothing to change; don't fabricate codegen computation for a native-only feature. AC #2 accepted under that corrected premise.
---

author: Claude (TASK-40)
created: 2026-07-25 01:24
---
NOTE (2026-07-25): this ticket's test file, tests/test_transformer_case.py, was deleted by TASK-40 along with the transformer-ref feature it exercised. Ticket stays Done -- the work was real and correct when it landed; the surface it covered no longer exists. Native/codegen parity for CASE itself remains covered by tests/test_diff_case.py and tests/test_case_window_integration.py.
---
<!-- COMMENTS:END -->

## Final Summary
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
---
id: TASK-40
title: Remove transformer support from SQLTransformer and the native interpreter
status: In Progress
assignee:
- '@Claude'
created_date: '2026-07-25 01:09'
updated_date: '2026-07-25 01:25'
labels:
- epic
dependencies: []
references:
- 'https://github.com/ahrzb/sql-transforms/pull/22'
priority: high
type: task
ordinal: 35000
---

## Description

<!-- SECTION:DESCRIPTION:BEGIN -->
Epic: rip out the transformer-ref / transformer-callout feature for now (v0, no backward compat — delete outright, no deprecation shims).

Scope, two removals:

1. **SQLTransformer surface (Python)** — remove the ability to reference fitted sklearn transformers inside a composition:
- `sql_transform/_transformer_ref.py`, `sql_transform/_transformer_udf.py` (delete)
- `is_transformer` branches in `sql_transform/_compose.py` (Ref.is_transformer, unfitted-transformer guard, callout build path)
- transformer paths in `sql_transform/_batch.py`, `sql_transform/_codegen/engine.py`, `sql_transform/__init__.py` exports
2. **Native interpreter (Rust)** — remove the transformer callout / UDF machinery:
- `Expr::Transform` and related plumbing in `src/expr.rs`, `src/plan.rs`, `src/types.rs`, `src/schema.rs`, `src/lib.rs`
- regenerate `sql_transform/_interpreter.pyi`

Tests to delete: `tests/test_transformer_udf.py`, `tests/test_transformer_ref.py`, `tests/test_transformer_case.py`, `tests/test_transformer_callout_infer.py`, `tests/test_diff_transformer_callout.py`, plus transformer cases in `sql_transform/_compose_test.py` and `tests/test_diff_errors.py`.

Ticket interactions to resolve before/while dispatching:
- TASK-3 (In Progress, transformer-refs follow-ups) works on the exact surface being deleted — needs a stop/land decision.
- TASK-34 (To Do, codegen transformer support) becomes obsolete — archive when this epic lands.
- TASK-31's parity test gets deleted with the feature.

Docs: update SQL_SUPPORT.md / README where transformer refs are mentioned; the design specs under docs/superpowers/specs stay as history.
<!-- SECTION:DESCRIPTION:END -->

## Acceptance Criteria
<!-- AC:BEGIN -->
- [x] #1 No transformer-ref / callout code paths remain in sql_transform (grep for is_transformer, transformer_ref, transformer_udf comes back empty)
- [x] #2 No Expr::Transform / callout machinery remains in src/*.rs; native extension rebuilds clean
- [x] #3 All transformer tests removed; full test suite passes on both engines (codegen + native)
- [x] #4 TASK-3 resolved (stopped or landed) and TASK-34 archived or re-scoped
- [x] #5 Docs no longer advertise transformer refs
<!-- AC:END -->

## Implementation Notes

<!-- SECTION:NOTES:BEGIN -->
Removal done on branch `claude/remove-transformer-support-deb837`; PR open, ticket stays In Progress until it merges.

PYTHON SURFACE
- Deleted `_transformer_ref.py` (239 lines: is_transformer, _bind_names, _probe, resolve_transformer_refs) and `_transformer_udf.py` (84 lines: the DataFusion ScalarUDF wrapper).
- `_compose.py`: dropped `Ref.is_transformer`, the `is_transformer(v)` branch, and the transformer-shaped-but-unfitted guard. A sklearn object now falls through to the existing TypeError, which names the type: "interpolation {sc} must be a SQLTransform or its .transform, got StandardScaler". SQLTransform-to-SQLTransform composition is untouched.
- `__init__.py`: dropped `_udf_specs`, the sqlt/tfm ref split, and the `transformers=` kwarg to InferFn; `inline_references` now takes `self._refs` whole.
- `_batch.py`: dropped the `transformers` param and UDF registration loop.
- `_codegen/engine.py`: dropped the `__tfm_` UnsupportedInCodegen branch (an unknown function is now uniformly a ValueError).

NATIVE INTERPRETER
- `expr.rs`: removed the `Expr::Transform` variant and its ~90-line eval arm (the GIL-attach / feature reorder / .transform() / .tolist() marshalling path).
- `lib.rs`: removed `ResolvedTransformer`, `read_feature_names_in`, the 70-line `resolve_transformers` tree rewrite, and the `transformers` constructor param.
- `types.rs`, `plan.rs`: removed the `Expr::Transform` arms from infer_type and validate_expr.
- `schema.rs`: removed `arrow_schema_to_ordered_fields` (its only caller was the transformer path).
- Unused imports cleaned: `Arc` + `FieldType` in expr.rs, `HashSet` in types.rs.

VERIFICATION
- `cargo check`: clean, zero warnings.
- `uv run pytest`: 523 passed, 12 xfailed (the xfails are pre-existing, unrelated to transformers).
- `ruff check` + `ruff format --check`: clean.
- Ran the rewritten README example end to end: transform() gives [1.0, 1.0, 1.0, 1.0], infer() gives m=1.0, a bare `{t}` re-fit ref gives [0.4, 0.8, 1.2, 1.6], and a fitted StandardScaler is rejected with the expected TypeError.
- `cargo fmt --check` reports 11 diffs, all pre-existing (master has 13 in the same files); cargo fmt is not in the project's `mise run fmt` pipeline, so the crate was never fmt-clean. Not touched -- reformatting would bury the deletion in noise.

TICKET FALLOUT
- TASK-3 closed Done: its five landed commits are on master, ACs #1-#6 are void (they describe deleted code). Its standing pre-authorization to dispatch to Wren is retired.
- TASK-34 archived: it exists to port this feature to codegen, so its premise is gone. Its pre-authorization to auto-dispatch to Ritchie when TASK-29 lands is retired -- that was the urgent part, otherwise a dev gets sent at deleted code.
- TASK-31's parity test (tests/test_transformer_case.py) was deleted with the feature; TASK-31 stays Done with a note.

DOCS
- README: replaced the "Referencing a fitted sklearn transformer" section with "Referencing another SQLTransform" (the surviving composition feature, with a verified runnable example), and noted the sklearn surface is gone for now. Design specs under docs/superpowers/specs and the backlog docs/decisions/drafts are left as history.
<!-- SECTION:NOTES:END -->

## Comments

<!-- COMMENTS:BEGIN -->
author: Claude
created: 2026-07-25 01:25
---
PR open: https://github.com/ahrzb/sql-transforms/pull/22 (+152 -1616 across 24 files). All 5 ACs met and verified; ticket stays In Progress until the PR merges.
---
<!-- COMMENTS:END -->
13 changes: 2 additions & 11 deletions sql_transform/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
from sql_transform._schema import synthesize_this_model
from sql_transform._sql import find_window_aggregates, parse_and_validate
from sql_transform._state import build_state_tables
from sql_transform._transformer_ref import resolve_transformer_refs

__all__ = ["InferFn", "SQLTransform"]

Expand Down Expand Up @@ -64,7 +63,6 @@ def __init__(
self._state_tables: dict[str, pa.Table] | None = None
self._rewritten_sql: str | None = None
self._infer_fn: InferFn | None = None
self._udf_specs: dict[str, tuple] = {}

@classmethod
def from_file(cls, path: str) -> SQLTransform:
Expand All @@ -84,11 +82,7 @@ def fit(
ctx = datafusion.SessionContext()
ctx.from_arrow(table, name="__THIS__")

sqlt_refs = {n: r for n, r in self._refs.items() if not r.is_transformer}
tfm_refs = {n: r.transform for n, r in self._refs.items() if r.is_transformer}
self._udf_specs = resolve_transformer_refs(tree, tfm_refs, table)

inline = inline_references(tree, sqlt_refs, ctx, table)
inline = inline_references(tree, self._refs, ctx, table)
windows = find_window_aggregates(tree)

own_state = build_state_tables(
Expand All @@ -102,9 +96,6 @@ def fit(
self._rewritten_sql,
row_tables={"__THIS__": this_model},
static_tables=self._state_tables,
transformers={
n: (obj, out_s) for n, (obj, in_s, out_s) in self._udf_specs.items()
},
)
return self

Expand All @@ -116,7 +107,7 @@ def transform(self, table: pa.Table, /) -> pa.Table:
the Rust engine instead."""
if self._infer_fn is None:
raise RuntimeError("Must call fit() before transform")
out = run_batch(self._rewritten_sql, table, self._state_tables, self._udf_specs)
out = run_batch(self._rewritten_sql, table, self._state_tables)
if self._output == "dense":
return _table_to_dense(out)
return out
Expand Down
5 changes: 0 additions & 5 deletions sql_transform/_batch.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,13 @@
import pyarrow as pa
import sqlglot

from sql_transform._transformer_udf import _transformer_udf

_ROW_ID = "__row_id__"


def run_batch(
rewritten_sql: str,
table: pa.Table,
state_tables: dict[str, pa.Table],
transformers: dict[str, tuple[object, pa.Schema, pa.Schema]] | None = None,
) -> pa.Table:
"""Execute `rewritten_sql` against `table` (as __THIS__) and every state
table (registered by name) via DataFusion, returning a pyarrow Table.
Expand All @@ -48,8 +45,6 @@ def run_batch(
ctx.from_arrow(numbered_table, name="__THIS__")
for name, state_table in state_tables.items():
ctx.from_arrow(state_table, name=name)
for name, (obj, in_schema, out_schema) in (transformers or {}).items():
ctx.register_udf(_transformer_udf(obj, in_schema, out_schema, name))
df = ctx.sql(tree.sql())
# collect() returns [] for a zero-row result, so pass the schema explicitly.
out = pa.Table.from_batches(df.collect(), schema=df.schema())
Expand Down
6 changes: 0 additions & 6 deletions sql_transform/_codegen/engine.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,12 +94,6 @@ def _emit_expr(e: Any, env: dict) -> str:
if isinstance(e, cp.Func):
fn = _BUILTINS.get(e.name)
if fn is None:
if e.name.startswith("__tfm_"):
# A transformer placeholder call: codegen has no transformer
# support (native-only, TASK-34) -- deferred surface, not an error.
raise UnsupportedInCodegen(
f"{e.name}() is not supported in codegen yet"
)
raise ValueError(f"Unknown function: {e.name}")
return f"{fn}({', '.join(_emit_expr(a, env) for a in e.args)})"
if isinstance(e, cp.Case):
Expand Down
Loading