feat(sql,core): correlated subqueries — cross-scope resolution and routing to the relational engine (story 22.3, elasticsql half) - #341
Merged
Conversation
… them to the relational engine Story 22.3, elasticsql half: all of [a] plus [b] task B1. The arrow and jdbc halves of [b] remain. ONE scope model in `sql` (`SubqueryScope.Scope` / `chain` / `resolve` / `ScopeSource`), a POST-PASS over the AST the parser already annotated — no new `Identifier` field, so `GenericIdentifier`'s arity and its three extensions readers are untouched. It answers what a name NAMES (which source, how many scopes out), which is what the planner needs and what the un-qualified-name rules in arrow's `JoinPlanner` duplicate today. The flip: `SubqueryCriteria.commonChecks` loses its correlated `Left` and `SingleSearch.relationalClosureRequired` gains `|| hasCorrelatedSubqueries`. Every venue inherits with zero edit — `CoreDqlExtension`, the `searchAs` macro abort, the `resolveWithSchema` seam (which already read the statement-level predicate) and arrow's `JoinExtension.canHandle`. A venue without the engine refuses the statement through `RelationalClosureGuard`, naming the construct. Three defects found and fixed along the way, each measured: * `SubqueryScope.correlationNames` read `From.tableAliases`, a ListMap keyed by TABLE that has already collapsed two sources sharing an alias-map key. On `FROM orders o JOIN UNNEST(o.orders) AS i` the alias `o` was simply gone, so a body's `o.region` was not correlated, the statement did not route, and Elasticsearch read `o.region` as an object path — zero rows, HTTP 200. It now derives from `scopeOf`, which reads the lossless `aliasesToTable`. * The LATERAL walk did not descend into WHERE-subquery bodies, where the shape was caught only as a side effect of the correlated rejection this commit deletes. Without the extension the deletion would have turned a loud rejection into an accepted statement. * `GatewayApi.excerpt` caps a rejection reason at 200 characters and elides the MIDDLE, so `lateralMessage` reached the REPL without the word LATERAL. Found by executing against real Elasticsearch; no unit test goes through `GatewayApi`. The message now leads with the construct name. Also: `++ 2.12.20 core/Test/compile` was red on main (`client.StringValue` shadows the `sql` one and 2.12 prefers the enclosing package over an explicit import); fixed with aliased imports. `Parser` gains `private[sql] parseUnvalidated` beside `apply`, sharing one `grammar` helper and one `internalFailure` builder. Green: sql 1237, core 1057, macros 24, 2.12 + 2.13 compile, ES 8.18 (71 REPL + 16 completeness) and ES 7.17 (71 REPL) on real clusters. ParserSpec median 859 ms vs an interleaved same-session control on main at 862 ms. Closed Issue #340 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
fupelaqu
marked this pull request as ready for review
September 15, 2026 11:06
This was referenced Sep 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🔴 Release gate (spec PD-9), discharged rather than forgotten. This PR ships 22.3a and task B1 together, on the lead's OQ-2 ruling. B1 flips a correlated subquery from a parse-time rejection to an engine-routed statement, and the arrow half that executes it has not merged — so between this merge and arrow's 22.3b a correlated statement at a REPL with extensions is refused with a message telling the user to install a jar they already have. That is acceptable only because no release is cut in that window: the lead has ruled that no
0.24.0goes out before the end of epic 22 and its usual train, by which time arrow's 22.3b has merged. Details in the release notes below.Closes #340
What this is
Story 22.3, elasticsql scope only: all of the spec's
[a](A1–A5) plus[b]task B1. The arrow and jdbc halves of[b]— DuckDB decorrelation, the classifier, the pushdown, the capability flags — are separate work insoftclient4es-arrowandsoftclient4es-jdbc.WHERE EXISTS (SELECT 1 FROM orders o WHERE o.customer_id = c.id)and its family used to be rejected at parse with "story 22.3 … not executed yet". They now parse and route to the relational engine; a venue without the engine refuses them loudly, naming the construct.The design in one paragraph
One scope model in
sql—SubqueryScope.Scope/chain/resolve/ScopeSource— as a post-pass over the AST the parser already annotated, not a newIdentifierfield. Annotating inupdate()would have cost an arity change on a 13-field case class read by three extensions files, a second place wheretableAlias/tableare set, and an arm whose input (the enclosing statement) is not in hand there at all. The unresolved-qualifier-left-in-namesignature already is the annotation. The flip itself is two lines:SubqueryCriteria.commonChecksloses its correlatedLeft, andSingleSearch.relationalClosureRequiredgains|| hasCorrelatedSubqueries. Every venue inherits with zero edit —CoreDqlExtension, thesearchAsmacro abort,JoinExtension.canHandle, and theresolveWithSchemaseam, which already read the statement-level predicate (so OQ-5's "one identifier" turned out to be zero).Three defects found on the way, each measured
1.
correlationNameswas blind to a collapsed alias — a silent wrong answer. It readFrom.tableAliases, aListMapkeyed by TABLE that has already collapsed two sources sharing analiasKey(21.2 AD-6 / BIDC-8). OnFROM orders o JOIN UNNEST(o.orders) AS iit isListMap(orders -> i): aliasois simply gone, so a body'so.regionwas not reported as correlated, the statement did not route, and Elasticsearch reado.regionas an object path — zero rows, HTTP 200. It now derives fromscopeOf, which reads the losslessaliasesToTable, which is what makes "one resolver" true rather than aspirational. Found by the independent review. Pinned by a regression row and by the converse agreement property — every reference the resolver places in an enclosing scope must have been reported by the detector; the forward direction alone is satisfied by a detector that reports nothing.2. The LATERAL walk is load-bearing for B1, not a nicety. A derived table nested inside a WHERE-subquery body was caught only as a side effect of the correlated rejection this PR deletes. Extending
lateralOffendersto descend into subquery bodies is what keeps the deletion from turning a loud rejection into an accepted statement.3.
LATERALnever reached the user.GatewayApi.excerpt(#262) caps a parse-rejection reason at 200 characters and keeps head(120) +...+ tail(77) — it elides the middle.lateralMessagewas 241 characters and the elision landed exactly on "this is LATERAL, which is not supported". No unit test could see this: none of them goes throughGatewayApi. Found by executing against real Elasticsearch 8.18. The message now leads with the construct name, andDerivedTableSpec(22.1) /WhereSubquerySpecpins were retargeted — contract unchanged, order fixed. Generalises: the load-bearing terms of anyvalidate()message must fit in the first 120 characters.Also fixed here
++ 2.12.20 core/Test/compilewas red onmain— verified with a control worktree at10f5e4d5, same seven errors.client.StringValue/client.BooleanValueshadow thesqlones and Scala 2.12 prefers an enclosing-package member over an explicit wildcard import; no CI job and no documented build line compiles core test sources on 2.12, which is how it survived 22.2. Fixed with aliased imports.Parse cost (no regression)
Measured against an interleaved same-session control, never a quoted number. Two worktrees, alternating, 5 runs each, first (cold) run discarded:
main@ 10f5e4d)Pass 1's control is a warm-up artefact of a freshly compiled tree; pass 2 is the honest reading: 862 vs 859 ms, −0.3 %, deep inside noise.
Verification
sql1237 ·core1057 ·macrosTests24 · bridge + testkit compile ·+ sql/compile+ core/compile++ 2.12.20 sql/Test/compile++ 2.12.20 core/Test/compile· CI lint line, withscalafmtCheckfalsified (a deliberate mis-format reddens it) rather than trusted. Real clusters: ES 8.18 71 REPL + 16 completeness, ES 7.17 71 REPL. ES 6.8 / 9.0 left to CI — the diff touches no client module.Every new rejection was proved able to fail, one mutation at a time: dropping the WHERE-subquery lateral descent reddens 2 rows; reverting the closure predicate reddens 10; removing the resolver's defensive arm reddens 1; removing the unresolved-qualifier check reddens 1. Every rejection row also asserts
not startWith Parser.InternalParseFailure, so a restoredthrowcannot masquerade as a grammar rejection (21.4's lesson).Release notes
WHEREsubquery now parses. At a venue with the relational engine it will execute (once arrow's 22.3b lands); at every other venue it is refused with HTTP 400 naming "A correlated subquery" andsoftclient4es-arrow-extensions, where it used to be refused with "Correlated subquery … story 22.3 … not executed yet". Anything matching the old text breaks.DELETE/UPDATEby query with a correlatedWHEREget the same 400 — they never ran.main:JoinExtension.canHandlestill readss.from.enrichmentRequired(22.4 has not merged there either), so a correlated no-JOIN statement is not claimed by the join extension at all — it falls through toCoreDqlExtensionand the user is told to putsoftclient4es-arrow-extensionson a classpath that already has it. Loud, but a misleading dead end at the very venue meant to run the shape. (This differs from the 22.3 spec's PD-9 prediction ofNo JOIN found in query, which assumed 22.4 had landed in arrow.) Acceptable only because no release ships in this window.LATERALrejection text changed — it now leads withLATERAL is not supported:. Any downstream fixture pinning the old wording needs updating.sqlchanges ⇒ downstream rebuild on the open0.24.0-SNAPSHOTtrain:SubqueryScope.correlatedMessagedeleted (dead once the rejection moves), and new public typesSubqueryScope.{Scope, ScopeSource, PlainSource, DerivedSource, UnnestSource, Resolution, Resolved, Ambiguous, Unresolved}. No arity change to any existing case class;GenericIdentifierstays at 13.SearchApi.bareNameCorrelationbecameprivateandscopeCorrelationtook its place — bothprivate[client], so no published surface moved.select.json'snotes[]andknown_limitations.md(PD-7). The web mirror is a hand-off, not opened here.Not in scope / hand-offs
supportsCorrelatedSubqueries. 🔴 The one fact the arrow half must know:hasCorrelatedSubqueriesis relative to a scope chain, so asking an isolated body gives the wrong answer — read the top-level node'scorrelatedRefs, or walk with scopes accumulated ascorrelatedReferencesdoes.pendingon arrow#144 (two legs over one index) when 22.3b lands — named, never silently green.Criteria.embeddedStatements/mapEmbeddedStatementsdo not exist yet (22.5 has not landed), so the body walks go through 22.2'swhereSubqueries/inner. No thirdCriteriawalk was added. 22.5 must pinsubqueries.map(_.query) == embeddedStatementswhen it lands.known_limitations.md's "What a not-yet-supported query looks like" block is still stale (it says aWHEREsubquery is rejected by the parser — false since 22.2). That state was explicitly accepted by the lead; owner: 22.7.🤖 Generated with Claude Code