From a695297414e492b988f4b5f46a52575ef098ba22 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Manciot?= Date: Tue, 15 Sep 2026 11:43:53 +0200 Subject: [PATCH] feat(sql,core): resolve correlated subqueries across scopes and route them to the relational engine MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .../resources/help/commands/dql/select.json | 2 +- .../client/RelationalClosureGuard.scala | 37 +- .../elastic/client/SearchApi.scala | 75 ++++- .../elastic/client/SubqueryResolver.scala | 11 +- .../client/RelationalClosureGuardSpec.scala | 41 +++ .../elastic/client/SubqueryResolverSpec.scala | 118 ++++++- .../extensions/CoreDqlExtensionSpec.scala | 20 ++ documentation/sql/known_limitations.md | 3 +- .../elastic/sql/parser/Parser.scala | 79 ++++- .../elastic/sql/query/SubqueryScope.scala | 259 ++++++++++++-- .../softnetwork/elastic/sql/query/Where.scala | 17 +- .../elastic/sql/query/package.scala | 81 +++-- .../elastic/sql/parser/DerivedTableSpec.scala | 16 +- .../sql/parser/WhereSubquerySpec.scala | 116 +++++-- .../elastic/sql/query/SubqueryScopeSpec.scala | 318 +++++++++++++++++- .../WhereSubqueryCompletenessSpec.scala | 6 +- .../repl/ReplGatewayIntegrationSpec.scala | 29 +- 17 files changed, 1090 insertions(+), 138 deletions(-) diff --git a/core/src/main/resources/help/commands/dql/select.json b/core/src/main/resources/help/commands/dql/select.json index 99b4f0c5..4a206267 100644 --- a/core/src/main/resources/help/commands/dql/select.json +++ b/core/src/main/resources/help/commands/dql/select.json @@ -139,7 +139,7 @@ "Nested fields are addressed with JOIN UNNEST() AS ; see the JOIN clause above", "A FROM-less SELECT is evaluated against a hidden softclient4es_handshake index created on first use, so a read-only account needs it pre-created", "A derived table (a SELECT in FROM or JOIN) must carry an alias; a SELECT * body exposes an unchecked projection", - "A WHERE subquery must be self-contained: a reference to an outer alias (a correlated subquery) is refused until the relational engine executes it, and a bare column name inside the subquery is read as the subquery's own column", + "A WHERE subquery that reads an outer alias (a correlated subquery) executes through the relational engine shipped in softclient4es-arrow-extensions, and is refused with HTTP 400 at every venue without it. An outer reference must be QUALIFIED with the outer table's alias - a bare column name inside the subquery is read as the subquery's own column - and an object path inside the body should be qualified with the body's own alias", "NOT IN follows SQL: when the subquery's values contain NULL, no row matches. ANY/SOME over an empty subquery is false and ALL over an empty subquery is true" ], "limitations": [ diff --git a/core/src/main/scala/app/softnetwork/elastic/client/RelationalClosureGuard.scala b/core/src/main/scala/app/softnetwork/elastic/client/RelationalClosureGuard.scala index c687cd8c..e9a3d719 100644 --- a/core/src/main/scala/app/softnetwork/elastic/client/RelationalClosureGuard.scala +++ b/core/src/main/scala/app/softnetwork/elastic/client/RelationalClosureGuard.scala @@ -17,7 +17,7 @@ package app.softnetwork.elastic.client import app.softnetwork.elastic.client.result.ElasticError -import app.softnetwork.elastic.sql.query.{derivedTablesPresent, Statement} +import app.softnetwork.elastic.sql.query.{closureSearches, derivedTablesPresent, Statement} /** The ONE rejection every venue WITHOUT the relational engine emits for a closure-shaped statement * — a cross-index JOIN or a derived table (epic 22 AD-5; #157's discipline, widened). @@ -38,9 +38,28 @@ object RelationalClosureGuard { * did not choose. */ def shapeOf(statement: Statement): String = - if (derivedTablesPresent(statement)) "A derived table (subquery in FROM/JOIN)" + if (closureSearches(statement).exists(_.hasCorrelatedSubqueries)) CorrelatedShape + else if (derivedTablesPresent(statement)) "A derived table (subquery in FROM/JOIN)" else "A cross-index JOIN" + /** Story 22.3 — reported FIRST, for the same reason the derived table outranks the JOIN: it is + * the construct with the narrowest remedy (qualify differently, or rewrite as a JOIN), and a + * statement that carries both is refused for the one the analyst is least likely to have chosen. + */ + private val CorrelatedShape = + "A correlated subquery (a WHERE subquery that reads an outer alias)" + + /** The same rejection for a caller that holds a NODE rather than a statement — story 22.3b's + * defensive arm in `SubqueryResolver`, which is reached only through the + * `GatewayApi.run(statement: Statement)` path that never validates. ONE message, never two. + */ + def correlatedRejection: ElasticError = + ElasticError( + message = messageFor(CorrelatedShape), + statusCode = Some(400), + operation = Some("search") + ) + /** `operation` stays `"join"` on the gateway path for BOTH shapes: nothing downstream * distinguishes them (the JDBC driver relays `message` verbatim), so changing it would be a * second behaviour change with no consumer. @@ -54,13 +73,15 @@ object RelationalClosureGuard { */ def rejection(statement: Statement, operation: String = "join"): ElasticError = ElasticError( - message = - s"${shapeOf(statement)} requires the relational engine shipped in the $ExtensionJar jar " + - "(Java 11+); this venue has none, so the statement is refused rather than executed " + - s"against the first index it names. Put $ExtensionJar on the classpath (at the REPL: " + - "re-run the installer, or drop --no-extensions). See " + - "documentation/client/repl.md#extensions-cross-index-joins-materialized-views.", + message = messageFor(shapeOf(statement)), statusCode = Some(400), operation = Some(operation) ) + + private def messageFor(shape: String): String = + s"$shape requires the relational engine shipped in the $ExtensionJar jar " + + "(Java 11+); this venue has none, so the statement is refused rather than executed " + + s"against the first index it names. Put $ExtensionJar on the classpath (at the REPL: " + + "re-run the installer, or drop --no-extensions). See " + + "documentation/client/repl.md#extensions-cross-index-joins-materialized-views." } diff --git a/core/src/main/scala/app/softnetwork/elastic/client/SearchApi.scala b/core/src/main/scala/app/softnetwork/elastic/client/SearchApi.scala index 4ddc2e88..92492920 100644 --- a/core/src/main/scala/app/softnetwork/elastic/client/SearchApi.scala +++ b/core/src/main/scala/app/softnetwork/elastic/client/SearchApi.scala @@ -253,21 +253,76 @@ trait SearchApi extends ElasticConversion with ElasticClientHelpers with SchemaC private[client] def innerColumnType(inner: SingleSearch, name: String): Option[SQLType] = resolvedSchema(inner).flatMap(_.find(name).map(_.dataType)) - /** Story 22.2 (PD-2) — the SCHEMA-aware half of the correlation rule. + /** Story 22.3 (AD-3) — the SCHEMA-aware SCOPE check, generalised from story 22.2's + * `bareNameCorrelation`. Reached only for a statement the closure guard did NOT already route to + * the relational engine, so it answers the two questions parse time provably cannot: * - * A QUALIFIED reference to an outer alias is caught structurally at parse time - * (`SubqueryScope`). A BARE one cannot be: SQL resolves a bare name innermost-first, so `cid` - * inside the subquery is the INNER column whenever the inner index has it — and that is the - * right reading in the overwhelming majority of statements. When BOTH mappings are in hand, - * though, a bare name the inner index does NOT map while the outer one DOES is a correlated - * reference beyond reasonable doubt, and executing it as uncorrelated would send an unknown - * field to Elasticsearch and answer zero rows with HTTP 200 — the silent-wrong-answer mode epic - * 22 exists to close. + * 1. a BARE name the inner index does not map while the outer one DOES. SQL resolves a bare + * name innermost-first, so `cid` inside the subquery is the INNER column whenever the inner + * index has it — the right reading in the overwhelming majority of statements — but when + * BOTH mappings are in hand the other case is a correlated reference beyond reasonable + * doubt, and executing it as uncorrelated would send an unknown field to Elasticsearch and + * answer zero rows with HTTP 200. It is refused with the remedy that makes it EXECUTABLE + * since story 22.3b: qualify it, and the statement routes. 2. a QUALIFIED name that + * resolves in NO scope of the chain and is not a mapped object field of the inner index + * either. At parse time those two are indistinguishable (story 22.2's PD-2 assumes the + * object path, which is why no arm rejects it there); with the mapping in hand they are + * not, and the message NAMES every scope it searched. * * Any schema-absent condition answers `None` (assume inner — PD-2's documented boundary), so * this never turns a schema outage into a rejection. */ - private[client] def bareNameCorrelation( + private[client] def scopeCorrelation( + outer: SingleSearch, + inner: SingleSearch + ): Option[String] = + bareNameCorrelation(outer, inner).orElse(unresolvedQualifier(outer, inner)) + + /** Question 2 of [[scopeCorrelation]]. `Schema.find(head)` answers for an OBJECT field too (it + * returns the object column itself), so one lookup separates `address.city` from a typo. + */ + private def unresolvedQualifier( + outer: SingleSearch, + inner: SingleSearch + ): Option[String] = + // 🔴 An EMPTY mapping is a mapping GAP, never evidence that a name does not exist: an index + // created but not yet written to, or one whose dynamic mapping has not caught up, would + // otherwise turn every dotted object path in a subquery body into a 400. Same posture as the + // schema-absent conditions in `resolvedSchema` — a mapping we do not have must never become a + // rejection. + resolvedSchema(inner).filter(_.columns.nonEmpty).flatMap { innerMapping => + val chain = SubqueryScope.chain(inner, Seq(outer)) + inner.referencedIdentifiers.iterator + .filter(id => + id.tableAlias.isEmpty && id.table.isEmpty && !id.nested && id.name.contains(".") + ) + .flatMap { id => + SubqueryScope.resolve(id, chain) match { + // 🔴 The resolver sees a name the DETECTOR did not report. That is possible because the + // two used to read different maps, and it is exactly the shape that answers HTTP 200 + // with zero rows if it executes: Elasticsearch reads `o.region` as an object path. + // `correlationNames` now derives from `scopeOf`, so this should be unreachable — + // it is the belt for the day the two drift again, and it fails LOUD, never silent. + case SubqueryScope.Resolved(depth, _, _) if depth >= 1 => + Some( + SubqueryScope.bareCorrelatedMessage( + id.name, + inner.sources.headOption.getOrElse("the subquery's table"), + outer.sources.headOption.getOrElse("the outer table") + ) + ) + // Unresolved: a typo, or an object path. Only the mapping can tell, and it does. + case SubqueryScope.Unresolved + if innerMapping.find(id.name.split("\\.", 2)(0)).isEmpty => + Some(SubqueryScope.unresolvedMessage(id, chain, inner.sql)) + case _ => None + } + } + .toSeq + .headOption + } + + private def bareNameCorrelation( outer: SingleSearch, inner: SingleSearch ): Option[String] = diff --git a/core/src/main/scala/app/softnetwork/elastic/client/SubqueryResolver.scala b/core/src/main/scala/app/softnetwork/elastic/client/SubqueryResolver.scala index 2291986c..cbe51753 100644 --- a/core/src/main/scala/app/softnetwork/elastic/client/SubqueryResolver.scala +++ b/core/src/main/scala/app/softnetwork/elastic/client/SubqueryResolver.scala @@ -85,7 +85,7 @@ object SubqueryResolver { resolve( single, s => api.search(s)(EntityContext), // an absent key stays ABSENT — never null-filled - api.bareNameCorrelation(single, _), + api.scopeCorrelation(single, _), api.innerColumnType ) @@ -154,6 +154,15 @@ object SubqueryResolver { // Unreachable after `validate()`; `GatewayApi.run(statement)` does not validate a // programmatically built statement, so the arm is a named 400 rather than a MatchError. case None => Left(bad(s"Unsupported subquery body in ${node.sql}")) + // Story 22.3b — DEFENSIVE, and UNREACHABLE through every route that exists today: a node with + // non-empty `correlatedRefs` implies `hasCorrelatedSubqueries`, which both + // `SearchApi.resolveWithSchema` (before phase one) and `CoreDqlExtension.execute` refuse + // first. It is kept because the invariant it protects is a SILENT wrong answer if it ever + // breaks — executing a correlated body as if it were self-contained — and because a future + // caller of this object need not know the guard exists one layer up. Do not read it as + // evidence of an open hole. + case Some(_) if node.correlatedRefs.nonEmpty => + Left(RelationalClosureGuard.correlatedRejection) case Some(inner) => correlation(inner) match { case Some(reason) => Left(bad(reason)) diff --git a/core/src/test/scala/app/softnetwork/elastic/client/RelationalClosureGuardSpec.scala b/core/src/test/scala/app/softnetwork/elastic/client/RelationalClosureGuardSpec.scala index a7d88cc0..29b745ad 100644 --- a/core/src/test/scala/app/softnetwork/elastic/client/RelationalClosureGuardSpec.scala +++ b/core/src/test/scala/app/softnetwork/elastic/client/RelationalClosureGuardSpec.scala @@ -62,6 +62,8 @@ class RelationalClosureGuardSpec extends AnyFlatSpec with Matchers { private val DerivedSelect = "SELECT COL FROM (SELECT 1 AS COL) AS d" private val JoinSelect = "SELECT o.id, c.name FROM orders o JOIN customers c ON o.cid = c.id" + private val CorrelatedSelect = + "SELECT c.id FROM customers c WHERE EXISTS (SELECT 1 FROM orders o WHERE o.customer_id = c.id)" // ---- the ONE seam ------------------------------------------------------------------------ @@ -192,6 +194,45 @@ class RelationalClosureGuardSpec extends AnyFlatSpec with Matchers { RelationalClosureGuard.shapeOf(stmt) should include("derived table") } + /** Story 22.3b — the correlated shape is reported FIRST when several are present, for the same + * reason the derived table outranks the JOIN: it carries the narrowest remedy. + */ + it should "name the CORRELATED shape, and prefer it over the others" in { + val correlated = searchStatement(CorrelatedSelect) + relationalClosureRequired(correlated) shouldBe true + RelationalClosureGuard.shapeOf(correlated) should include("A correlated subquery") + val both = searchStatement( + "SELECT o.id FROM orders o JOIN customers c ON o.cid = c.id " + + "WHERE EXISTS (SELECT 1 FROM refunds r WHERE r.oid = o.id)" + ) + RelationalClosureGuard.shapeOf(both) should include("A correlated subquery") + } + + it should "refuse a correlated statement at the seam, before phase one ever runs" in { + val err = refusalOf(client().search(searchStatement(CorrelatedSelect))) + err.statusCode shouldBe Some(400) + err.operation shouldBe Some("search") + err.message should include("A correlated subquery") + err.message should include(RelationalClosureGuard.ExtensionJar) + // 🔴 falsifiable in the right direction: the resolver would have reported the INNER statement's + // own failure ("Subquery …", as the uncorrelated row above asserts). Seeing the closure message + // instead is what proves the guard ran FIRST. + err.message should not include "Subquery" + } + + it should "give DELETE ... WHERE EXISTS (correlated) the same refusal" in { + val err = refusalOf( + client() + .asInstanceOf[IndicesApi] + .deleteByQuery( + "orders", + "DELETE FROM orders WHERE EXISTS (SELECT 1 FROM refunds r WHERE r.oid = orders.id)" + ) + ) + err.statusCode shouldBe Some(400) + err.message should include("A correlated subquery") + } + it should "say the statement was refused rather than executed against the first index (PD-2)" in { val msg = RelationalClosureGuard.rejection(searchStatement(JoinSelect)).message msg should include("refused rather than executed against the first index it names") diff --git a/core/src/test/scala/app/softnetwork/elastic/client/SubqueryResolverSpec.scala b/core/src/test/scala/app/softnetwork/elastic/client/SubqueryResolverSpec.scala index d7612b2b..b04bf19e 100644 --- a/core/src/test/scala/app/softnetwork/elastic/client/SubqueryResolverSpec.scala +++ b/core/src/test/scala/app/softnetwork/elastic/client/SubqueryResolverSpec.scala @@ -18,6 +18,13 @@ package app.softnetwork.elastic.client import app.softnetwork.elastic.client.result._ import app.softnetwork.elastic.sql._ +// 🔴 Scala 2.12 resolves a member of the ENCLOSING PACKAGE over an explicit wildcard import, and +// `app.softnetwork.elastic.client` defines its OWN `StringValue` / `BooleanValue`. Without these +// aliases the file compiles on 2.13 and fails on the 2.12 leg with `found client.StringValue, +// required sql.StringValue`. MEASURED pre-existing on `origin/main` at 10f5e4d5 (a control run of +// `++ 2.12.20 core/Test/compile` reproduces the same seven errors) — no gate in CI or in the +// documented build line compiles core TEST sources on 2.12, which is exactly how it survived. +import app.softnetwork.elastic.sql.{BooleanValue => SqlBooleanValue, StringValue => SqlStringValue} import app.softnetwork.elastic.sql.`type`.SQLTypes import app.softnetwork.elastic.sql.operator.{DIFF, GT, NOT} import app.softnetwork.elastic.sql.parser.Parser @@ -67,6 +74,43 @@ class SubqueryResolverSpec extends AnyFlatSpec with Matchers { case other => fail(s"expected a success, got $other") } + /** Both mappings in the schema cache, so `SearchApi.resolvedSchema` answers for the inner AND the + * outer index (story 22.3 AD-3 needs both). + */ + private def seededClient(): ElasticClientApi = { + val client = new NopeClientApi { + override protected def logger: org.slf4j.Logger = + org.slf4j.LoggerFactory.getLogger(classOf[SubqueryResolverSpec]) + } + client.updateSchema( + "orders", + schema.Table( + "orders", + columns = List( + schema.Column("id", SQLTypes.Keyword), + schema.Column("cid", SQLTypes.Keyword), + schema.Column("amount", SQLTypes.Double), + schema.Column( + "address", + SQLTypes.Struct, + multiFields = List(schema.Column("city", SQLTypes.Keyword)) + ) + ) + ) + ) + client.updateSchema( + "customers", + schema.Table( + "customers", + columns = List( + schema.Column("id", SQLTypes.Keyword), + schema.Column("tier", SQLTypes.Keyword) + ) + ) + ) + client + } + private def errorOf(r: ElasticResult[_]): ElasticError = r match { case ElasticFailure(e) => e case other => fail(s"expected a failure, got $other") @@ -110,14 +154,14 @@ class SubqueryResolverSpec extends AnyFlatSpec with Matchers { single("SELECT id FROM t WHERE name IN (SELECT name FROM u)"), new Recording({ case _ => rows("name", "a", "b") }).execute ) - valuesOf(whereOf(s)) shouldBe StringValues(Seq(StringValue("a"), StringValue("b"))) + valuesOf(whereOf(s)) shouldBe StringValues(Seq(SqlStringValue("a"), SqlStringValue("b"))) val b = SubqueryResolver.resolve( single("SELECT id FROM t WHERE flag IN (SELECT flag FROM u)"), new Recording({ case _ => rows("flag", java.lang.Boolean.TRUE, java.lang.Boolean.FALSE) }).execute ) - valuesOf(whereOf(b)) shouldBe BooleanValues(Seq(BooleanValue(true), BooleanValue(false))) + valuesOf(whereOf(b)) shouldBe BooleanValues(Seq(SqlBooleanValue(true), SqlBooleanValue(false))) } /** 🔴 FIXED-WIDTH milliseconds, not `ISO_INSTANT`. `ISO_INSTANT` varies its fraction width, so @@ -133,7 +177,7 @@ class SubqueryResolverSpec extends AnyFlatSpec with Matchers { new Recording({ case _ => rows("created", zdt, zdt.toInstant) }).execute ) // both cells are the SAME instant, so the set collapses to one value - valuesOf(whereOf(res)) shouldBe StringValues(Seq(StringValue("2024-01-01T00:00:00.000Z"))) + valuesOf(whereOf(res)) shouldBe StringValues(Seq(SqlStringValue("2024-01-01T00:00:00.000Z"))) // and the ordering the quantified reduction reads IS chronological across fraction widths val half = zdt.plusNanos(500000000L) TermValues.sorted(Seq(half, zdt).map(TermValues.canonical)) shouldBe @@ -144,7 +188,7 @@ class SubqueryResolverSpec extends AnyFlatSpec with Matchers { ) shouldBe GenericExpression( sc.whereSubqueries.head.asInstanceOf[ScalarSubquery].identifier, GT, - StringValue("2024-01-01T00:00:00.000Z"), + SqlStringValue("2024-01-01T00:00:00.000Z"), None ) } @@ -196,7 +240,7 @@ class SubqueryResolverSpec extends AnyFlatSpec with Matchers { single("SELECT id FROM t WHERE tag IN (SELECT UPPER(tag) AS up FROM u)"), new Recording({ case _ => rows("up", List("A"), List("B", "C")) }).execute ) - valuesOf(whereOf(res)) shouldBe StringValues(Seq("A", "B", "C").map(StringValue)) + valuesOf(whereOf(res)) shouldBe StringValues(Seq("A", "B", "C").map(SqlStringValue)) } it should "keep a body's own LIMIT and bound a row-shaped no-LIMIT body at MaxTerms+1" in { @@ -547,6 +591,70 @@ class SubqueryResolverSpec extends AnyFlatSpec with Matchers { .value should be theSameInstanceAs s } + /** Story 22.3 (AD-3) — `scopeCorrelation` through a REAL client with both mappings seeded, so the + * schema plumbing (`resolvedSchema` -> the cache) is exercised, not stubbed. + */ + it should "[22.3] name every scope searched for a qualified name in NO scope" in { + val client = seededClient() + val outer = + single( + "SELECT id FROM customers c WHERE id IN (SELECT cid FROM orders o WHERE zip.code = 'X')" + ) + val inner = outer.whereSubqueries.head.inner.getOrElse(fail("no body")) + val msg = client.scopeCorrelation(outer, inner).getOrElse(fail("expected a rejection")) + msg should include("names no source in scope") + msg should include("Scopes searched (innermost first)") + msg should include("[0] o=orders") + msg should include("[1] c=customers") + } + + /** The control that makes the row above non-vacuous: `address` IS a mapped object of the inner + * index, so `address.city` is an OBJECT PATH and must pass. A check that rejected every dotted + * name would redden here. + */ + it should "[22.3] let a mapped OBJECT field through" in { + val client = seededClient() + val outer = single( + "SELECT id FROM customers c WHERE id IN (SELECT cid FROM orders o WHERE address.city = 'X')" + ) + val inner = outer.whereSubqueries.head.inner.getOrElse(fail("no body")) + client.scopeCorrelation(outer, inner) shouldBe None + } + + it should "[22.3] keep story 22.2's bare-name rule, with the QUALIFY remedy" in { + val client = seededClient() + val outer = + single( + "SELECT id FROM customers c WHERE id IN (SELECT cid FROM orders o WHERE tier = 'gold')" + ) + val inner = outer.whereSubqueries.head.inner.getOrElse(fail("no body")) + val msg = client.scopeCorrelation(outer, inner).getOrElse(fail("expected a rejection")) + msg should include("'tier' is not a column of 'orders'") + msg should include("must be QUALIFIED") + } + + /** 🔴 Story 22.3b's DEFENSIVE arm. `GatewayApi.run(statement: Statement)` never calls + * `validate()` and the seam's closure guard reads the STATEMENT, so a node handed straight to + * this object must never have its correlated body executed as if it were self-contained. The + * executor `fail`s on any call, so a regression is a test failure, not a silent pass. + */ + it should "[22.3b] refuse a correlated node outright and never execute its body" in { + // since story 22.3b this statement PARSES — the rejection moved from the parser to the venues, + // and THIS object must still never run it + val outer = single( + "SELECT c.id FROM customers c WHERE EXISTS (SELECT 1 FROM orders o WHERE o.customer_id = c.id)" + ) + outer.whereSubqueries.head.correlatedRefs should not be empty + val res = SubqueryResolver.resolve( + outer, + new Recording({ case s => fail(s"must not execute ${s.sql}") }).execute + ) + val err = errorOf(res) + err.statusCode shouldBe Some(400) + err.message should include("A correlated subquery") + err.message should include(RelationalClosureGuard.ExtensionJar) + } + it should "reject a correlated bare name through the injected mapping check" in { val outer = single("SELECT id FROM orders WHERE cid IN (SELECT id FROM customers WHERE r = 1)") val res = SubqueryResolver.resolve( diff --git a/core/src/test/scala/app/softnetwork/elastic/client/extensions/CoreDqlExtensionSpec.scala b/core/src/test/scala/app/softnetwork/elastic/client/extensions/CoreDqlExtensionSpec.scala index 6ec57e78..fb1df024 100644 --- a/core/src/test/scala/app/softnetwork/elastic/client/extensions/CoreDqlExtensionSpec.scala +++ b/core/src/test/scala/app/softnetwork/elastic/client/extensions/CoreDqlExtensionSpec.scala @@ -457,6 +457,26 @@ class CoreDqlExtensionSpec extends AnyFlatSpec with Matchers { client.searchedStatement.get() shouldBe null } + /** Story 22.3b — the CORRELATED shape reaches the SAME guard through the SAME predicate, with + * zero edit in this extension: `relationalClosureRequired` gained one disjunct and every venue + * inherited it. The falsifiable half is that NEITHER client seam is touched — a correlated + * statement that leaked past the guard would have been scrolled against the OUTER index alone, + * silently dropping the correlation (HTTP 200, wrong answer). + */ + it should "reject a CORRELATED WHERE subquery, naming the shape, and never execute it" in { + val (client, res) = run( + "SELECT c.id FROM customers c WHERE EXISTS (SELECT 1 FROM orders o WHERE o.customer_id = c.id)", + Quota.Community + ) + res shouldBe a[ElasticFailure] + val err = res.asInstanceOf[ElasticFailure].elasticError + err.statusCode shouldBe Some(400) + err.message should include("A correlated subquery") + err.message should include("softclient4es-arrow-extensions") + client.scrolledStatement.get() shouldBe null + client.searchedStatement.get() shouldBe null + } + it should "reject INSERT ... SELECT and CTAS carrying a derived table, and claim them" in { Seq( "INSERT INTO target SELECT COL FROM (SELECT 1 AS COL) AS d", diff --git a/documentation/sql/known_limitations.md b/documentation/sql/known_limitations.md index 8fafdc68..b40f44fb 100644 --- a/documentation/sql/known_limitations.md +++ b/documentation/sql/known_limitations.md @@ -65,7 +65,7 @@ compose nested queries — where the tool lets you: ## Not in this release (coming in the next release, Quarter 4 2026) -- **Subqueries**: scalar, `IN (SELECT …)`, `EXISTS (SELECT …)`, derived tables `FROM (SELECT …)`, and correlated subqueries. +- **Subqueries**: scalar, `IN (SELECT …)`, `EXISTS (SELECT …)`, derived tables `FROM (SELECT …)`. - **CTEs**: `WITH name AS (SELECT …)` — recursive and non-recursive. - **Set operators**: `UNION` (with row de-duplication), `INTERSECT`, and the `EXCEPT` **set operator**. The `EXCEPT` set operator is **distinct from** the `SELECT * EXCEPT(cols)` column-exclusion clause above — that one works; the set operator does not. - **Positional / tiling window functions**: `NTILE`, `LAG`, `LEAD` — not yet implemented; coming with the next release's analytical-SQL work. (Note: `PERCENTILE_CONT` / `PERCENTILE_DISC` — percentile *aggregates* — already work in the current release; the positional/tiling window functions are a different family.) @@ -189,6 +189,7 @@ permanent. See [STDDEV / VARIANCE family](functions_aggregate.md#function-stddev ## Coming in the upcoming release (Quarter 1 2027) - **Heterogeneous federation**: JOIN or correlate Elasticsearch with PostgreSQL, MySQL, ClickHouse, Snowflake, and more — plus cross-cluster subqueries (e.g. correlate one cluster's data against another's). + **Not this**: correlating one Elasticsearch index against **another Elasticsearch index** — `EXISTS` / `NOT EXISTS` / `IN` / `NOT IN` / a scalar comparison against a subquery that reads the outer row — is **single-cluster** and runs through the relational engine shipped in `softclient4es-arrow-extensions`. Its one rule: the outer reference must be **qualified** with the outer table's alias (`… WHERE EXISTS (SELECT 1 FROM orders o WHERE o.customer_id = c.id)`), because a bare column name inside a subquery is read as the subquery's own column. A venue without that jar refuses the statement with HTTP 400 rather than executing it as if it were self-contained. ## Deferred (a future release, demand-driven — tell us what you need) diff --git a/sql/src/main/scala/app/softnetwork/elastic/sql/parser/Parser.scala b/sql/src/main/scala/app/softnetwork/elastic/sql/parser/Parser.scala index cf68afb5..994defc3 100644 --- a/sql/src/main/scala/app/softnetwork/elastic/sql/parser/Parser.scala +++ b/sql/src/main/scala/app/softnetwork/elastic/sql/parser/Parser.scala @@ -1603,9 +1603,27 @@ object Parser */ val InternalParseFailure: String = "Internal parser error" - def apply( - query: String - ): Either[ParserError, Statement] = { + /** The grammar half of [[apply]], WITHOUT `validate()` — ONE owner of the normalisation and of + * the packrat reader construction (story 22.3, lead ruling OQ-8). + * + * `Parser.single`'s action still runs `.update()` on the statement it builds, so everything the + * UPDATE pass records (`Identifier.tableAlias` / `table`, story 22.2's `correlatedRefs`, the + * bucket names) is populated exactly as `apply` would have populated it — only the VALIDATION + * pass is skipped. That is what a test of a rule that `validate()` itself enforces needs: it + * must reach the AST of a statement `apply` refuses, and it must not re-implement the reader (a + * second construction would drift from `apply`'s, and packrat memoisation keys on the parser + * INSTANCE). + * + * `private[sql]` on purpose: nothing outside this module may execute an unvalidated statement. + */ + private[sql] def parseUnvalidated(query: String): Either[ParserError, Statement] = + grammar(query) + + /** Shared by [[apply]] and [[parseUnvalidated]]: normalise, build the packrat reader, run + * `phrase(statement)`. The `NonFatal` boundary catch (#250) wraps the WHOLE body because the AST + * surface `.update()` drags in can throw before `parse` even returns. + */ + private def grammar(query: String): Either[ParserError, Statement] = { try { val normalizedQuery = normalize(query) @@ -1635,13 +1653,8 @@ object Parser // one shape this cannot catch: `DELETE FROM orders customers` stays a valid single-table // DELETE, because an alias without AS is standard SQL — pinned as such in ParserSpec. parse(phrase(statement), reader) match { - case NoSuccess(msg, _) => - Left(ParserError(msg)) - case Success(result, _) => - result.validate() match { - case Left(error) => Left(ParserError(error)) - case _ => Right(result) - } + case NoSuccess(msg, _) => Left(ParserError(msg)) + case Success(result, _) => Right(result) } } catch { // #250. Totality is claimed for `NonFatal` only: VirtualMachineError (a StackOverflowError @@ -1676,16 +1689,48 @@ object Parser // all named JDK classes on JDK 11 - which is exactly why they are guarded rather than // trusted. `getSimpleName` also returns "" for an anonymous class, which would render // `Internal parser error: : -1`. - def safely(read: => String, fallback: String): String = - try Option(read).map(_.trim).filter(_.nonEmpty).getOrElse(fallback) - catch { case NonFatal(_) => fallback } - val className = safely(e.getClass.getSimpleName, safely(e.getClass.getName, "Throwable")) - val message = safely(e.getMessage, "") - val detail = if (message.isEmpty) "" else s": $message" - Left(ParserError(s"$InternalParseFailure: $className$detail", Some(e))) + Left(internalFailure(e)) } } + /** ONE builder for both boundary catches (#250). Two copies of this rule in one file is the + * story-21.3 desync class, one file over. + */ + private def internalFailure(e: Throwable): ParserError = { + // 🔴 Both accessors can THEMSELVES throw, which would escape the boundary this exists to + // provide: `Class.getSimpleName` raises `InternalError: Malformed class name` on JDK 8 for some + // Scala inner/anonymous classes (and the drivers promise JDK 8 for ES 6/7/8), and a custom + // `Throwable` may override `getMessage` to throw. `getSimpleName` also returns "" for an + // anonymous class, which would render `Internal parser error: : -1`. + def safely(read: => String, fallback: String): String = + try Option(read).map(_.trim).filter(_.nonEmpty).getOrElse(fallback) + catch { case NonFatal(_) => fallback } + val className = safely(e.getClass.getSimpleName, safely(e.getClass.getName, "Throwable")) + val message = safely(e.getMessage, "") + val detail = if (message.isEmpty) "" else s": $message" + ParserError(s"$InternalParseFailure: $className$detail", Some(e)) + } + + def apply( + query: String + ): Either[ParserError, Statement] = + grammar(query) match { + case Right(result) => + // 🔴 `validate()` runs INSIDE the boundary catch's sibling, not inside it: `grammar` has + // already returned, so a `validate()` that threw would escape `apply`. It cannot today — + // every `validate()` in the tree returns `Either` — and the AST surface that DOES throw + // (`bucketNames`, #253) is reached from `.update()`, which runs inside `grammar`. Guarded + // here anyway, for the same reason #250 guarded the grammar half: totality is a property + // of the METHOD, not of today's arms. + try result.validate() match { + case Left(error) => Left(ParserError(error)) + case _ => Right(result) + } catch { + case NonFatal(e) => Left(internalFailure(e)) + } + case left => left + } + } trait CompilationError diff --git a/sql/src/main/scala/app/softnetwork/elastic/sql/query/SubqueryScope.scala b/sql/src/main/scala/app/softnetwork/elastic/sql/query/SubqueryScope.scala index 969fc424..649217c8 100644 --- a/sql/src/main/scala/app/softnetwork/elastic/sql/query/SubqueryScope.scala +++ b/sql/src/main/scala/app/softnetwork/elastic/sql/query/SubqueryScope.scala @@ -42,11 +42,193 @@ import app.softnetwork.elastic.sql.Identifier */ object SubqueryScope { - /** Every name a statement's FROM makes addressable: the alias-map keys AND values (index names, + // ── The scope model (story 22.3 AD-1) ──────────────────────────────────────────────────────── + // + // A POST-PASS value object over the AST the parser already annotated — deliberately NOT a new + // `Identifier` field. Annotating in `GenericIdentifier.update` would cost an arity change on a + // 13-field case class read by three extensions files, a second place where `tableAlias`/`table` + // get set (the `TemporalLiterals` "key language" obligation would gain a third speaker), and an + // arm whose input — the ENCLOSING statement — is not in hand inside `update` at all. The + // unresolved-qualifier-left-in-`name` signature IS the annotation; this object reads it. + + /** A source a name can resolve to inside ONE statement's FROM. + * + * `key` is the alias-map KEY (`From.tableAliases` / `From.aliasesToTable` key language — the + * bare index unless the bare name is ambiguous, story 21.2 AD-6), `alias` is the correlation + * name the statement writes, and `projection` is the column list when it is KNOWN without a + * mapping. `None` there means OPAQUE — a plain index (no schema is attached at parse time) or a + * `SELECT *` derived body — and an opaque source is never guessed at. + * + * 🔴 Any consumer comparing `key` against `Identifier.table` inherits the `joinSourceKeys` + * obligation (`From.scala`, `TemporalLiterals`): the key of an ambiguous bare name is the + * QUALIFIED reference, not the index name. + */ + sealed trait ScopeSource { + def key: String + def alias: String + def projection: Option[Seq[String]] + } + + final case class PlainSource(key: String, alias: String) extends ScopeSource { + override val projection: Option[Seq[String]] = None + } + + final case class DerivedSource(alias: String, projection: Option[Seq[String]]) + extends ScopeSource { + + /** A derived table's key IS its alias (story 22.1 AD-1: `DerivedTable.name == alias.alias`). */ + override val key: String = alias + } + + final case class UnnestSource(alias: String, path: String) extends ScopeSource { + override val key: String = path + override val projection: Option[Seq[String]] = None + } + + /** One statement's scope: `depth` 0 is the statement itself, 1 its enclosing statement, and so on + * outward. + */ + final case class Scope(depth: Int, sources: Seq[ScopeSource]) { + lazy val names: Set[String] = sources.flatMap(s => Seq(s.key, s.alias)).toSet + def byName(n: String): Option[ScopeSource] = sources.find(s => s.alias == n || s.key == n) + } + + /** The sources a statement's FROM declares, in the order it writes them (FROM items first, then + * each JOIN leg). + * + * The key is taken from `From.aliasesToTable`, which is the LOSSLESS alias -> key direction + * (story BIDC-8): reading `tableAliases` instead would lose one leg of a self-join, because that + * map is keyed by TABLE and holds exactly one alias per key. + */ + def scopeOf(s: SingleSearch, depth: Int = 0): Scope = { + val from = s.from + def keyOf(alias: String, fallback: String): String = + from.aliasesToTable.getOrElse(alias, fallback) + val fromSources: Seq[ScopeSource] = from.tables.map { t => + t.derived match { + case Some(d) => DerivedSource(d.name, d.outputNames) + case None => + val alias = t.tableAlias.map(_.alias).filter(_.nonEmpty).getOrElse(t.name) + PlainSource(keyOf(alias, t.name), alias) + } + } + val joinSources: Seq[ScopeSource] = from.joins.collect { + case sj: StandardJoin => + sj.source match { + case d: DerivedTable => DerivedSource(d.name, d.outputNames) + case src => + val alias = sj.alias.map(_.alias).filter(_.nonEmpty).getOrElse(src.name) + PlainSource(keyOf(alias, src.name), alias) + } + case u: Unnest => + UnnestSource(u.alias.map(_.alias).filter(_.nonEmpty).getOrElse(u.name), u.name) + } + Scope(depth, fromSources ++ joinSources) + } + + /** The chain an INNER statement resolves against: its own scope first, then each enclosing + * statement outward. `outers` is innermost-enclosing FIRST — the order `correlatedReferences` + * accumulates scopes in. + */ + def chain(inner: SingleSearch, outers: Seq[SingleSearch]): Seq[Scope] = + scopeOf(inner, 0) +: outers.zipWithIndex.map { case (o, i) => scopeOf(o, i + 1) } + + sealed trait Resolution + + /** The identifier names `column` of `source`, `depth` scopes out (0 = the statement's own, so + * anything >= 1 IS a correlated reference). + */ + final case class Resolved(depth: Int, source: ScopeSource, column: String) extends Resolution + + /** More than one source of the innermost scope could own a bare name — never guessed. */ + case object Ambiguous extends Resolution + + /** A qualified name whose head is no correlation name in ANY scope: an object path into the + * innermost source (`address.city`) or a typo. Parse time cannot tell them apart; the seam can, + * when the inner mapping is loadable (AD-3). + */ + case object Unresolved extends Resolution + + /** SQL scoping, innermost first. + * + * QUALIFIED names walk the chain outward and take the FIRST scope declaring the head, so an + * inner declaration SHADOWS an outer one of the same name — story 22.2's rule, now a function, + * which is what keeps the detector and this resolver from drifting apart. + * + * UN-QUALIFIED names resolve in the INNERMOST scope only, by four rules (story 22.4 AD-4, + * subsumed here): (0) a lone source owns every bare name; (1) exactly one derived source + * projects the name; (2) no derived source projects it, exactly one plain source, and no OPAQUE + * derived source stands beside it; (3) else `Ambiguous`. A bare name is NEVER resolved outward: + * SQL does that only when the inner source lacks the column, which needs a mapping — story + * 22.2's PD-2 boundary, re-checked at the seam where the mappings are in hand. + * + * 🔴 Two DELIBERATE deviations, stated so a consumer does not inherit them by surprise: + * - rule (1) lets a derived source that projects the name win even when a plain source might + * also own it (`SELECT country FROM bi_events JOIN (SELECT country …) d` resolves `country` + * to `d`, where ANSI would say ambiguous). Recorded by story 22.4's review as an accepted + * SQL-92 deviation: only a derived table's projection is knowable without a mapping, and the + * ON-clause key resolution depends on it. + * - an [[UnnestSource]] takes part in NO un-qualified rule: it never projects, never counts as + * a plain source and never makes the scope opaque, so a bare name beside an UNNEST resolves + * to the plain leg rather than becoming `Ambiguous`. Qualified resolution DOES see it + * (`byName` searches every source). + */ + def resolve(id: Identifier, chain: Seq[Scope]): Resolution = + chain.headOption match { + case None => Unresolved + case Some(innermost) => + (id.tableAlias, id.table) match { + case (Some(q), _) => + innermost.byName(q).map(Resolved(0, _, id.name)).getOrElse(Unresolved) + case (None, Some(k)) => + innermost.byName(k).map(Resolved(0, _, id.name)).getOrElse(Unresolved) + case (None, None) if id.name.contains(".") => + val parts = id.name.split("\\.", 2) + val head = parts(0) + val rest = parts(1) + // `collectFirst`, not `.toStream.headOption`: `Stream` is deprecated on 2.13 and `sql` + // is cross-built. + chain.iterator + .map(sc => sc.byName(head).map(src => Resolved(sc.depth, src, rest))) + .collectFirst { case Some(r) => r } + .getOrElse(Unresolved) + case (None, None) if id.name.nonEmpty && id.name != "*" => + innermost.sources match { + case Seq(lone) => Resolved(0, lone, id.name) + case sources => + val projecting = sources.filter(_.projection.exists(_.contains(id.name))) + val plain = sources.collect { case p: PlainSource => p } + val opaque = sources.exists { + case d: DerivedSource => d.projection.isEmpty + case _ => false + } + (projecting, plain) match { + case (Seq(one), _) => Resolved(0, one, id.name) + case (Seq(), Seq(lone)) if !opaque => Resolved(0, lone, id.name) + case _ => Ambiguous + } + } + case _ => Unresolved // a literal, an ordinal or `*` — names no column + } + } + + /** Every name a statement's FROM makes addressable — the alias-map keys AND values (index names, * aliases, qualified keys) plus the UNNEST aliases. + * + * 🔴 Derived from [[scopeOf]] since story 22.3, and that is a FIX, not a tidy-up. It used to + * read `from.tableAliases` directly, and that `ListMap` is keyed by TABLE, so it has ALREADY + * collapsed two sources sharing an `aliasKey` by the time anyone reads it (story 21.2 AD-6 / + * BIDC-8). MEASURED on `FROM orders o JOIN UNNEST(o.orders) AS i`: `tableAliases` is + * `ListMap(orders -> i)`, so the alias `o` was NOT a correlation name, a body's `o.region` was + * not reported as correlated, the statement did not route, and Elasticsearch read `o.region` as + * an object path — zero rows, HTTP 200. `scopeOf` reads the LOSSLESS `aliasesToTable`, so both + * legs survive. + * + * Widening is safe in BOTH directions: a name gained on the OUTER side makes a reference + * correlated that would otherwise have been silently wrong, and a name gained on the INNER side + * SHADOWS an outer one, which is what SQL specifies. */ - def correlationNames(s: SingleSearch): Set[String] = - (s.from.tableAliases.keys ++ s.from.tableAliases.values ++ s.from.unnestAliases.keys).toSet + def correlationNames(s: SingleSearch): Set[String] = scopeOf(s).names def correlatedReferences(body: DqlStatement, outer: SingleSearch): Seq[Identifier] = correlatedReferences(body, correlationNames(outer)) @@ -118,21 +300,18 @@ object SubqueryScope { } val innerIds = inner.map(_._2).toSet correlatedReferences(d.query, here).filterNot(innerIds.contains).map(d.name -> _) ++ inner - } - } - - /** Story 22.2 — the message a CORRELATED WHERE subquery is refused with. It names the offending - * reference, the outer alias it reads, the story that will execute it, and the two rewrites that - * work today — plus the object-field disambiguation, because a dotted path into an object field - * whose head happens to equal an outer alias lands here too (loud beats zero rows with HTTP - * 200). - */ - def correlatedMessage(id: Identifier, node: Criteria): String = { - val alias = id.name.split("\\.", 2)(0) - s"Correlated subquery: '${id.name}' reads the outer alias '$alias' inside ${node.sql}. " + - "Correlated subqueries require the relational engine (story 22.3, softclient4es-arrow-extensions) " + - "and are not executed yet. Rewrite as a JOIN, make the subquery self-contained, or " + - s"if '$alias' is an object field of the inner table, qualify it with the inner table's alias." + } ++ + // Story 22.3 (A2) — the SECOND half of the walk: a derived table nested inside a WHERE + // SUBQUERY body is LATERAL too, and nothing saw it before. + // + // 🔴 It is not cosmetic and it is not optional. Until story 22.3b the shape was caught by + // `SubqueryCriteria.commonChecks`' correlated arm — as a "Correlated subquery", the wrong + // message — and 22.3b DELETES that arm. Without this line the deletion would turn + // `WHERE c.id IN (SELECT x FROM (SELECT … WHERE o.cid = c.id) d)` from a loud rejection into an + // accepted statement routed to the relational engine, which refuses a body carrying a derived + // table anyway. The walk goes through `whereSubqueries` (story 22.2's NODE-level walker), + // never through a second `Criteria` traversal of its own. + s.whereSubqueries.flatMap(_.inner.toSeq).flatMap(i => lateralOffenders(i, here)) } /** Story 22.2 (PD-2) — the BARE-name half of the correlation rule, decided by the two MAPPINGS at @@ -141,12 +320,44 @@ object SubqueryScope { */ def bareCorrelatedMessage(name: String, innerIndex: String, outerIndex: String): String = s"Correlated subquery: '$name' is not a column of '$innerIndex' but is a column of " + - s"'$outerIndex', so the subquery reads the outer row. Correlated subqueries require the " + - "relational engine (story 22.3, softclient4es-arrow-extensions) and are not executed yet. " + - "Rewrite as a JOIN, or make the subquery self-contained." + s"'$outerIndex', so the subquery reads the outer row. An outer reference must be QUALIFIED " + + "with the outer table's alias — a bare name is read as the subquery's own column, so this " + + "statement cannot route to the relational engine as written. Qualify it, rewrite as a JOIN, " + + "or make the subquery self-contained." + + /** Story 22.3 (AD-3) — a QUALIFIED name whose head names no source in ANY enclosing scope, and + * which the inner index does not map as an object field either. Parse time cannot tell a typo + * from an object path; the seam can, once the mapping is in hand, and the message then names + * every scope it searched so the analyst can see which alias they meant to write. + */ + def unresolvedMessage(id: Identifier, chain: Seq[Scope], context: String): String = { + val head = id.name.split("\\.", 2)(0) + s"'${id.name}' inside $context names no source in scope. " + + // "visible at this level", not "every scope": the seam resolves one level at a time (an inner + // body is executed through its own `resolveWithSchema`), so a three-deep nest reports the two + // scopes THIS check actually searched rather than the whole chain. + "Scopes searched (innermost first): " + + chain + .map(sc => + s"[${sc.depth}] " + sc.sources.map(src => s"${src.alias}=${src.key}").mkString(", ") + ) + .mkString("; ") + + s". If '$head' is an object field of the subquery's own table, qualify it with the subquery's " + + "alias; if it is a column of an enclosing table, qualify it with that table's alias." + } + /** 🔴 The word LATERAL comes FIRST, and that is not a style choice. `GatewayApi.excerpt` caps a + * parse-rejection reason at 200 characters and, above that, keeps the first 120 and the last 77 + * with an ellipsis BETWEEN them. MEASURED at the REPL against real Elasticsearch 8.18 while + * writing this story: the previous wording was 241 characters and the elision landed exactly on + * "this is LATERAL, which is not supported", so the user was told a derived table cannot + * reference an outer alias and NEVER saw the name of the construct or why it is refused. + * + * Generalises to every `validate()` message: the terms a reader must see have to fit in the + * first 120 characters, because the middle is what gets cut. + */ def lateralMessage(id: Identifier, derivedAlias: String): String = - s"A derived table cannot reference an outer alias: '${id.name}' inside derived table " + - s"'$derivedAlias' reads the enclosing FROM (SQL-92 §7.6; this is LATERAL, which is not " + - "supported). Move the condition to the outer WHERE or write it as a correlated WHERE subquery." + s"LATERAL is not supported: a derived table cannot reference an outer alias. '${id.name}' " + + s"inside derived table '$derivedAlias' reads the enclosing FROM (SQL-92 §7.6). Move the " + + "condition to the outer WHERE, or write it as a correlated WHERE subquery." } diff --git a/sql/src/main/scala/app/softnetwork/elastic/sql/query/Where.scala b/sql/src/main/scala/app/softnetwork/elastic/sql/query/Where.scala index 43a7e257..0a365f3d 100644 --- a/sql/src/main/scala/app/softnetwork/elastic/sql/query/Where.scala +++ b/sql/src/main/scala/app/softnetwork/elastic/sql/query/Where.scala @@ -1333,9 +1333,16 @@ sealed trait SubqueryCriteria extends Criteria with ElasticFilter { protected def notAsString: String = maybeNot.map(_ => "NOT ").getOrElse("") - /** Shared validation, in order: a body kind this story executes; the body's OWN rules - * (`Parser.apply` validates the TOP level only — story 22.1's trap, one clause over); NOT - * correlated (PD-2). + /** Shared validation, in order: a body kind this story executes, then the body's OWN rules + * (`Parser.apply` validates the TOP level only — story 22.1's trap, one clause over). + * + * 🔴 Story 22.3b DELETED the third arm — the `correlatedRefs.headOption => Left` rejection. That + * single deletion, plus ONE widened disjunct in `SingleSearch.relationalClosureRequired`, IS the + * rejection-to-routing flip: a correlated subquery now PARSES and routes to the relational + * engine, and every venue WITHOUT the engine refuses it loudly at `SearchApi.resolveWithSchema` + * / `CoreDqlExtension` through `RelationalClosureGuard`. Nothing else in `sql` moved. + * `correlatedRefs` stays RECORDED on the node — it is what the planner reads to build the + * correlated leg. */ protected def commonChecks: Either[String, Unit] = for { @@ -1354,10 +1361,6 @@ sealed trait SubqueryCriteria extends Criteria with ElasticFilter { case other => Left(s"A WHERE subquery body must be a SELECT, got ${other.getClass.getSimpleName}") } - _ <- correlatedRefs.headOption match { - case Some(id) => Left(SubqueryScope.correlatedMessage(id, this)) - case None => Right(()) - } } yield () /** The projected column of the inner statement — exactly ONE, never `*` — for IN / quantified / diff --git a/sql/src/main/scala/app/softnetwork/elastic/sql/query/package.scala b/sql/src/main/scala/app/softnetwork/elastic/sql/query/package.scala index 9ab48d37..1a053cc4 100644 --- a/sql/src/main/scala/app/softnetwork/elastic/sql/query/package.scala +++ b/sql/src/main/scala/app/softnetwork/elastic/sql/query/package.scala @@ -167,11 +167,17 @@ package object query { */ lazy val hasDerivedTables: Boolean = derivedTables.nonEmpty - /** Epic 22 AD-4 — THE predicate every venue routes on. Read THIS, never `from.` directly: later - * stories widen it with constructs a `From` cannot see (22.3's correlated subqueries, 22.5's - * CTEs) and a consumer reading the FROM member would silently miss them. + /** Epic 22 AD-4 — THE predicate every venue routes on. Read THIS, never `from.` directly: it + * carries constructs a `From` cannot see (story 22.3's correlated subqueries; story 22.5's + * CTEs next) and a consumer reading the FROM member would silently miss them. + * + * 🔴 The predicate family, stated ONCE for whoever lands last of 22.3 / 22.5 / 22.6: each + * story adds exactly ONE disjunct HERE, the package function `relationalClosureRequired` reads + * THIS member, and `MultiSearch` folds its branches through it. Any spec that re-derives the + * disjunction somewhere else is the story-21.3 desync class. */ - lazy val relationalClosureRequired: Boolean = from.relationalClosureRequired + lazy val relationalClosureRequired: Boolean = + from.relationalClosureRequired || hasCorrelatedSubqueries /** Every WHERE-subquery node this statement carries, in statement order (story 22.2). * @@ -189,6 +195,31 @@ package object query { */ lazy val hasWhereSubqueries: Boolean = whereSubqueries.nonEmpty + /** The TOP-LEVEL WHERE-subquery nodes whose subtree reads an enclosing scope (story 22.3). + * + * 🔴 Read what `correlatedRefs` MEANS, not its name (AD-2): story 22.2's walk recurses into a + * body's own subqueries with the scopes ACCUMULATED, so a reference from the innermost body to + * the MIDDLE body's alias — not correlated to THIS statement at all — is still reported on the + * top-level node. `correlatedRefs.nonEmpty` therefore means "this node's SUBTREE reads outside + * its own body", which is exactly the routing question: a middle body whose inner is + * correlated to it cannot run ES-natively either. + */ + lazy val correlatedSubqueries: Seq[SubqueryCriteria] = + whereSubqueries.filter(_.correlatedRefs.nonEmpty) + + /** DEEP (story 22.3 AC 5): any WHERE-subquery node at any nesting depth whose subtree reads an + * enclosing scope. A correlated body two levels down makes the MIDDLE body un-executable + * ES-natively, so the WHOLE statement routes to the relational engine. + * + * The `s.inner.exists(...)` half is redundant with the accumulated walk described above and is + * kept deliberately: it costs one already-computed boolean and it does not depend on the + * walk's reach staying what it is today. + */ + lazy val hasCorrelatedSubqueries: Boolean = + whereSubqueries.exists(s => + s.correlatedRefs.nonEmpty || s.inner.exists(_.hasCorrelatedSubqueries) + ) + /** Every identifier this statement NAMES, across every clause that can carry one — the SELECT * list (through each item's function chain), WHERE, HAVING, GROUP BY, ORDER BY and each * standard JOIN's ON. @@ -625,32 +656,29 @@ package object query { val scopes = from.derivedTables if (scopes.isEmpty) Right(()) else { - val sole: Option[DerivedTable] = from.tables match { - case Seq(t) if t.joins.isEmpty => t.derived - case _ => None - } + // Story 22.3 (AD-1) — resolution goes through the ONE resolver, so this check and the + // planner's cannot drift. It also retires the stale-`Identifier.table` fallback story 22.5 + // would otherwise have had to add: `SubqueryScope.resolve` reads `tableAlias` FIRST and + // falls back to `table`, absorbing the re-`update()` staleness where every other consumer + // already keys on the alias. + val here = Seq(SubqueryScope.scopeOf(this)) // `fieldAliases` is built over `fieldsWithComputedAliases`, so it also holds the synthetic // `__cN` names — harmless here, since no real column is spelled that way. val outerAliases: Set[String] = select.fieldAliases.values.toSet referencedIdentifiers.iterator .filter(id => id.name.nonEmpty && id.name != "*") + // An OUTER SELECT alias (`SELECT COL AS c … ORDER BY c`) is not a derived-table + // reference; it names a projection of THIS statement. + .filterNot(id => + id.tableAlias.isEmpty && id.table.isEmpty && !id.name.contains(".") && + outerAliases.contains(id.name) + ) .flatMap { id => - val scope: Option[DerivedTable] = - id.table - .flatMap(scopes.get) - // A WIDENING over the specced `id.table` alone: `Identifier.update` derives `table` - // only when `parts.size > 1`, so a node re-`update()`-d after its name was already - // normalised (which `SearchApi.resolveWithSchema` does to EVERY executed statement) - // keeps `tableAlias` right while `table` may be stale or absent. Checking both is - // what keeps the scope rule stable across the second pass. - .orElse(id.tableAlias.flatMap(scopes.get)) - .orElse { - if ( - id.tableAlias.isEmpty && id.table.isEmpty && !id.name.contains(".") && - !outerAliases.contains(id.name) - ) sole - else None - } + val scope: Option[DerivedTable] = SubqueryScope.resolve(id, here) match { + case SubqueryScope.Resolved(0, src: SubqueryScope.DerivedSource, _) => + scopes.get(src.alias) + case _ => None // Ambiguous / Unresolved / an enclosing scope: never guessed at + } val head = id.name.split("\\.", 2)(0) scope.flatMap(d => d.outputNames.filterNot(_.contains(head)).map(names => (id, d, names)) @@ -675,7 +703,10 @@ package object query { * HTTP 200. The detector is `SubqueryScope`, shared with stories 22.2 / 22.3. */ private lazy val lateralCheck: Either[String, Unit] = - if (!from.hasDerivedTables) Right(()) + // Story 22.3 — `hasWhereSubqueries` joins the guard because the walk now descends into + // WHERE-subquery bodies: a statement whose OWN FROM carries no derived table can still hold + // one inside a subquery body, and that shape is LATERAL just the same. + if (!from.hasDerivedTables && !hasWhereSubqueries) Right(()) else SubqueryScope.lateralOffenders(this).headOption match { // The offender is carried WITH the derived table whose body names it, so the message can diff --git a/sql/src/test/scala/app/softnetwork/elastic/sql/parser/DerivedTableSpec.scala b/sql/src/test/scala/app/softnetwork/elastic/sql/parser/DerivedTableSpec.scala index a8775e96..89c24be0 100644 --- a/sql/src/test/scala/app/softnetwork/elastic/sql/parser/DerivedTableSpec.scala +++ b/sql/src/test/scala/app/softnetwork/elastic/sql/parser/DerivedTableSpec.scala @@ -223,18 +223,24 @@ class DerivedTableSpec extends AnyFlatSpec with Matchers { rejects( "SELECT c.id FROM customers c JOIN (SELECT o.cid FROM orders o WHERE o.cid = c.id) d " + "ON d.cid = c.id", - "A derived table cannot reference an outer alias", + // 🔴 story 22.3 moved "LATERAL" to the FRONT of this message: `GatewayApi.excerpt` caps a + // rejection reason at 200 characters and elides the MIDDLE, so the old wording reached the + // REPL user without the word that names the construct. Contract unchanged, order fixed. + "LATERAL is not supported", + "derived table cannot reference an outer alias", "'c.id'", - "derived table 'd'", - "LATERAL" + "derived table 'd'" ) } it should "be rejected in FROM position, against a later comma-list table" in { rejects( "SELECT d.x FROM (SELECT o.cid AS x FROM orders o WHERE o.cid = customers.id) d, customers", - "A derived table cannot reference an outer alias", - "LATERAL" + // 🔴 story 22.3 moved "LATERAL" to the FRONT of this message: `GatewayApi.excerpt` caps a + // rejection reason at 200 characters and elides the MIDDLE, so the old wording reached the + // REPL user without the word that names the construct. Contract unchanged, order fixed. + "LATERAL is not supported", + "derived table cannot reference an outer alias" ) } diff --git a/sql/src/test/scala/app/softnetwork/elastic/sql/parser/WhereSubquerySpec.scala b/sql/src/test/scala/app/softnetwork/elastic/sql/parser/WhereSubquerySpec.scala index f2060374..c1804030 100644 --- a/sql/src/test/scala/app/softnetwork/elastic/sql/parser/WhereSubquerySpec.scala +++ b/sql/src/test/scala/app/softnetwork/elastic/sql/parser/WhereSubquerySpec.scala @@ -257,40 +257,116 @@ class WhereSubquerySpec extends AnyFlatSpec with Matchers { // ── correlation (AC 4) ─────────────────────────────────────────────────────────────────────── - "A correlated subquery" should "be rejected with the 22.3 message when it names an outer alias" in { - rejects( + /** 🔴 Story 22.3b — THE FLIP. These three statements were `Left`s carrying "Correlated subquery … + * story 22.3" until this story deleted `SubqueryCriteria.commonChecks`' correlated arm. They now + * PARSE, and `relationalClosureRequired` routes them to the relational engine; every venue + * WITHOUT the engine refuses them at `SearchApi.resolveWithSchema` / `CoreDqlExtension` + * (`RelationalClosureGuardSpec`, `CoreDqlExtensionSpec`, `ReplGatewayIntegrationSpec` 6d). + * + * The contract that did NOT change is the one that matters: a correlated subquery is still never + * EXECUTED as if it were self-contained. + */ + "A correlated subquery" should "parse and route to the relational engine (story 22.3b)" in { + val routed = Seq( "SELECT o.id FROM orders o WHERE o.cid IN " + "(SELECT c.id FROM customers c WHERE c.region = o.region)", - "Correlated subquery", - "o.region", - "outer alias 'o'", - "story 22.3" - ) - rejects( "SELECT o.id FROM orders o WHERE EXISTS (SELECT 1 FROM customers c WHERE c.id = o.cid)", - "Correlated subquery", - "o.cid" - ) - rejects( // the outer INDEX name is a correlation name too + // the outer INDEX name is a correlation name too "SELECT id FROM orders WHERE cid IN (SELECT id FROM customers WHERE region = orders.region)", - "Correlated subquery", - "orders.region" + // a nested body's reference to the OUTERMOST alias — DEEP + "SELECT o.id FROM orders o WHERE o.cid IN (SELECT c.id FROM customers c WHERE c.k IN " + + "(SELECT k FROM z WHERE z.r = o.region))" ) + routed.foreach { sql => + withClue(s"[$sql] ") { + val s = parse(sql) + s.hasCorrelatedSubqueries shouldBe true + s.relationalClosureRequired shouldBe true + s.from.relationalClosureRequired shouldBe false // the FROM half is untouched + relationalClosureRequired(s) shouldBe true // the package fn reads the SingleSearch val + } + } } it should "honour shadowing: an alias the inner statement declares is the INNER one" in { - parse( + val s = parse( "SELECT o.id FROM orders o WHERE o.cid IN (SELECT o.id FROM customers o WHERE o.r = 'EU')" ) + // the control that keeps the flip above non-vacuous: shadowing is NOT correlation, so this + // statement must stay ES-native. + s.hasCorrelatedSubqueries shouldBe false + s.relationalClosureRequired shouldBe false } - it should "see a nested body's reference to the OUTERMOST alias" in { + it should "keep an UNcorrelated WHERE subquery off the engine (story 22.2's PASSTHROUGH)" in { + val s = parse( + "SELECT id FROM orders WHERE customer_id IN (SELECT id FROM customers WHERE region = 'EU')" + ) + s.hasWhereSubqueries shouldBe true + s.hasCorrelatedSubqueries shouldBe false + s.relationalClosureRequired shouldBe false + } + + /** 🔴 Story 22.3 (PD-2) — a derived body reading an ENCLOSING correlation name is SQL:1999 + * `LATERAL`, refused at every venue and on every Elasticsearch major because the refusal lives + * in the `sql` module. + * + * The FROM/JOIN-position arm shipped with story 22.1; the second row is story 22.3's OWN + * extension of the walk to a derived table nested inside a WHERE-subquery BODY. Before it, that + * row was caught by the correlated arm this story DELETED — so without the extension the + * deletion would have turned a loud rejection into an accepted statement. + */ + "A LATERAL-shaped derived table" should "be rejected by validate() with the ANSI message" in { rejects( - "SELECT o.id FROM orders o WHERE o.cid IN (SELECT c.id FROM customers c WHERE c.k IN " + - "(SELECT k FROM z WHERE z.r = o.region))", - "Correlated subquery", - "o.region" + "SELECT c.id FROM customers c JOIN (SELECT o.cid FROM orders o WHERE o.cid = c.id) d ON d.cid = c.id", + "LATERAL is not supported", + "derived table cannot reference an outer alias", + "'c.id'", + "derived table 'd'" ) + rejects( + "SELECT c.id FROM customers c WHERE c.id IN " + + "(SELECT x FROM (SELECT o.cid AS x FROM orders o WHERE o.cid = c.id) d)", + "LATERAL is not supported", + "derived table cannot reference an outer alias", + "derived table 'd'" + ) + } + + it should "leave a derived body that names ONLY its own sources alone" in { + parse("SELECT d.total FROM (SELECT amount AS total FROM t) d WHERE d.total > 1") + () + } + + /** The canonical render of every correlated form, MEASURED (story 22.3 Task 0 row 6) — the text + * and the fixed point, so a render that could not be re-read fails here rather than at a + * customer's cluster. `= ANY` canonicalises to `IN`, as story 22.2 already pinned. + */ + private val correlatedRenders = Seq( + "SELECT c.id FROM customers c WHERE EXISTS (SELECT 1 FROM orders o WHERE o.customer_id = c.id)" -> + "SELECT c.id FROM customers AS c WHERE EXISTS (SELECT 1 FROM orders AS o WHERE o.customer_id = c.id)", + "SELECT c.id FROM customers c WHERE NOT EXISTS (SELECT 1 FROM orders o WHERE o.customer_id = c.id)" -> + "SELECT c.id FROM customers AS c WHERE NOT EXISTS (SELECT 1 FROM orders AS o WHERE o.customer_id = c.id)", + "SELECT c.id FROM customers c WHERE c.id IN (SELECT o.customer_id FROM orders o WHERE o.amount > c.credit_limit)" -> + "SELECT c.id FROM customers AS c WHERE c.id IN (SELECT o.customer_id FROM orders AS o WHERE o.amount > c.credit_limit)", + "SELECT c.id FROM customers c WHERE c.id NOT IN (SELECT o.customer_id FROM orders o WHERE o.amount > c.credit_limit)" -> + "SELECT c.id FROM customers AS c WHERE c.id NOT IN (SELECT o.customer_id FROM orders AS o WHERE o.amount > c.credit_limit)", + "SELECT c.id FROM customers c WHERE c.credit_limit > (SELECT AVG(o.amount) FROM orders o WHERE o.customer_id = c.id)" -> + "SELECT c.id FROM customers AS c WHERE c.credit_limit > (SELECT AVG(o.amount) FROM orders AS o WHERE o.customer_id = c.id)", + "SELECT c.id FROM customers c WHERE c.id = ANY (SELECT o.customer_id FROM orders o WHERE o.amount > c.credit_limit)" -> + "SELECT c.id FROM customers AS c WHERE c.id IN (SELECT o.customer_id FROM orders AS o WHERE o.amount > c.credit_limit)", + "SELECT c.id FROM customers c WHERE c.id IN (SELECT o.cid FROM orders o WHERE o.amount > (SELECT AVG(r.amount) FROM refunds r WHERE r.cid = c.id))" -> + "SELECT c.id FROM customers AS c WHERE c.id IN (SELECT o.cid FROM orders AS o WHERE o.amount > (SELECT AVG(r.amount) FROM refunds AS r WHERE r.cid = c.id))" + ) + + correlatedRenders.foreach { case (in, text) => + it should s"[22.3b] parse, route and round-trip [$in]" in { + val stmt = parse(in) + stmt.hasCorrelatedSubqueries shouldBe true + stmt.relationalClosureRequired shouldBe true + stmt.sql shouldBe text + Parser(stmt.sql) shouldBe Right(stmt) + } } it should "assume a BARE name is the inner column at parse time (PD-2)" in { diff --git a/sql/src/test/scala/app/softnetwork/elastic/sql/query/SubqueryScopeSpec.scala b/sql/src/test/scala/app/softnetwork/elastic/sql/query/SubqueryScopeSpec.scala index 93f5fad1..42e1754a 100644 --- a/sql/src/test/scala/app/softnetwork/elastic/sql/query/SubqueryScopeSpec.scala +++ b/sql/src/test/scala/app/softnetwork/elastic/sql/query/SubqueryScopeSpec.scala @@ -16,6 +16,7 @@ package app.softnetwork.elastic.sql.query +import app.softnetwork.elastic.sql.{GenericIdentifier, Identifier} import app.softnetwork.elastic.sql.parser.Parser import org.scalatest.flatspec.AnyFlatSpec import org.scalatest.matchers.should.Matchers @@ -46,6 +47,29 @@ class SubqueryScopeSpec extends AnyFlatSpec with Matchers { SubqueryScope.correlationNames(s) should contain allOf ("orders", "o") } + /** 🔴 REGRESSION PIN (story 22.3, found by the independent review). `correlationNames` used to + * read `from.tableAliases` directly, and that `ListMap` is keyed by TABLE, so it has ALREADY + * collapsed two sources sharing an `aliasKey`. MEASURED on `FROM orders o JOIN UNNEST(o.orders) + * AS i`: `tableAliases` is `ListMap(orders -> i)`, alias `o` was GONE, and a subquery body's + * `o.region` was therefore NOT reported as correlated — the statement did not route, and + * Elasticsearch read `o.region` as an object path: zero rows, HTTP 200. + * + * The fix is that `correlationNames` now derives from `scopeOf`, which reads the LOSSLESS + * `aliasesToTable`. Same class as story BIDC-8 / softclient4es-arrow#144. + */ + it should "keep BOTH aliases when two sources share an alias-map key (the collapse trap)" in { + val unnest = single("SELECT o.id FROM orders o JOIN UNNEST(o.orders) AS i") + unnest.from.tableAliases.keySet should have size 1 // the collapse, still there by design + SubqueryScope.correlationNames(unnest) should contain allOf ("o", "i", "orders") + val selfJoin = single("SELECT a.id FROM orders a JOIN orders b ON a.id = b.id") + SubqueryScope.correlationNames(selfJoin) should contain allOf ("a", "b", "orders") + // and the consequence that matters: the reference IS reported as correlated + refs( + "SELECT id FROM customers WHERE region = o.region", + "SELECT o.id FROM orders o JOIN UNNEST(o.orders) AS i" + ) shouldBe Seq("o.region") + } + "A qualified reference to an outer name" should "be reported" in { refs( "SELECT c.id FROM customers c WHERE c.region = o.region", @@ -80,18 +104,292 @@ class SubqueryScopeSpec extends AnyFlatSpec with Matchers { ) shouldBe Seq("o.region") } - "The messages" should "name the offender, the scope and the story that will execute it" in { - val node = single("SELECT id FROM t WHERE a IN (SELECT a FROM u)").whereSubqueries.head - val id = single("SELECT id FROM customers WHERE region = orders.region").referencedIdentifiers - .find(_.name.contains(".")) - .getOrElse(fail("no qualified identifier")) - val qualified = SubqueryScope.correlatedMessage(id, node) - qualified should include("Correlated subquery") - qualified should include("orders.region") - qualified should include("story 22.3") + "The bare-name message" should "name the two indices and the remedy that makes it EXECUTABLE" in { + // Story 22.3b: a QUALIFIED outer reference now routes to the relational engine, so the ONE + // correlation shape still refused in core is the BARE one — and the remedy is to qualify it. val bare = SubqueryScope.bareCorrelatedMessage("vip", "orders", "customers") bare should include("'vip' is not a column of 'orders'") bare should include("is a column of 'customers'") - bare should include("story 22.3") + bare should include("must be QUALIFIED") + } + + // ══ Story 22.3 (AD-1) — the scope model ══════════════════════════════════════════════════════ + + import SubqueryScope._ + + /** 🔴 An UNVALIDATED parse (lead ruling OQ-8). Several rows below are statements `validate()` + * REFUSES — a LATERAL-shaped derived body — so `Parser.apply` cannot feed them, and a test-side + * re-implementation of the reader would drift from `apply`'s. `Parser.single`'s action still + * runs `.update()`, so `correlatedRefs` and every resolved qualifier are populated. + */ + private def outerOf(sql: String): SingleSearch = Parser.parseUnvalidated(sql) match { + case Right(s: SingleSearch) => s + case other => fail(s"[$sql] $other") + } + + private def firstBody(s: SingleSearch): SingleSearch = + s.whereSubqueries.headOption + .flatMap(_.inner) + .getOrElse(fail(s"[${s.sql}] no WHERE-subquery body")) + + /** The RAW correlated shape — `tableAlias` and `table` both empty, the qualifier still in the + * name — which is exactly what `GenericIdentifier.update` leaves behind for an unresolved + * qualifier (story 22.2 AD-3). + */ + private def ref(name: String): Identifier = GenericIdentifier(name) + + behavior of "SubqueryScope.scopeOf / chain" + + it should "speak the alias-map KEY language for every source" in { + val s = outerOf( + "SELECT o.id FROM orders o JOIN \"prod_eu\".orders p ON o.cid = p.id " + + "JOIN UNNEST(o.items) AS i WHERE o.x IN (SELECT y FROM z)" + ) + val sc = scopeOf(s) + sc.sources.map(_.alias) shouldBe Seq("o", "p", "i") + // story 21.2 AD-6: an AMBIGUOUS bare name keys by the QUALIFIED reference + sc.sources.collect { case ps: PlainSource => ps.key } should contain("prod_eu.orders") + sc.sources.collect { case ps: PlainSource => ps.key }.foreach { k => + withClue(s"key [$k] must be an alias-map key ") { + s.from.tableAliases.keySet should contain(k) + } + } + sc.names should contain allOf ("o", "orders", "p", "prod_eu.orders", "i") + } + + it should "build the chain innermost first" in { + val outer = + outerOf( + "SELECT c.id FROM customers c WHERE EXISTS (SELECT 1 FROM orders o WHERE o.cid = c.id)" + ) + val ch = chain(firstBody(outer), Seq(outer)) + ch.map(_.depth) shouldBe Seq(0, 1) + ch.head.sources.map(_.alias) shouldBe Seq("o") + ch(1).sources.map(_.alias) shouldBe Seq("c") + } + + behavior of "SubqueryScope.resolve - qualified names, innermost first" + + it should "resolve an inner-declared alias at depth 0 and an outer one at depth 1 (correlated)" in { + val outer = outerOf( + "SELECT c.id FROM customers c WHERE EXISTS (SELECT 1 FROM orders o WHERE o.customer_id = c.id)" + ) + val body = firstBody(outer) + val ch = chain(body, Seq(outer)) + resolve(ref("c.id"), ch) shouldBe Resolved(1, PlainSource("customers", "c"), "id") + // the inner operand: `Identifier.update` already set tableAlias = Some("o") + val inner = + body.referencedIdentifiers.find(_.tableAlias.contains("o")).getOrElse(fail("no o.*")) + resolve(inner, ch) shouldBe Resolved(0, PlainSource("orders", "o"), "customer_id") + } + + it should "let an inner alias SHADOW the outer (innermost wins) - and agree with the detector" in { + val outer = outerOf( + "SELECT o.id FROM orders o WHERE o.cid IN (SELECT o.id FROM customers o WHERE o.region = 'EU')" + ) + outer.whereSubqueries.head.correlatedRefs shouldBe Nil // the story-22.2 row + val body = firstBody(outer) + val region = body.referencedIdentifiers.find(_.name == "region").getOrElse(fail("no region")) + resolve(region, chain(body, Seq(outer))) should matchPattern { + case Resolved(0, PlainSource("customers", "o"), "region") => + } + } + + it should "resolve two levels out (depth 2) and agree with the detector at the outermost update" in { + val outer = outerOf( + "SELECT c.id FROM customers c WHERE c.id IN (SELECT o.cid FROM orders o WHERE o.amount > " + + "(SELECT AVG(r.amount) FROM refunds r WHERE r.cid = c.id))" + ) + val middle = firstBody(outer) + val innermost = firstBody(middle) + resolve(ref("c.id"), chain(innermost, Seq(middle, outer))) shouldBe + Resolved(2, PlainSource("customers", "c"), "id") + // AD-2's agreement property: every reference the DETECTOR calls correlated resolves OUTWARD. + // The two cannot drift without reddening this. + correlatedReferences(middle, outer).foreach { id => + withClue(s"[${id.name}] ") { + resolve(id, chain(middle, Seq(outer))) should matchPattern { + case Resolved(d, _, _) if d >= 1 => + } + } + } + outer.hasCorrelatedSubqueries shouldBe true // DEEP + // 🔴 MEASURED, and it corrects the spec: correlation is RELATIVE TO A SCOPE CHAIN. Seen on its + // own, the middle statement declares `o`/`orders` and its innermost body reads `c.id` — a name + // the middle does not declare EITHER, so the middle's own node records nothing and + // `middle.hasCorrelatedSubqueries` is FALSE. Nothing is lost: story 22.2's walk accumulates + // scopes downward, so the reference IS reported on the OUTERMOST node, which is where routing + // is decided. ⚠️ A consumer (the story-22.3b planner) that asks an ISOLATED body this question + // gets the wrong answer — it must read the top-level node's `correlatedRefs`, or walk with the + // scopes accumulated, exactly as `correlatedReferences` does. + middle.hasCorrelatedSubqueries shouldBe false + // 🔴 story 22.2's walk accumulates scopes, so the INNERMOST body's `c.id` is reported on the + // TOP-LEVEL node: `correlatedRefs` means "this subtree reads outside its own body" (AD-2). + outer.correlatedSubqueries.map(_.correlatedRefs.map(_.name)) shouldBe Seq(Seq("c.id")) + // and a MIDDLE-only correlation is reported outward too — the middle body cannot run + // ES-natively either, which is exactly why the whole statement must route. + val midOnly = outerOf( + "SELECT c.id FROM customers c WHERE c.id IN (SELECT o.cid FROM orders o WHERE o.amount > " + + "(SELECT AVG(r.amount) FROM refunds r WHERE r.cid = o.cid))" + ) + midOnly.hasCorrelatedSubqueries shouldBe true + midOnly.whereSubqueries.head.correlatedRefs.map(_.name) shouldBe Seq("o.cid") + } + + /** The CONVERSE of the agreement property above, and the one that can actually FAIL: 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. + */ + it should "report EVERY reference the resolver resolves outward (detector completeness)" in { + val shapes = Seq( + "SELECT c.id FROM customers c WHERE EXISTS (SELECT 1 FROM orders o WHERE o.cid = c.id)", + "SELECT o.id FROM orders o JOIN UNNEST(o.orders) AS i WHERE o.id IN " + + "(SELECT cid FROM customers WHERE name = o.region)", + "SELECT a.id FROM orders a JOIN orders b ON a.id = b.id WHERE a.id IN " + + "(SELECT cid FROM customers WHERE r = b.region)", + "SELECT id FROM \"prod_eu\".orders p WHERE id IN (SELECT cid FROM customers WHERE r = p.region)" + ) + shapes.foreach { sql => + val outer = outerOf(sql) + val body = firstBody(outer) + val ch = chain(body, Seq(outer)) + val reported = correlatedReferences(body, outer).map(_.name).toSet + body.referencedIdentifiers.foreach { id => + resolve(id, ch) match { + case Resolved(d, _, _) if d >= 1 => + withClue( + s"[$sql] '${id.name}' resolves at depth $d but the detector did not report it " + ) { + reported should contain(id.name) + } + case _ => () + } + } + } + } + + it should "answer Unresolved for a head in no scope (an object path at parse time)" in { + val outer = + outerOf("SELECT id FROM orders address WHERE id IN (SELECT id FROM t WHERE zip.code = 'X')") + resolve(ref("zip.code"), chain(firstBody(outer), Seq(outer))) shouldBe Unresolved + } + + behavior of "SubqueryScope.resolve - un-qualified names (story 22.4 AD-4, subsumed)" + + private val w5005 = outerOf( + "SELECT country AS country, sum(amount) AS \"SUM(amount)\" FROM bi_events " + + "JOIN (SELECT country AS country__, sum(amount) AS mme_inner__ FROM bi_events GROUP BY country " + + "ORDER BY sum(amount) DESC LIMIT 10) AS series_limit ON country = country__ " + + "GROUP BY country ORDER BY \"SUM(amount)\" DESC LIMIT 10000" + ) + + it should "row 1: send a name exactly one derived table projects to that derived table" in { + resolve(ref("country__"), Seq(scopeOf(w5005))) shouldBe + Resolved(0, DerivedSource("series_limit", Some(Seq("country__", "mme_inner__"))), "country__") + } + + it should "row 2: send a name no derived table projects to the SOLE plain index" in { + resolve(ref("country"), Seq(scopeOf(w5005))) shouldBe + Resolved(0, PlainSource("bi_events", "bi_events"), "country") + resolve(ref("amount"), Seq(scopeOf(w5005))) should matchPattern { + case Resolved(0, PlainSource("bi_events", _), "amount") => + } + } + + it should "row 3: stay Ambiguous between two PLAIN legs, and beside an OPAQUE derived leg" in { + resolve( + ref("total"), + Seq(scopeOf(outerOf("SELECT total FROM orders o JOIN customers c ON o.cid = c.id"))) + ) shouldBe Ambiguous + resolve( + ref("total"), + Seq(scopeOf(outerOf("SELECT total FROM orders o JOIN (SELECT * FROM x) AS d ON o.id = d.id"))) + ) shouldBe Ambiguous + // the control that makes the `opaque` clause non-vacuous: once the derived leg's projection IS + // known, `total` cannot be its column, so the sole plain leg owns it. + resolve( + ref("total"), + Seq(scopeOf(outerOf("SELECT total FROM orders o JOIN (SELECT a FROM x) AS d ON o.id = d.a"))) + ) shouldBe Resolved(0, PlainSource("orders", "o"), "total") + } + + it should "row 4: resolve BOTH operands of ON country = country to the SAME source" in { + // the planner's same-alias guard case: a self-comparison must not be read as a join key, and + // it is the RESOLVER's job to make both operands land on one source so the guard can see it. + val s = outerOf( + "SELECT country FROM bi_events JOIN (SELECT country, SUM(amount) AS s FROM bi_events " + + "GROUP BY country) AS d ON country = country" + ) + resolve(ref("country"), Seq(scopeOf(s))) should matchPattern { + case Resolved(0, DerivedSource("d", _), "country") => + } + } + + it should "let a LONE source own every bare name, opaque or not (story 22.4's fourth rule)" in { + resolve(ref("zzz"), Seq(scopeOf(outerOf("SELECT a FROM (SELECT * FROM x) AS d")))) should + matchPattern { case Resolved(0, DerivedSource("d", None), "zzz") => } + } + + it should "never resolve a bare name OUTWARD (PD-2: assumed inner; the seam re-checks)" in { + val outer = + outerOf( + "SELECT id FROM customers WHERE id IN (SELECT customer_id FROM orders WHERE vip = true)" + ) + resolve(ref("vip"), chain(firstBody(outer), Seq(outer))) should matchPattern { + case Resolved(0, PlainSource("orders", "orders"), "vip") => + } + } + + behavior of "SubqueryScope.lateralReferences" + + it should "report a FROM-derived, a JOIN-derived and a WHERE-nested derived body reading an outer alias" in { + lateralReferences( + outerOf( + "SELECT c.id FROM customers c JOIN (SELECT o.cid FROM orders o WHERE o.cid = c.id) d ON d.cid = c.id" + ) + ).map(_.name) shouldBe Seq("c.id") + lateralReferences( + outerOf( + "SELECT d.x FROM (SELECT o.cid AS x FROM orders o WHERE o.cid = customers.id) d, customers" + ) + ).map(_.name) shouldBe Seq("customers.id") + // 🔴 story 22.3's OWN extension of the walk. Before it, this shape was caught by the CORRELATED + // arm of `SubqueryCriteria.commonChecks` — which 22.3b deletes — so without the extension the + // deletion would have turned a loud rejection into an accepted statement. + lateralReferences( + outerOf( + "SELECT c.id FROM customers c WHERE c.id IN (SELECT x FROM (SELECT o.cid AS x FROM orders o WHERE o.cid = c.id) d)" + ) + ).map(_.name) shouldBe Seq("c.id") + lateralReferences( + outerOf("SELECT d.total FROM (SELECT amount AS total FROM t) d WHERE d.total > 1") + ) shouldBe Nil + } + + it should "name the derived table whose body reads the outer alias" in { + val msg = SubqueryScope.lateralMessage(ref("c.id"), "d") + msg should include("derived table cannot reference an outer alias") + msg should include("'c.id'") + msg should include("derived table 'd'") + // 🔴 LATERAL must be in the first 120 characters: `GatewayApi.excerpt` caps a rejection reason + // at 200 and keeps head(120) + "..." + tail(77), so anything in the middle never reaches the + // user. MEASURED against real ES 8.18 — the previous wording lost exactly this word. + msg should startWith("LATERAL is not supported") + msg.indexOf("derived table cannot reference an outer alias") should be < 120 + } + + behavior of "SubqueryScope.unresolvedMessage" + + it should "name every scope it searched, innermost first" in { + val outer = + outerOf( + "SELECT id FROM customers c WHERE id IN (SELECT cid FROM orders o WHERE zip.code = 'X')" + ) + val body = firstBody(outer) + val msg = SubqueryScope.unresolvedMessage(ref("zip.code"), chain(body, Seq(outer)), body.sql) + msg should include("names no source in scope") + msg should include("Scopes searched (innermost first)") + msg should include("[0] o=orders") + msg should include("[1] c=customers") } } diff --git a/testkit/src/main/scala/app/softnetwork/elastic/client/WhereSubqueryCompletenessSpec.scala b/testkit/src/main/scala/app/softnetwork/elastic/client/WhereSubqueryCompletenessSpec.scala index 0b918c68..c66a022e 100644 --- a/testkit/src/main/scala/app/softnetwork/elastic/client/WhereSubqueryCompletenessSpec.scala +++ b/testkit/src/main/scala/app/softnetwork/elastic/client/WhereSubqueryCompletenessSpec.scala @@ -170,8 +170,10 @@ trait WhereSubqueryCompletenessSpec /** 🔴 `searchAs` is a MACRO that validates the statement at COMPILE time, so every query below is * an inline string LITERAL — a `val` holding the same text is rejected by the macro. That is - * also why the correlated-subquery rejection (a parse-time `Left`) lives in the REPL integration - * spec, which dispatches at run time, and not here. + * also why the correlated-subquery rejection lives in the REPL integration spec, which + * dispatches at run time, and not here. (Since story 22.3b that rejection is no longer a + * parse-time `Left`: a correlated subquery PARSES and routes to the relational engine, and a + * venue without the engine refuses it through `RelationalClosureGuard`.) */ private def idsOf(result: ElasticResult[Seq[WsqId]]): Seq[String] = result match { case ElasticSuccess(rows) => rows.map(_.id) diff --git a/testkit/src/main/scala/app/softnetwork/elastic/client/repl/ReplGatewayIntegrationSpec.scala b/testkit/src/main/scala/app/softnetwork/elastic/client/repl/ReplGatewayIntegrationSpec.scala index 7c505704..66152b5f 100644 --- a/testkit/src/main/scala/app/softnetwork/elastic/client/repl/ReplGatewayIntegrationSpec.scala +++ b/testkit/src/main/scala/app/softnetwork/elastic/client/repl/ReplGatewayIntegrationSpec.scala @@ -1409,7 +1409,13 @@ trait ReplGatewayIntegrationSpec extends ReplIntegrationTestKit { () } - it should "refuse a CORRELATED subquery with the story-22.3 message, not the JOIN message" in { + /** Story 22.3b RETARGETED this row. The CONTRACT it pins is unchanged — a correlated subquery is + * never executed as if it were self-contained at a venue with no relational engine — but the + * rejection moved from the PARSER to `RelationalClosureGuard`, so the message now names the + * shape and the jar that runs it. Story 22.2 asserted `not include "… jar"`; that is now the + * opposite, deliberately, and it is the pin that proves the flip reached a real cluster. + */ + it should "refuse a CORRELATED subquery by naming the shape and the engine jar" in { val res = executeSync( "SELECT o.id FROM dql_orders o WHERE EXISTS " + "(SELECT 1 FROM dql_orders x WHERE x.customer_id = o.customer_id)" @@ -1417,7 +1423,26 @@ trait ReplGatewayIntegrationSpec extends ReplIntegrationTestKit { res shouldBe a[ExecutionFailure] val error = res.asInstanceOf[ExecutionFailure].error error.statusCode shouldBe Some(400) - error.message should include("Correlated subquery") + error.message should include("A correlated subquery") + error.message should include("softclient4es-arrow-extensions") + } + + /** Story 22.3 — a derived table reading an outer alias is SQL:1999 LATERAL, refused in the `sql` + * module, so the refusal holds at every venue and on every Elasticsearch major. + */ + it should "refuse a derived table reading an outer alias (LATERAL) with the ANSI message" in { + val res = executeSync( + "SELECT o.id FROM dql_orders o JOIN " + + "(SELECT x.id AS cid FROM dql_orders x WHERE x.id = o.id) d ON d.cid = o.id" + ) + res shouldBe a[ExecutionFailure] + val error = res.asInstanceOf[ExecutionFailure].error + // 🔴 Both substrings must survive `GatewayApi.excerpt`, which caps the reason at 200 characters + // and elides the MIDDLE (head 120 + "..." + tail 77). This row is why `lateralMessage` leads + // with the construct name: the first wording put "LATERAL" at character ~128 and the REPL user + // never saw it. MEASURED here on real Elasticsearch 8.18, not reasoned about. + error.message should include("LATERAL is not supported") + error.message should include("derived table cannot reference an outer alias") } it should "still answer the handshake — the subquery phase did not widen" in {