fix(sql): bound the LATERAL walk to the derived table, and resolve names case-insensitively - #344
Merged
Merged
Conversation
…mes case-insensitively Two defects introduced by story 22.3a (PR #341, merged today) and never released. #342 — a correlated subquery living entirely INSIDE a derived table's body was rejected as LATERAL. `lateralOffenders` reused the CORRELATION walk, which accumulates the enclosing names as it descends, so the derived body's own alias counted as "enclosing" for a WHERE subquery nested inside it. That accumulation is correct for correlation (report it at the outermost node, where routing is decided) and wrong for LATERAL (the boundary is the derived table and must not move). One walk now takes the boundary rule as a parameter, so the two questions cannot drift. Story 22.3's own AC 11 shape parses again; every reference that genuinely escapes the derived table keeps its rejection and its message. #343 — retiring the arrow planner's `resolveUnqualified` in favour of `SubqueryScope.resolve` narrowed derived-projection matching from `equalsIgnoreCase` to `Seq.contains`. `SELECT Total ... AS total` then answered "Ambiguous column" beside two plain legs and, beside one, silently resolved to the WRONG leg. Restored case-insensitively, and the halves made to agree: `Scope.byName` matches aliases and keys the same way, and `derivedScopeCheck` takes its column from the resolution instead of re-deriving it from the name. Both directions are pinned in `SubqueryScopeSpec` and `DerivedTableSpec`, and every new assertion was made RED by reverting its own fix before being trusted. Closes #342 Closes #343
…as its contract says The third narrowing from the same story-22.3 retirement, folded into #343's fix on the lead's ruling: match the documented contract. `resolve`'s scaladoc already promised that an `UnnestSource` "takes part in NO un-qualified rule", but rule (0) -- a lone source owns every bare name -- counted EVERY source, and `scopeOf` emits the UNNEST as a source of its own, where the retired planner helper counted `TableInfo`s and an UNNEST is part of one. So `FROM (SELECT * FROM x) d JOIN UNNEST(d.items) i` answered `Ambiguous` for every bare name while the same statement without the UNNEST resolved to `d`, and a derived table beside an UNNEST stopped being scope-checked at all: a column nothing projects was silently accepted where its UNNEST-free twin is refused by name. The UNNEST sources are now dropped before rule (0), not only inside rules (1)-(3). Nothing else moves: `Scope.names` still carries the UNNEST alias (story 22.2's alias-collapse pin depends on it), `byName` still resolves a qualified `i.field`, and `unresolvedMessage` still lists every source it searched. The prose was already right, so it is unchanged. Both directions pinned, and the one line reverted to confirm exactly the two new rows go red.
fupelaqu
marked this pull request as ready for review
September 15, 2026 17:31
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.
Closes #342
Closes #343
Two defects in core
0.24.0-SNAPSHOT, both introduced by story 22.3a (PR #341, merged today) andnot released. The version line is untouched.
#342 — a false-positive LATERAL rejection made story 22.3's own AC 11 unreachable
Reproduction (measured on
origin/main36f0884e, before any edit)ois declared byd's own body. The offender set says so — it holds onlyd— and the walkreported
o.idanyway.Cause
lateralOffendersreusedcorrelatedReferences, whose recursion accumulates the enclosing names asit descends (
deeper = outerScopes ++ innerNames). Accumulating is right for the CORRELATIONquestion — a body reading any enclosing scope must be reported at the outermost node, where routing
is decided — and wrong for the LATERAL question, where the boundary is the derived table and must
not move. By the time the walk reached the WHERE subquery nested inside
d's body,d's ownohad been folded into the "outer" set one level up.
Fix
One walk,
escapingReferences(body, outerScopes, accumulate); the boundary rule is the parameter, sothe two questions cannot drift into two traversals.
correlatedReferencesisaccumulate = true(unchanged behaviour);
lateralOffendersisaccumulate = false, where the set only ever SHRINKS bythe names each level declares.
What stays rejected, and the pin that proves it
JOIN (SELECT o.cid FROM orders o WHERE o.cid = c.id) dFROM (… WHERE o.cid = customers.id) d, customersc.id… (SELECT y FROM (… WHERE o.cid = c.id) e) d)e… WHERE u.k = d.k) d)relationalClosureRequired == trueDerivedTableSpec's two pre-existing LATERAL rows needed no retargeting — they are the"still rejected" direction and stayed green byte-for-byte.
#343 —
SubqueryScope.resolvenarrowed derived-projection matching to case-sensitiveReproduction (measured on
origin/main36f0884e)Retiring the arrow planner's
resolveUnqualified(which matched with_.equalsIgnoreCase(name)) infavour of
resolve(which matches withSeq.contains) narrowed resolution in two ways:The halves already disagreed: the consumer maps the answer back to a
TableInfowithalias.equalsIgnoreCase(source.alias), so the ALIAS side was case-insensitive while the PROJECTIONside was not.
Fix, and the quoted-identifier decision
Scope.byNameand the projection lookup both match case-insensitively, restoring what the retiredhelper did.
derivedScopeChecknow takes its column head fromResolved.columninstead ofre-deriving it from
Identifier.name— without that, the widenedbyNamewould have compared theQUALIFIER (
DofD.total) against the projection and refused a statementresolvehad justresolved.
A quoted name is matched the same way, deliberately. ANSI would compare a delimited identifier
exactly, but (a) the projection side carries no quoting information —
DerivedTable.outputNamesis aSeq[String]— so an exact rule could only be applied to one half, and"Total"would then resolveagainst
Totalexactly whiletotalresolved case-insensitively; and (b) the helper this restoreswas case-insensitive for every operand, quoted or not, so exempting quoted names would be a NEW
narrowing with no mandate. Nothing is rewritten by the choice:
Resolved.columncarries the caller'sown spelling, so the column reaches DuckDB / Elasticsearch exactly as written — this decides WHICH
SOURCE owns a name, never how it is spelled. Pinned.
What stays rejected
resolve(nope)over two plain legs is stillAmbiguous;SELECT d.nope FROM (SELECT amount AS total FROM t) dstill gets "Column 'nope' is not projected by derived table 'd' (it projects: total)".Third fix — an UNNEST leg changed what a bare name resolved to (no separate issue: same regression as #343)
🔴 This carries no
Closes #Nof its own. It is the same retirement regression as #343 — story22.3 replaced the planner's
resolveUnqualifiedwithSubqueryScope.resolveand the replacement wasnarrower in three places, of which #343 names the case-sensitivity. Folded in here on the lead's
ruling rather than minted as a third issue, so the PR's issue coverage stays honest: two issues, three
regressions, all from one change.
Reproduction (measured on this branch with the first two fixes already in)
resolve's own scaladoc already promised that anUnnestSource"takes part in NO un-qualifiedrule". Rule (0) — a lone source owns every bare name — counted every source, and
scopeOfemits the UNNEST as a source of its own; the retired helper counted
TableInfos, and an UNNEST ispart of one. The code contradicted its own documented contract, in a resolver three repos read.
It was not only a lost resolution. A derived table beside an UNNEST stopped being scope-checked at
all, so the two halves of a twin pair disagreed:
Fix
One line: the UNNEST sources are dropped before rule (0), not only inside rules (1)–(3). The
prose was already correct, so it is unchanged — that was the point.
Both directions pinned
FROM (SELECT * FROM x) d JOIN UNNEST(d.items) i, bare named— same as the UNNEST-free controlSELECT a FROM (SELECT b FROM x) d JOIN UNNEST(d.items) iAmbiguousAmbiguouscorrelationNames/byName/unresolvedMessageThe "name on both sides" row the brief asked for is not representable, and that is the answer.
UnnestSource.projectionisNoneby construction — nothing but a mapping could know the unnestedelement's columns — so a bare name can never be known to exist on both. The nearest reachable shape
is pinned instead:
SELECT items FROM (SELECT items FROM x) d JOIN UNNEST(d.items) i, where thederived table projects the very column being unnested. It resolves to
d, and that is the rightowner:
d.itemsis the array,iis the alias of its element. Qualified resolution stillreaches the element (
byNamesearches every source,i.nameresolves) — only the un-qualified rulesdo not.
Neighbours checked (the
derivedScopeChecklesson, applied)Every consumer of
resolve/scopeOf/Scope.sourcesinsql/src/mainandcore/src/mainwasenumerated:
SingleSearch.derivedScopeCheck(sql)SearchApi.unresolvedQualifier(core)id.name.contains("."), so it only ever takes the qualified branchScope.names/correlationNamessources.flatMap(key, alias), untouched, so story 22.2's alias-collapse pin (which requires the UNNEST alias) stays greenSubqueryScope.unresolvedMessageScope.byNameNothing else re-derives a source count: the other
.sourceshits in the tree areSingleSearch.sources(index names from FROM), a different member.Falsification — the sixth perturbation
Reverting the one line reddens exactly the two new rows (
SubqueryScopeSpec+DerivedTableSpec),68 of 70 still green; restored after.
Falsification — every new assertion was made RED before being trusted
Each half of each fix was reverted in turn, the suites re-run, and the fix restored:
lateralOffendersback toaccumulate = trueSubqueryScopeSpec+DerivedTableSpec)projectsback toSeq.containsScope.byNameback to==SubqueryScopeSpec)derivedScopeCheckback to a case-sensitivecontainsDerivedTableSpec)derivedScopeCheckback to re-deriving the head fromid.nameDerivedTableSpec)resolverule (0) back to counting UNNEST sourcesEvery rejection assertion also asserts
not startWith Parser.InternalParseFailure(story 21.4:a rejection test is unfalsifiable once a boundary catch exists) — the shared
rejectshelper.Verification
sql/testcore/testmacrosTests/testsoftclient4es7-sql-bridge/test(run alone)+ sql/compile/+ core/compile(2.13.16 + 2.12.20)++ 2.12.20 sql/Test/compile/++ 2.12.20 core/Test/compileheaderCheck scalafmtSbtCheck scalafmtCheck Test/scalafmtCheckNot run, deliberately: the Dockerised per-major ES suites. The diff touches only
validate()and the scope resolver — no emission path, no client module, no bridge template — so the cheapest
reproducing surface is the unit legs above, and the ES matrix is left to CI. No claim is made about
them here.
Blast radius
In this repo: no existing pin needed retargeting. The full
sqlsuite (1242) is green,DerivedTableSpec's two LATERAL rows and theSubqueryScopeSpeclateralReferencesrows included.softclient4es-arrow(PR #188, open, built against core WITH these defects): all twelvestatements its
JoinPlannerSpec/JoinDetectorSpec/QueryPlannerSpecrows feed through corewere re-parsed on this branch — every one keeps core's verdict (
Right). Its four"Ambiguous column 'total'"/'nope'/'Total'pins and the AD-4 equivalence row all useall-lowercase identifiers, so the case-insensitivity widening cannot reach them; the LATERAL fix only
widens what parses, and a shape that never parsed cannot have been asserted.
grep -l LATERALover the
feature/22.3branch returns nothing but a local findings note.Re-run with the third fix in, since
JOIN UNNESTis live arrow territory: the check was widened toevery UNNEST-bearing statement its
JoinPlannerSpec/JoinDetectorSpec/PredicatePushdownSpec/WriteWithJoinExecutorSpecrows feed through core — the derived-plus-UNNEST refusal row(
SELECT d.id, i.product FROM (SELECT id, items FROM orders) AS d JOIN UNNEST(d.items) AS i), theUNNEST-alias attribution rows, the UNNEST-alias collision row, the pushdown row and the
INSERT … JOIN UNNEST(raw.items) AS itemrow. All keep core's verdict. The one row that rejects,SELECT * FROM orders JOIN UNNEST(items) AS item("UNNEST identifier items must be a nested field"),was control-run with all three fixes stashed and rejects identically — pre-existing, an
Unnest.validaterule this diff does not touch, and its arrow row classifies by regex without callingParserat all. Expected impact on #188: none — but it needs a core0.24.0-SNAPSHOTrepublish before its CI sees this.softclient4es-jdbc: no pin on either message. The single grep hit is a prose comment inJdbcIntegrationSpec:1303explaining the planner's "Ambiguous column" guard; the statement itguards is unaffected.
For the lead
0.24.0-SNAPSHOTafter merge — arrow fix: read local files without Hadoop in COPY INTO (JDK 23+) — R1FIX.4 / #183 #188 and the jdbc half both resolveagainst it.
folded into SubqueryScope.resolve matches a derived-table projection case-sensitively #343 rather than minted as a third issue — say the word if you would rather it carried
its own number.