Skip to content

feat: introduce pine variables (|= syntax)#38

Open
ahmadnazir wants to merge 60 commits into
masterfrom
feat/variables/1
Open

feat: introduce pine variables (|= syntax)#38
ahmadnazir wants to merge 60 commits into
masterfrom
feat/variables/1

Conversation

@ahmadnazir

Copy link
Copy Markdown
Member

Summary

Introduces variables to the pine language via a |= assignment syntax. A variable is a named pine expression that can be referenced by name in subsequent expressions.

Syntax

company | where: active = true |= active_companies
active_companies | employee | s: name

Both |= and | = (with optional space) are valid.

How it works

Instead of sending a single expression, the API now accepts a list of pine expressions evaluated sequentially. Variables defined in earlier expressions are available in later ones. The backend is stateless — no global state is touched.

{
  "expressions": [
    "company | where: active = true |= active_companies",
    "active_companies | employee | s: name"
  ]
}

Only the last expression's result is returned.

Design: Variables as Nested ASTs

A variable is a first-class entry in :tables. Real table entries look like {:table "company" :schema "public" :alias "c_0"}. Variable entries carry an additional :ast field — its presence is the discriminator:

{:table "active_companies" :schema nil :ast <nested-AST> :alias "ac_0"}

There is no separate :variable flag. :ast present → variable. :ast absent → real table.

A top-level :variables {} map is maintained in the AST state and populated incrementally as assignments are processed. This is included in the API response for debugging and frontend use.

Join resolution

The existing join-helper / join-tables code requires zero changes. Before the main pipeline runs, pre-handle augments the local :references map with entries for each variable:

  1. Inspect the variable's nested AST :columns
  2. Filter out auto-id columns to find explicitly selected columns
  3. If none explicitly selected → use the :current table only; otherwise trace each column's :alias through :aliases to find all source real tables
  4. For each source real table, copy (get-in refs [:table source-table]) into (get-in refs [:table varname])

The join system then sees active_companies exactly as it sees company — no special casing.

SQL generation

Variable tables emit CTEs. build-select-query is refactored into:

  • build-bare-select — generates SELECT ... FROM ... WHERE ... (no CTE prefix), using the variable name (no schema) for variable tables in FROM/JOIN clauses
  • collect-ctes — recurses into nested variable ASTs in topological order, deduplicates by name, returns a flat list of [name bare-sql] pairs
  • build-cte-body — like build-bare-select but strips the column-alias from the current table's auto-id column so it is exposed as plain id in the CTE (required for join conditions like active_companies.id = employee.company_id to resolve)
  • build-select-query — wraps the above: collects CTEs, prepends WITH ... AS (...) clause

Composed variables (variable referencing another variable) are handled naturally: collect-ctes recurses, all CTEs flatten to a single top-level WITH clause (PostgreSQL does not allow nested WITH).

Files changed

File Change
src/pine/pine.bnf Add ASSIGN rule: |= symbol as optional tail on OPERATIONS
src/pine/parser.clj Normalize ASSIGN node; extract :assign from ops list, return as separate key so it never enters handle-ops
src/pine/ast/main.clj Add :variables {} and :assign nil to default state; extend pre-handle to accept variables and seed references; record :assign after handle-ops; keep :variables through post-handle
src/pine/ast/table.clj Check :variables first in handle; variable entries carry :ast; join resolution unchanged
src/pine/eval.clj collect-ctes, build-cte-body, build-bare-select, updated build-select-query; FROM/JOIN use CTE name for variable tables
src/pine/api.clj api-eval and api-build accept expressions: [...]; sequential evaluation with local variable map; return last expression's result

Related

  • Frontend PR: beamlynx/beamlynx-ui#(TBD) — multi-expression editor and variable list UI

🤖 Generated with Claude Code

@Koziar

Koziar commented May 21, 2026

Copy link
Copy Markdown
Contributor

.

ahmadnazir and others added 28 commits May 21, 2026 23:48
- Add |= syntax to assign expressions to named variables
- Variables compile to CTEs; later expressions reference them as tables
- Multi-expression API: expressions[] array, last is active, preceding are context
- Fix build-query empty-expression guard (was hardcoded "x_0" alias, now checks actual table name)
- Skip auto-id columns for variable/CTE tables (only real tables get them)
- Add docs/variables.md explaining the feature

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Guard api-build and api-eval against blank/nil last expression to avoid
  Instaparse NPE ("text is null") when the input is empty
- Add technical implementation section to docs/variables.md

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
When a variable has explicit user columns (select, group, etc.), override the
column hint list with the actual CTE output columns rather than all columns of
the underlying source tables. For group operations, also add a synthetic
"count" column. Variables using * (no explicit columns) fall back to the
source table's columns as before.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…econd

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
All docs now follow: why → syntax/overview → examples → how it works →
constraints → implementation. Add auto-id.md (hidden id columns for row
tracking) and result-updates.md (editing cells in join query results).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
auto-id is an implementation detail of result-updates, not a standalone concept.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Variables don't belong to a DB schema. seed-variable-references copies the
source table's :in map, causing variables to display as e.g. "public.active_co"
in hints. Now create-hint-from-table checks against state :variables and emits
{:schema nil :table name} for any variable, giving a clean unqualified hint.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The ASSIGN span includes the leading "|" from the grammar rule. When
prettify joins operations with "\n | ", this produced "| | =name".
Strip the leading pipe from ASSIGN expressions before joining.

Also add parser tests covering assignment prettification:
- bare assignment (|= name)
- spaced assignment (| = name)
- assignment after filter (where: ... |= name)
- trailing pipe after assignment is a parse error (assignment must be last)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
api-eval was passing the raw last expression (e.g. "tenant |") directly
to generate-state/run-query, producing an empty query and a PostgreSQL
"No results returned" error. Now mirrors api-build: splits context/last,
trims pipes on the last expression, and guards on blank after trimming.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Previously seed-variable-references only gave the variable its own
entry in the refs map (inheriting the source table's relationships).
This meant 'real-table | variable' and 'variable-x | variable-y' joins
could never be resolved because the other side had no knowledge of the
variable.

New patch-variable-relations pass runs after all variables are seeded:
for every entity T (real table or variable) where T[:referred-by][S]
exists (S is the variable's source table), T[:referred-by][variable] is
also registered with the same via-details. This enables:

- T | V  and  V | T  (real table <-> variable)
- V | W  and  W | V  (variable <-> variable, when W already inherited
  S from its own source table via the seeding pass)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… hints

- Refactor patch-variable-relations to cover both :referred-by and
  :refers-to directions via a patch-direction helper, so joins like
  `employee | mytest` (where mytest wraps the parent table) resolve
  correctly.
- Fix relation-hints to nil the schema for variable entries, preventing
  hints like "y.mytest .col" when "mytest .col" is correct.
- Add tests for both join directions and both hint directions.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Two variables wrapping the same source table (e.g. company |= c1 and company |= c2)
had no join path between them because company has no self-referential FK. Added
patch-same-source-variable-joins which registers a synthetic :variable-join entry
at refs[:table v1 :referred-by v2] for each pair of variables sharing a source table
with an id column, gated on the absence of an existing join path so self-referential
FK propagation (employee/reports_to) is not disturbed.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
synthetic :variable-join entries always join on id=id so there is no
disambiguation need. Suppress the column from the generated hint so the
pine expression reads "c2" instead of "c2 .id".

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The previous test used "c1 | c2" where "c2" exactly matches the target
variable — trivially correct. Changed to "var_x | var" with var_x and
var_y as names so the test exercises partial typing (token "var" filters
to var_y without needing the full name), closer to actual UI behaviour.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
pipeline.md: add parameter table for ast/generate explaining why both
parse-tree and expression are needed, and what cursor/variables/assign do.

variables.md: clarify state threading loop (parse → generate → store);
add Pass 3 (patch-same-source-variable-joins) to join resolution section.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- |= is now a regular pipe operation (not a terminal), enabling
  'company |= x | w: active = true' — the snapshot is taken at the
  assignment point while the pipeline continues
- Variable names assigned via |= can be used as column qualifiers in
  subsequent ops within the same expression (e.g. 's: x.id', 'w: x.id = 1')
  by resolving through :pending-assignments rather than a separate map
- Adds docs/expressions.md covering multi-expression evaluation, operation
  table, variable threading, and error short-circuiting
- Updates docs/pipeline.md and docs/variables.md accordingly
- Adds what/why ns docstrings to api.clj, ast/main.clj, ast/assign.clj,
  parser.clj

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ent join, and assign snapshot

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…t-partial hints

When a partial condition/column had an explicit alias (e.g. w: e.company_id),
hints were incorrectly looking up columns from the current context table instead
of the aliased table. Also, after a comma in select-partial, hints now default
to the current context table instead of the last completed column's table.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add `alias <".">` (e.g., `e.`) as a valid partial form in s:, w:, o:,
and u! operations so the LSP can generate column hints for a specific
alias even when no column name has been typed yet.

- Grammar: new `partial-alias` rule; SELECT-PARTIAL, ORDER-PARTIAL,
  WHERE-PARTIAL (via partial-condition), and UPDATE-PARTIAL now accept
  it alongside their existing forms, including the "columns, alias."
  composite case (e.g. `s: id, e.`)
- Parser: `parse-partial-alias` helper; SELECT/ORDER-PARTIAL handlers
  rewritten to detect the partial-alias child node and attach it as
  `:partial-alias` on the op map; UPDATE-PARTIAL gets two new match
  arms; WHERE-PARTIAL encodes it inside `partial-condition`
- Hints: `generate-column-hints` reads `:partial-alias` from the
  operation for select/order-partial before falling back to current

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Cover three gaps where the partial-alias path crosses existing logic:
- s: e.id, e. → exclude already-selected alias column
- w: id = 1, e. → complete condition then alias-dot shows right table
- u! id = '1', e. → prior assignment excluded from alias-dot results

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
GROUP and LIMIT produce bounded result sets that cannot be directly
joined — further table ops after them produce malformed SQL. This
change detects when a table operation follows a GROUP or LIMIT op
within the same expression and automatically wraps the preceding query
in an anonymous CTE (__pine_0__, etc.), then continues the pipeline on
top of it.

An optional |= name between the checkpoint op and the following table
lets the user name the CTE explicitly instead of using the auto-name.

Introduces: checkpoint-op-types, reset-for-cte, seal-as-cte,
flush-checkpoint in ast/main.clj; pending-assignments lookup in
table/handle; :ast-based CTE guard in add-auto-id-columns.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
When LIMIT followed GROUP, flush-checkpoint held instead of firing —
then handle-ops overwrote the GROUP checkpoint with a LIMIT checkpoint,
silently dropping the CTE wrapping. Same bug applied to |= x | l: 1.

Fix: flush-checkpoint now fires when the incoming op is in
checkpoint-op-types (GROUP or LIMIT), not only when it's a :table op.
count: is unaffected since it is not a checkpoint op type.

Adds tests for GROUP | LIMIT (auto-named and user-named) and a
cross-expression baseline that verifies all three forms produce
equivalent SQL.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
patch-direction scanned every table in the schema (T) for every variable (v)
to find entities that already relate to that variable's source table. With a
reverse index built once (source-table -> referring entities), each variable
only visits entities that actually have a relation to patch, cutting this
from O(v · T) to O(T + v · k) where k is real fan-in per source table.

The index is threaded alongside refs and updated on every write so entries
added mid-pass stay visible — this matters for variable-of-variable
composition, where a later variable's source table is an earlier variable
processed in the same pass.
…ble-joins

Previously scanned every ordered pair among all variables regardless of
whether they shared a source table, making the has-id?/shared-sources check
run v² times even when no two variables were related. Grouping variables by
source table first means only variables that could plausibly match are ever
paired, degrading to v² only in the (rare) case where a whole group shares
one source — which is the actual output size in that case, not avoidable
overhead.
resolve-alias unconditionally prefers a |= variable's snapshot alias over a
same-named alias assigned later via 'as' in the same expression. Repro:
'company |= x | employee as x | w: x.id = 1' filters on company's id instead
of employee's — silently wrong SQL, no error. Currently failing; fix follows.
…liases

resolve-alias in where/select/order/group always translated a token through
:pending-assignments first, even when that token had since been re-bound to
a different table via an explicit `as` in the same expression. Since `as`
registers a live entry in :aliases, checking that first makes the most
recent, explicit binding win — matching what the user actually wrote.

Fixes the regression test added in the previous commit.
…hing

x.company | group: id => count | s: id throws an NPE in
hints/generate-column-hints: the group's COUNT(1) aggregate column has no
:operation-index, and (> nil (state :current-index)) blows up. select: isn't
a checkpoint-firing op, so the group's columns are still live when hints run.
Currently erroring; fix follows.
… order

group.clj built :columns via (concat merged-columns fn-columns), a lazy seq.
Later ops that grow :columns via (update :columns into ...) (e.g. select.clj)
call conj under the hood, which prepends onto a plain seq instead of
appending onto a vector — silently reversing column order so the GROUP's
aggregate column was no longer last. hints/generate-column-hints assumes the
last :columns entry is the one just typed and reads its :operation-index,
which the aggregate column doesn't have, throwing an NPE the moment a
non-checkpoint op (e.g. select:) follows GROUP.

Fixes it at the source (:columns and :group are now real vectors), plus a
defensive nil-guard in hints.clj so a column missing :operation-index for
any other reason falls back to :current instead of crashing.

Fixes the regression test added in the previous commit.
…source-variable-joins passes

Pass 2 previously described the O(v·T) "iterate every entity" scan;
Pass 3 previously described an unconditional all-pairs scan. Both were
replaced with indexed approaches (reverse index for Pass 2, source-table
grouping for Pass 3) — this updates the doc to describe the actual
mechanism instead of the superseded one. Also fixes "two passes" -> "three
passes" in the section intro, which was already inconsistent with the doc
describing three.
group: c.name | s: (no table op in between, so the checkpoint hasn't sealed
into a CTE yet) shows hints for the underlying employee table's raw schema
instead of the group's own output columns (name, count). :current still
points at the pre-group table alias since GROUP never seals/reassigns it,
and hint generation falls back to a real schema lookup for that alias.
Repros with and without an explicit |= before the trailing s:. Currently
failing; fix follows.
… scope

GROUP doesn't change :current or seal into a CTE until a table op follows
(flush-checkpoint), so hint generation for anything typed right after an
un-sealed group: (e.g. a trailing s:) fell back to generate-all-column-hints'
normal schema lookup for :current — which still names the pre-group real
table, showing its full raw column list instead of the group's own grouped
columns + aggregate.

pending-group-columns short-circuits that lookup when we're in exactly this
scope (:pending-checkpoint set, :group non-empty, hinting for :current) and
builds hints straight from the state's own :columns instead — mirroring how
seed-variable-references already derives a variable's output columns once a
checkpoint IS sealed. Applies uniformly to select/select-partial/order/
order-partial/where, since they all route through generate-all-column-hints.

Fixes the regression test added in the previous commit.
…ry equality

The prior GROUP+select test only asserted the query includes "GROUP BY",
which passes even on malformed SQL (verified: it currently emits duplicated
id columns and unrelated auto-id pollution). select:/where:/order: after an
un-sealed GROUP checkpoint don't seal it into a CTE, so build-query dispatches
on the trailing op's literal type instead of :group, silently falling through
to the plain-select builder against a group-shaped state:
- raw non-aggregated table columns and .* leak into SELECT
- WHERE/ORDER BY resolve against the stale pre-group table alias, not the
  group's own scope

The correctly-sealed cross-expression form (splitting the same pipeline so
the second expression references the GROUP by name) is used as the oracle:
it must produce byte-identical SQL to the inline form, including for an
order-partial with a trailing comma, which carries the same parsed value as
its complete counterpart and should seal identically. All four currently
fail. Fix follows.
flush-checkpoint only fired (sealed a pending GROUP/LIMIT into a CTE) for a
following table op or another checkpoint op. Any select:/where:/order: typed
directly after an un-sealed GROUP — complete or partial, e.g. an order-partial
from a dangling trailing comma — left the checkpoint pending, so build-query
dispatched on that trailing op's literal type instead of :group and silently
fell through to the plain-select builder against a group-shaped state: raw
non-aggregated columns and `.*` leaked into SELECT, and WHERE/ORDER BY
resolved against the stale pre-group table alias instead of the group's own
scope.

checkpoint-consuming-op-types now fires on select/select-partial/where/
where-partial/order/order-partial uniformly — partial vs complete makes no
difference here, since a partial with a value already carries whatever was
fully typed before the trailing comma. count/delete/update stay excluded:
they build their own wrapper query generically and were never broken.

This also fixes the root duplication behind the previous commit's hints fix:
variable-output-columns and the hints-only pending-group-columns special case
were two independent implementations of "what columns does this GROUP
actually expose", and only one of them was correct. With checkpoint sealing
now covering these ops, every hint request goes through the same sealed-CTE
path the working cross-expression case always used — so pending-group-columns
is removed as redundant, and the one remaining implementation
(variable-output-columns) is fixed: it was unconditionally appending "count"
whenever op-type was :group, even though group.clj already folds the
aggregate into :columns when one exists (`=> count` is optional in the
grammar), so it was double-counting when an aggregate was given and
fabricating one when it wasn't.

Also aligns two of the new regression tests to include an explicit |= so the
inline and cross-expression forms share a CTE name and compare byte-for-byte,
and adds a fourth case (no |=) confirming auto-named sealing works too.
Previously described firing as table-op-or-checkpoint-op only, and stated
"ORDER does not create a checkpoint" in a way that read as ORDER never
seals — no longer accurate now that select:/where:/order: (complete or
partial) also fire a pending checkpoint. Adds an ORDER checkpoint example
mirroring the existing GROUP/LIMIT ones, clarifies the count:/delete:/
update: exclusion is about not needing sealing (not about being a
non-table op), and documents the new checkpoint-consuming-op-types set
alongside checkpoint-op-types.
variable-output-columns no longer appends count separately — it double-
counted when a GROUP had an aggregate and fabricated one when it didn't,
since :columns already includes the aggregate whenever group.clj folded
one in. Describes the corrected (read-only) behavior instead.
…erb)

No code or docs anywhere actually stated the relationship between the two
terms, which read as arbitrary/redundant to a fresh reader. Checkpoint is
the pending state; sealing is the action that resolves it into a CTE — the
same relationship as a pending transaction and its commit. No renames:
seal-as-cte is private with zero external callers and reads clearly once
the relationship is explained.
… prior blocks

Root cause of the frontend sluggishness that scaled with the number of
chained expression blocks (not fixed by the earlier CodeMirror memoization,
which addressed a real but separate per-keystroke cost).

api-build's :ast response trimmed :pending-assignments entries down to
{:tables :selected-tables :joins :columns} (matching VariableAst in
client.ts), but sent :variables untouched. A raw variable snapshot still
carries the full :variables and :references it was built from (pre-handle
seeds them, post-handle only strips :references off the *top-level* state,
never off snapshots captured mid-pipeline by assign/handle). So variable N's
snapshot embedded variable N-1's entire untrimmed snapshot inside its own
:variables key, which embedded N-2's, and so on — every additional chained
|= block re-embedded the full history of every earlier block on top of
itself, growing the response payload superlinearly instead of linearly.

Measured on a 4-block chain (tenant/company grouped, chained through three
|= assignments): the :ast payload went 843 -> 935 -> 978 -> 935 bytes after
the fix, versus 1,271 -> 26,939 -> 112,750 -> 284,287 bytes before it (a
224x blowup by the 4th block). That JSON has to be parsed and made
MobX-observable on every debounced keystroke, which is what actually scaled
with expression count.

There was a second, independent leak at the same root cause: any
variable-backed table entry (in :tables/:selected-tables, added by
table.clj's handle-as-variable for the query builder's CTE generation)
carries a full :ast — the wrapped variable's own var-ast. Trimming only the
:variables map wasn't enough; each table entry needed the same trim
(select-table), both at the top level and recursively inside each trimmed
variable's own :tables/:selected-tables, or a variable-of-variable chain
reintroduced the exact same recursive embedding one level down.

select-var-ast and select-table are the two shared trims, replacing the
inline select-keys logic that only handled :pending-assignments. The :query
field is unaffected — it's built from the original untrimmed state, which
still needs the full :ast-carrying table entries for eval.clj's CTE
generation.

New test/pine/api_test.clj (api.clj had no test file before this) verifies
variable/table entries never carry :variables/:references/:ast, and pins
the payload size for a 4-block chain under 5KB as a regression guard.
Left un-reformatted by an earlier commit; clj -M:fmt fix catches it now.
select-table/select-var-ast/build-ast read as plain projection, but the
whole reason they exist is to cut away recursive, self-referential growth
(a variable's embedded :variables/:references, a table's embedded :ast) —
prune-table/prune-var-ast/prune-ast names that intent directly.

Also replaces the brittle "<5000 bytes" regression guard in api_test.clj
with a structural invariant: an earlier variable's own pruned entry must be
byte-for-byte identical whether it's standing alone or more blocks have
chained onto it since. That's the actual property the bug violated (entries
kept growing as more blocks chained on) — a byte ceiling was an arbitrary
proxy for it that could drift or break for unrelated reasons.
…lper

The prior test called #'api/prune-ast directly, which only proves the
helper itself behaves correctly — it proves nothing about whether api-build
still routes its result through prune-ast at all, or would catch a future
edit that forgets to call it, passes the wrong state, or leaks a new field
through some other path. None of that wiring was under test.

api-build couldn't be called directly in tests before this: it calls
connections/get-connection-name, which requires a real registered
connection pool and throws "Connection not found" for :test. :test was
already a recognized bypass for schema lookups (postgres.clj, used
throughout the existing test suite) but not for the connection-name lookup.

Rather than hardcode a second (= id :test) check in connections.clj,
test-connection-id/test-connection? are now defined once in connections.clj
(the lowest-level db namespace, with no dependents in this graph) and
postgres.clj's schema lookup calls the same predicate instead of its own
copy. api-build and api-eval can now be called directly with connection-id
:test, so the test exercises the real public entry point end to end.
parser.clj defaults :functions to ["count"] whenever => is omitted — there's
no syntax for a truly aggregate-less GROUP. Found while tracing a variable's
join behavior: the code change (reading :columns as-is, never appending
count separately) was already correct either way, but the docstring/doc
claimed a case that doesn't actually occur.
…d survives

get-source-tables currently treats any table referenced by an explicit
column as a join source, regardless of whether that table's own id column
is among the explicit columns. Since a variable's CTE never gets an auto-id
added (post-handle, which adds it, runs after handle-ops — after any |=
snapshot is already taken), a variable with explicit columns that don't
include a table's id has no way to actually join back to it. Joining
inherits that table's FK relations anyway, producing a query that
references e.g. "x"."id" — a column that doesn't exist in the CTE.

Confirmed this isn't GROUP-specific: `s: name` (no group involved) hits the
exact same bug as `group: name`. `s: id, name` / `group: id, name` are fine
since id survives. Two of the four cases here currently fail (the two
missing id); fix follows.
get-source-tables previously treated any table referenced by an explicit
column as a valid join source, regardless of whether that table's own id
column was among the explicit columns. A variable's CTE never gets an
auto-id column added (post-handle, which adds it, runs after handle-ops —
after any |= snapshot is already taken), so joining in a relation inherited
from a table whose id isn't actually in the CTE's output produces a query
referencing a column that doesn't exist (e.g. "x"."id").

This wasn't GROUP-specific, despite first looking that way: `s: name` hits
the identical bug without any GROUP involved, since explicit column
selection (via s: or group:) is the actual trigger, not the operation type.
No explicit columns (implicit '*') was always safe and is unaffected.

Now filters explicit columns down to ones literally named "id" before
mapping to tables, matching the id-based join convention used everywhere
else in this file (patch-same-source-variable-joins's has-id? check). Can
return zero sources (nothing joinable, correctly disabling hints/joins
entirely), one, or several — e.g. `s: t.id, c.id` makes both valid,
mirroring the multi-table joins a plain non-variable pipeline already
supports.

Fixes the two previously-failing cases in the last commit's regression test.
Updates "Join resolution through variables" to lead with what
get-source-tables actually decides (a table is a join source only if its id
survives among the variable's exposed columns, zero/one/many tables can
qualify) before describing the three passes that build on it — they were
previously described as if any referenced table were automatically a
source. Also annotates the existing "Grouped" example, which is exactly
this scenario: grouping by title alone leaves neither tenant nor company
joinable through x.
…g explicit id

The previous commit made get-source-tables require a table's id to be
explicitly selected before treating it as a join source — correct for GROUP,
but overly conservative for everything else: a plain `s: name |= x` doesn't
need explicit id in an ordinary (non-variable) pipeline, because post-handle
silently adds a hidden auto-id column regardless of what was explicitly
selected. That's the actual reason "you don't need explicit id to join" is
true in general — and it wasn't happening for variable/checkpoint snapshots
at all, since assign/handle and flush-checkpoint's auto-named branch take
their snapshot mid-fold, before post-handle ever runs.

Fixes the root cause instead: assign/handle and flush-checkpoint's
auto-named branch now run select/add-auto-id-columns on the snapshot the
same way post-handle would, guarded by the same should-add-auto-ids? check
that already excludes :group (along with :count/:delete-action/
:update-action). So:

- Non-GROUP snapshots always end up with an id for every real table in
  :tables, restoring the original permissive behavior for plain s:/where/
  table variables — now backed by an id that's actually in the CTE, not an
  assumption.
- GROUP snapshots still only get an id if the user explicitly grouped by
  it — there's no way to silently add an unaggregated column to a GROUP BY
  without changing what's grouped by, which is why this is the one
  operation that genuinely needs the explicit-id check.

get-source-tables no longer needs to special-case operation type at all: it
just filters :columns (including auto-id entries now) for "id" instead of
explicit-only columns. GROUP naturally has no auto-id entries to find;
everything else naturally does.

Updates the first hints_test.clj case from the previous commit: `s: name`
now correctly shows employee as joinable again (auto-id fixes it for real),
matching what a non-variable pipeline already does. The GROUP-without-id
case stays correctly disabled.
… gate

Previous doc update described get-source-tables as uniformly requiring
explicit id for any table with explicit columns — accurate right after that
commit, but superseded by the follow-up fix: auto-id is now added to
non-GROUP snapshots the same way an ordinary pipeline gets it via
post-handle, so explicit id is only actually required for GROUP. Rewrites
the section to explain why that asymmetry exists (post-handle's
should-add-auto-ids? guard, which was already excluding :group) instead of
presenting a single flat rule.
The previous fix reused select/add-auto-id-columns to make sealed CTEs
joinable — it happened to produce correct SQL, but for the wrong reason:
auto-id columns exist so the UI can identify which row to update (see
result-updates.md), an unrelated concern. Coupling the two meant a future
change to update-tracking (e.g. making it opt-out) could silently break
variable/checkpoint joins.

The actual reason a raw table join never needs explicit id in SELECT: a
JOIN ... ON clause can reference any column of a real table in FROM/JOIN
regardless of what's selected — SQL doesn't restrict that based on the
SELECT list. A CTE is different in kind, not degree: once state is sealed
into one, the outer query can only see whatever that CTE's own :columns
selected. So a table stays a valid join source through a sealed variable
only if its id is actually there — not because of anything to do with
auto-id specifically.

assign.clj now owns this as its own mechanism: preserve-join-keys adds a
column marked :hidden and :join-key (deliberately NOT :auto-id) for every
real table when a snapshot's explicit columns are non-empty and it isn't a
GROUP — GROUP still can't get one unless the user explicitly grouped by it,
since an unaggregated id can't be patched into a GROUP BY after the fact.
assign/snapshot (dissoc + preserve-join-keys) is the one shared entry point
for sealing state into a CTE, used identically by a |= assignment
(assign/handle) and a checkpoint's auto-named seal (flush-checkpoint in
main.clj) — the two places this actually happens.

get-source-tables and variable-output-columns now filter on :hidden
(shared by both mechanisms, for "don't show this in hints") rather than
:auto-id specifically, so they no longer care which mechanism produced a
column, only whether it's meant to be internal.

select/has-id-column? made public — a purpose-agnostic schema lookup now
shared between the two concerns, without sharing anything else. Behavior
is unchanged from the previous commit; this is a decoupling refactor, not a
bug fix, confirmed by the full suite passing unchanged plus one new test
asserting the join-key column is never marked :auto-id.
…d reuse

Updates "Join resolution through variables" for the decoupling refactor:
attributes join-key survival to assign/preserve-join-keys and assign/snapshot
instead of select/add-auto-id-columns, and leads with the actual SQL reason
a sealed CTE needs this while a raw table join doesn't (JOIN ... ON sees the
real table regardless of SELECT; a CTE's :columns is its entire visible
schema once sealed) rather than a post-handle timing detail.
Simplifies the previous commit's decoupled join-key mechanism away. The
actual rule is simpler: a table is a join source through a sealed variable
if its id is present in the CTE's :columns, however it got there — Pine
never adds one on its own for that purpose. get-source-tables only ever
checks; it doesn't need to know or care which mechanism produced the id.

For that check to find anything on a non-GROUP variable, the id needs to
already be in :columns by the time the snapshot is taken — the same
auto-id columns an ordinary query's post-handle would add work fine here,
since the only real requirement is that they run before the snapshot is
read for join resolution, not that they come from a separate mechanism.
assign/handle and flush-checkpoint's auto-named branch now call
select/add-auto-id-columns directly when building the snapshot, in that
order, for exactly that reason.

GROUP still ends up with no id unless the user explicitly grouped by one —
not through any special-casing in get-source-tables, but because
should-add-auto-ids? (select.clj) already excludes :group from getting an
auto-id column, for its own reasons. get-source-tables doesn't need to
know that either; it stays a plain, mechanism-agnostic check.

Net removal: assign/preserve-join-keys, create-join-key-column,
has-explicit-id?, the :join-key marker, and select/has-id-column?'s public
export are all gone — nothing outside select.clj needs that lookup now.
…directly

Updates "Join resolution through variables" for the simplified design:
get-source-tables is a plain, mechanism-agnostic check (is a table's id
present in :columns, however it got there), and select/add-auto-id-columns
is called directly at snapshot time rather than through a separate
mechanism. GROUP ends up different only because should-add-auto-ids?
already excludes it, not because get-source-tables special-cases it.
get-source-tables only ever checks whether a table's id is present in a
CTE's :columns; it never needed anything to actively put one there. The
add-auto-id-columns call at snapshot time was Pine still doing that on the
user's behalf for non-GROUP cases.

Simpler and now the actual rule: if the user explicitly selected a table's
id, that table is a valid join source through the variable; if not, it
isn't. No exception for non-GROUP operations. assign/handle and
flush-checkpoint's auto-named branch are back to a plain dissoc'd snapshot.

Updates the two test cases this flips: `s: name |= x` no longer shows the
sibling table as joinable (no id, no join); `s: id, name |= x` still does.
Updates "Join resolution through variables" for the final simplification:
a table is a join source only if the user explicitly selected its id, with
no exception for non-GROUP operations. Pine adds nothing on its own for
this purpose, for any operation type.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants