Skip to content

feat(sql,core): non-recursive CTEs — WITH name AS (SELECT ...) SELECT ... - #352

Merged
fupelaqu merged 2 commits into
mainfrom
feature/22.5
Sep 16, 2026
Merged

fupelaqu merged 2 commits into
mainfrom
feature/22.5

Conversation

@fupelaqu

Copy link
Copy Markdown
Contributor

Epic 22, story 22.5. Makes WITH name AS (SELECT …)[, …] SELECT … parse, render to a fixed point, and route to the relational engine.

Closes #351

Sibling PR (open it second, and see the CI note at the bottom): SOFTNETWORK-APP/softclient4es-arrow#190

The design, in one line

A CTE reference IS story 22.1's derived table, marked. The grammar substitutes every bare single-part FROM/JOIN reference whose name is a CTE name into Table(name = alias, derived = Some(DerivedTable(body, alias, cte = Some(part)))), ONCE, in the withQuery parser action. Consequence: no new Statement leaf, no new resolver, no new leg kind, no new venue arm, and no new help document.

  • SingleSearch.ctes (last, defaulted) carries the WITH list for the RENDER, for validate() and for the routing predicate. DerivedTable.cte is a render marker so .sql re-emits the CTE name rather than expanding the body — an expanded render would be persisted by MaterializedViewExtension and re-parsed as a different statement.
  • ONE closure predicate: relationalClosureRequired gains exactly one disjunct, ctes.nonEmpty, and every venue inherits it with no edit (the resolveWithSchema seam, the searchAs macro, CoreDqlExtension). A WITH clause routes to the engine even when no CTE is referenced — the arrow regex classifier cannot count references and the two predicates must agree.
  • ONE descent into embedded statements: Criteria.embeddedStatements is DERIVED from story 22.2's existing subqueries walk rather than duplicating it, and SubqueryCriteria.withQuery is abstract, so a future subquery kind cannot silently opt out of the substitution.
  • Refused by name, with the remedy in the message: WITH RECURSIVE, a column list, a duplicate name, a self reference, a forward reference, a qualified CTE name, a non-SELECT body. with and recursive stay un-reserved.

Release notes

  • Binary-incompatible — downstream rebuild required. SingleSearch arity 14 → 15 (ctes, defaulted last); DerivedTable arity 2 → 3 (cte, defaulted last); SubqueryCriteria gains an abstract withQuery; Criteria gains embeddedStatements and mapEmbeddedStatements. All source-compatible.
  • RECURSIVE becomes a REPL keyword (completion) — not a parser-reserved word.
  • A statement beginning with WITH now routes to the relational engine at every engine venue, and is refused with HTTP 400 naming softclient4es-arrow-extensions without it. That is the plain REPL's behaviour by lead ruling (OQ-1).
  • SHOW CREATE-style renders of a CTE statement re-emit the WITH list, never the expanded form.
  • A CTE name shadows an index of the same name for the statement's duration — and consequently a CTE may not be named after the index it reads. WITH orders AS (SELECT … FROM orders …) is refused; write WITH orders_f AS (…). This is a documented divergence from PostgreSQL/ANSI, which bind the inner name to the base table. Stated in select.json's notes[] and limitations[].
  • deleteByQuery(index, "WITH … SELECT …") — a leading WITH was already sniffed as SQL by IndicesApi.parseQueryForDeletion, so this is a message change only: it now parses and is refused by the index check or the closure guard instead of by a lexer error.
  • A FROM-less CTE body executes at the engine through Support FROM-less SELECT of literal expressions — SELECT 1 is the connection/health idiom of Tableau and Superset and is rejected #251's handshake rewrite, i.e. it creates softclient4es_handshake on first use exactly as SELECT 1 does.
  • Corpus: superset.flightsql.w6.006 flips rejectedparses; owner and scored: residual unchanged, so the Epic-21 headline stays 56/99.

Verification

sql 1274 · core 1064 · softclient4es-sql-bridge 206 · es6bridge 206 · macrosTests 27 · + sql/compile + core/compile + macros/compile · ++ 2.12.20 core/Test/compile · CI lint line green (run with the scalafmt caches cleared so it was not vacuous).

Real Elasticsearch, REPL legs: 75 tests green on 6.8 (rest) / 7.17 / 8.18 / 9.0, including all four new section-6e rows. The single ES 6.8 cancellation is the pre-existing enrich-policy skip.

Falsification — 13 mutations, 13 reds

Every new guard was perturbed and shown to redden the suite that names it (restored by bytes): the criteria descent · the ctes.nonEmpty routing disjunct (at the sql and core venues) · the canonical alias · the CTE render · unsubstitutedCteReference · the ctes validate arm · AD-7's SELECT * see-through · shapeOf's CTE arm · the WHERE/HAVING substitution walk · the forward/self-reference check · the derived-table marker · the corpus verdict flip.

Narrowing check — and the probe was proved to fire

20,841 frozen inputs (4 corpus CSVs + every SQL-shaped literal in sql/core test sources + every help-corpus example + every whitespace-boundary prefix + stray-delimiter mutations), taken from a control tree at de8f7594 so the input set is identical and pre-existing, fed to parsers built from both trees, comparing verdict and rendered AST.

164 differing rows (82 inputs), and every one begins with the token WITH — zero narrowing on the pre-existing surface. The control was then mutated (one alternative deleted from dqlStatement) and the probe produced 32 extra differing rows.

Parse cost — parity

ParseCostProbe (per-statement; ~4× more sensitive than suite wall time), control and branch interleaved in one session, 5 rounds: sum of medians 3751.3 µs → 3719.5 µs (−0.8 %), control range [3364.9, 4402.5] vs branch [3674.0, 4149.1] — fully overlapping.

ParserSpec wall time, 5 warm runs each: control median 1.442 s (1.130–2.230) → branch 1.389 s (1.106–2.499). No timing assertion is added (#269/#270's CI-flake rule).

Worth recording: sequential measurement said +5.8 % and three interleaved rounds said +13.0 %. A three-way experiment (control / withQuery first / withQuery after fromlessSelect) showed bare SELECT at +22 % in both branch orders, i.e. the alternative order explained none of it. The grammar was not reordered.

Pre-existing, measured against a control, left alone

  • WHERE NOT (k IN (SELECT …)) does not parse (end of input expected) — a story 22.2 grammar gap; the same statement with no CTE fails identically at de8f7594.
  • A pipeline statement and a watcher carrying an HTTP input are not AST fixed points on main (their renders are). Both neighbour pins assert the TEXT fixed point and name the control.

Review

Independently reviewed (round 1): 3 MEDIUM findings fixed in 01697eddCteSubstitution.apply silently dropping a WITH list for a SelectStatement; a vacuous macrosTests row; and the self-reference message misnaming the cause for the commonest analyst idiom. Plus three smaller ones (an unpinned WITH family, an over-claiming comment, an over-claiming scaladoc).

🤖 Generated with Claude Code

fupelaqu and others added 2 commits September 16, 2026 08:49
… ...

Story 22.5. A CTE reference IS story 22.1's derived table, marked: the grammar
substitutes every bare single-part FROM/JOIN reference whose name is a CTE name
into `Table(name = alias, derived = Some(DerivedTable(body, alias, cte = Some(part))))`,
ONCE, in the `withQuery` parser action. Zero new consumer arms, no new `Statement`
leaf, no new resolver, no new help document.

- `SingleSearch.ctes` (last, defaulted) carries the WITH list for the RENDER, for
  `validate()` and for the routing predicate; `DerivedTable.cte` is a render marker
  so `.sql` re-emits the CTE NAME rather than expanding the body — an expanded
  render would be persisted by `MaterializedViewExtension` and re-parsed as a
  different statement.
- ONE closure predicate: `relationalClosureRequired` gains exactly one disjunct,
  `ctes.nonEmpty`, and every venue inherits it with no edit (the seam guard, the
  macro abort, `CoreDqlExtension`). A WITH clause routes to the relational engine
  even when no CTE is referenced — the arrow regex classifier cannot count
  references, and the two predicates must agree.
- ONE descent into embedded statements: `Criteria.embeddedStatements` is DERIVED
  from story 22.2's `subqueries` walk rather than duplicating it, and
  `SubqueryCriteria.withQuery` is ABSTRACT so a future subquery kind cannot
  silently opt out of the substitution.
- Refused by name, with the remedy in the message: `WITH RECURSIVE`, a CTE column
  list, a duplicate name, a self reference, a forward reference, a qualified CTE
  name, a non-SELECT body. `with` and `recursive` stay UN-reserved.

Corpus: `superset.flightsql.w6.006` flips rejected -> parses (owner and
`scored: residual` unchanged; the Epic-21 headline stays 56/99), pinned in CODE.

Closed Issue #351
…H list, and make the macro row a real guard

M-1 (MEDIUM, introduced by this branch). `CteSubstitution.apply`'s `case other => other`
SILENTLY DROPPED the whole WITH list for a body kind that cannot carry one, and
`apply` is PUBLIC — `unsubstitutedCteReference` names it in the very message it
gives an embedder. MEASURED on the old form: `Right(SelectStatement)`,
`ctesPresent` false, `relationalClosureRequired` false, `validate()` `Right(())`,
the render lost the WITH clause, `sources = List(a)` — the statement would have
executed against an INDEX named after the CTE. Every safety net keys on
`ctes.nonEmpty`, so dropping the list disarmed all of them at once. Now `Left`,
naming the shape and the remedy (`apply` is `flatMap`-based; the programmatic
`MultiSearch(Nil)` corner is refused for the same reason instead of passing
through). `CteSpec` pins both, plus the SingleSearch control.

S-1 (MEDIUM). The new `macrosTests` row was VACUOUS for the reason it stated:
its SQL REFERENCED its CTE, so it carried a derived table and story 22.1's arm
already aborted it — deleting `|| ctes.nonEmpty` left the suite 25/25 green. It
now uses an UNREFERENCED CTE, which only the new disjunct can reject, and the
abort MESSAGE is asserted directly (`assertDoesNotCompile` reports only THAT a
snippet failed, never why, so the shape naming was asserted by nothing).
`closureAbortMessage` moves from the trait to the companion — it is a pure
message builder, and the move is what makes it callable from a test. Falsified:
both mutations the reviewer used now go RED.

S-2 (MEDIUM). `WITH orders AS (SELECT id FROM orders ...) SELECT * FROM orders` —
the commonest analyst filter-and-rename idiom — was refused as "recursive CTEs
(WITH RECURSIVE) are not supported", which is false for an author who wrote no
recursion. ANSI/PostgreSQL/DuckDB bind the inner name to the BASE TABLE; this
engine takes AD-2 rule 3 and refuses it, so the message now says what actually
happened and how to fix it, with the load-bearing terms inside the first 120
characters. The published `select.json` note that implied the idiom works is
corrected and the divergence is stated in `limitations[]`. Semantics unchanged.

S-4: the seventh and last `keyword("WITH")` family (the watcher HTTP input) is
pinned — TEXT fixed point only, because that statement is not an AST fixed point
on `origin/main` either (control at `de8f7594`: AST false, TEXT true), exactly
like the pipeline row.
S-3: the HAVING and JOIN ON arms of the substitution walk are DEFENCE — both
positions reject a subquery today, on the control as well as here — and the
comment no longer implies they are exercised.
O-6: the `mapEmbeddedStatements` scaladoc no longer claims the compiler prevents
what it only prevents for a `SubqueryCriteria`.

Closed Issue #351

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@fupelaqu
fupelaqu marked this pull request as ready for review September 16, 2026 08:44
@fupelaqu
fupelaqu merged commit b7f4095 into main Sep 16, 2026
4 checks passed
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.

Non-recursive CTEs (WITH ... AS (SELECT ...)) are rejected by the lexer

1 participant