From 7c41c3138364b4974c11a9eeb1d251395dfb5440 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Manciot?= Date: Mon, 14 Sep 2026 09:49:25 +0200 Subject: [PATCH] feat(sql,core): parse derived tables in FROM and JOIN, and refuse them loudly until the engine can run them `FROM (SELECT ...) [AS] alias` and `[join_type] JOIN (SELECT ...) [AS] alias [ON ...]` now parse into a `DerivedTable` node whose `name` IS its correlation name, so every existing alias map, qualifier resolution and join-leg key works with no new arm. The statement is validated -- an outer reference must name a column the subquery projects, a body that reads an enclosing alias is rejected as LATERAL, a correlation name may not name anything else in the same FROM, and the body's own rules are checked one level down -- and then refused with HTTP 400 at every execution site, because a derived table's `sources` is an ALIAS and executing it naively would search an index named after the subquery. Nine of the eleven derived-table statements captured from Tableau and Superset now parse; the two that do not are declared residuals with a named owner. The Epic 21 headline is unchanged at SCORES 56/99 -- these rows score `residual` -- while the raw parse count moves 81 -> 90. Also here, because the grammar needs it: `whereCriteria` becomes a depth-aware scanner that leaves a depth-0 `)` to whoever opened it. Without it, a body whose last clause is WHERE or HAVING loses its own closing parenthesis to the inner clause and the whole statement is rejected as unbalanced. Behaviour changes, all release-noted: - the direct client API (`search` / `searchAsync` / `scroll` / `searchWithInnerHits`) returns 400 for a cross-index JOIN where it used to return rows from the first index with HTTP 200; - `searchAs` / `scrollAs` no longer compile for a cross-index JOIN or a derived table; - the closure rejection message names the shape and is venue-neutral; - `DELETE`, `CREATE WATCHER` and `CREATE MATERIALIZED VIEW` refuse derived tables by name; - a stray `)` is still rejected, as trailing input rather than as unbalanced parentheses. `Table` gains a defaulted `derived` field (arity 4->5), so downstream repositories rebuild on the next core bump. Closed Issue #330 --- .../resources/help/commands/dql/select.json | 15 +- .../client/RelationalClosureGuard.scala | 66 ++++ .../elastic/client/SearchApi.scala | 18 + .../client/extensions/CoreDqlExtension.scala | 39 +- .../client/RelationalClosureGuardSpec.scala | 233 ++++++++++++ .../extensions/CoreDqlExtensionSpec.scala | 62 ++++ .../elastic/client/help/HelpCorpusSpec.scala | 15 +- .../sql/macros/SQLQueryValidatorSpec.scala | 35 ++ .../sql/macros/SQLQueryValidator.scala | 31 +- .../app/softnetwork/elastic/sql/package.scala | 7 +- .../elastic/sql/parser/FromParser.scala | 50 ++- .../elastic/sql/parser/Parser.scala | 102 ++++-- .../elastic/sql/parser/WhereParser.scala | 81 ++++- .../softnetwork/elastic/sql/query/From.scala | 224 +++++++++++- .../elastic/sql/query/SubqueryScope.scala | 125 +++++++ .../elastic/sql/query/package.scala | 197 +++++++++- .../resources/corpus/epic-21-attribution.csv | 18 +- .../elastic/sql/census/CorpusReplaySpec.scala | 65 +++- .../sql/parser/DerivedTableCorpusSpec.scala | 148 ++++++++ .../elastic/sql/parser/DerivedTableSpec.scala | 335 ++++++++++++++++++ .../sql/parser/ParserTotalitySpec.scala | 29 +- .../repl/ReplGatewayIntegrationSpec.scala | 33 ++ 22 files changed, 1833 insertions(+), 95 deletions(-) create mode 100644 core/src/main/scala/app/softnetwork/elastic/client/RelationalClosureGuard.scala create mode 100644 core/src/test/scala/app/softnetwork/elastic/client/RelationalClosureGuardSpec.scala create mode 100644 sql/src/main/scala/app/softnetwork/elastic/sql/query/SubqueryScope.scala create mode 100644 sql/src/test/scala/app/softnetwork/elastic/sql/parser/DerivedTableCorpusSpec.scala create mode 100644 sql/src/test/scala/app/softnetwork/elastic/sql/parser/DerivedTableSpec.scala diff --git a/core/src/main/resources/help/commands/dql/select.json b/core/src/main/resources/help/commands/dql/select.json index 492a122e9..3df98b93a 100644 --- a/core/src/main/resources/help/commands/dql/select.json +++ b/core/src/main/resources/help/commands/dql/select.json @@ -5,7 +5,9 @@ "syntax": [ "SELECT [DISTINCT] columns", "FROM table [AS alias]", + "FROM (SELECT ...) [AS] alias", "[JOIN table ON condition]", + "[JOIN (SELECT ...) [AS] alias ON condition]", "[WHERE condition]", "[GROUP BY columns]", "[HAVING condition]", @@ -96,15 +98,24 @@ "description": "UNION ALL keeps every row of both sides; bare UNION is not accepted", "sql": "SELECT id, amount FROM orders UNION ALL SELECT id, amount FROM archived_orders", "output": null + }, + { + "title": "Derived table", + "description": "Aggregate in a subquery, filter the aggregate outside", + "sql": "SELECT d.category, d.total FROM (SELECT category, SUM(amount) AS total FROM orders GROUP BY category) AS d WHERE d.total > 100", + "output": null } ], "notes": [ "An alias must not be a reserved word: count, day, hour, now, today, offset, distance and every function name are rejected as aliases", "Result sets larger than the index max_result_window are paged automatically; the query itself needs no scroll or search_after handling", "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 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" + ], + "limitations": [ + "Derived tables parse and are validated but execute only through the relational engine (softclient4es-arrow-extensions); without it the statement is refused with HTTP 400" ], - "limitations": [], "seeAlso": ["INSERT", "UPDATE", "DELETE"], "minVersion": null, "aliases": [] diff --git a/core/src/main/scala/app/softnetwork/elastic/client/RelationalClosureGuard.scala b/core/src/main/scala/app/softnetwork/elastic/client/RelationalClosureGuard.scala new file mode 100644 index 000000000..c687cd8ca --- /dev/null +++ b/core/src/main/scala/app/softnetwork/elastic/client/RelationalClosureGuard.scala @@ -0,0 +1,66 @@ +/* + * Copyright 2025 SOFTNETWORK + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package app.softnetwork.elastic.client + +import app.softnetwork.elastic.client.result.ElasticError +import app.softnetwork.elastic.sql.query.{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). + * + * Two readers, one message: `CoreDqlExtension.execute` (the gateway path, where a join-capable + * extension would have claimed the statement first) and `SearchApi.resolveWithSchema` (the direct + * client API, which no extension sees). A second message would drift; a second PREDICATE would + * drift faster, which is why both read + * `app.softnetwork.elastic.sql.query.relationalClosureRequired`. + */ +object RelationalClosureGuard { + + val ExtensionJar = "softclient4es-arrow-extensions" + + /** Which construct triggered the refusal โ€” NAMED because the remedy differs: a cross-index JOIN + * can be rewritten by hand, a derived table is usually emitted by a BI tool the user does not + * control. When BOTH are present the derived table is reported, because it is the one the user + * did not choose. + */ + def shapeOf(statement: Statement): String = + if (derivedTablesPresent(statement)) "A derived table (subquery in FROM/JOIN)" + else "A cross-index JOIN" + + /** `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. + * + * ๐Ÿ”ด The wording is deliberately VENUE-NEUTRAL, which #157's original text was not. Since this + * story widened `SearchApi.resolveWithSchema` (lead ruling on OQ-1) the same message is returned + * to a Scala embedder calling `client.search(...)` and โ€” through `gateway.run` โ€” to the JDBC + * driver's `handleFailure` and the Flight sidecar, verbatim. "Re-run the installer" and a + * `repl.md` anchor are not actionable there, so the remedy names the JAR and the REPL flag is + * given as the REPL's spelling of it rather than as the only one. + */ + 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.", + statusCode = Some(400), + operation = Some(operation) + ) +} 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 104db3d46..2c4af57cf 100644 --- a/core/src/main/scala/app/softnetwork/elastic/client/SearchApi.scala +++ b/core/src/main/scala/app/softnetwork/elastic/client/SearchApi.scala @@ -137,6 +137,24 @@ trait SearchApi extends ElasticConversion with ElasticClientHelpers with SchemaC * i.e. `shouldBeScripted` across the clauses. */ private[client] def resolveWithSchema(single: SingleSearch): ElasticResult[SingleSearch] = { + // Story 22.1 (epic 22 AD-5) โ€” THE seam. Every direct-API path crosses it BEFORE routing + // (`search`, `searchAsync`, `searchWithInnerHits`, `ScrollApi.scroll`, and `IndicesApi`'s + // by-query DELETE/UPDATE bodies through `resolveDmlWithSchema`), and none of them passes + // through `CoreDqlExtension` โ€” so a guard placed here cannot be bypassed by a routing + // decision, and two guards would be two places for it to go missing. + // + // A derived table has no index to resolve a schema against and `single.sources` yields its + // ALIAS; a cross-index JOIN resolves only its FIRST table. MEASURED on `origin/main` before + // this story: `client.search("SELECT o.id, c.name FROM orders o JOIN customers c ON โ€ฆ")` + // returned `ElasticSuccess` over index list `[orders]` with a `match_all` body โ€” the JOIN leg + // silently dropped, HTTP 200, wrong answer. That is the #157 residual on the raw client API + // (no sibling production code calls this API โ€” every one goes through `gateway.run` โ€” which is + // why it survived), and the lead's ruling for this story is to close it here rather than file + // it. + if (single.relationalClosureRequired) + return ElasticResult.failure( + RelationalClosureGuard.rejection(single, operation = "search") + ) // #306 -- this used to return early for a statement whose WHERE carried no temporal literal. // That was correct while the only job was rewriting those literals, and is WRONG now that the // schema is also attached to the AST: almost no statement carries a temporal WHERE literal, diff --git a/core/src/main/scala/app/softnetwork/elastic/client/extensions/CoreDqlExtension.scala b/core/src/main/scala/app/softnetwork/elastic/client/extensions/CoreDqlExtension.scala index 4b8b1d639..e1f332aae 100644 --- a/core/src/main/scala/app/softnetwork/elastic/client/extensions/CoreDqlExtension.scala +++ b/core/src/main/scala/app/softnetwork/elastic/client/extensions/CoreDqlExtension.scala @@ -94,7 +94,12 @@ class CoreDqlExtension extends ExtensionSpi { // SELECT with a cross-index JOIN, mirroring the arrow JoinExtension's canHandle) so they // fail loudly in execute instead of falling through to the core DML/DDL executors, which // would silently run the inner SELECT as a single-index search and write wrong data. - case other => joinRequired(other) + // Issue #157 / story 22.1 โ€” also claim write-with-CLOSURE statements (INSERT โ€ฆ SELECT / + // CREATE TABLE โ€ฆ AS SELECT carrying a cross-index JOIN or a derived table, mirroring the arrow + // JoinExtension's canHandle) so they fail loudly in execute instead of falling through to the + // core DML/DDL executors, which would silently run the inner SELECT as a single-index search + // and write wrong data. + case other => relationalClosureRequired(other) } override def execute( @@ -110,19 +115,8 @@ class CoreDqlExtension extends ExtensionSpi { // INSERT โ€ฆ SELECT / CTAS, write that wrong data. With a join-capable extension // registered ahead (priority < 100) join statements never get here; without one, fail // loudly instead of returning wrong data. - case s if joinRequired(s) => - Future.successful( - ElasticFailure( - ElasticError( - message = - "Cross-index JOIN requires the softclient4es-arrow-extensions jar (Java 11+). " + - "Re-run the installer, or run with --no-extensions removed. " + - "See documentation/client/repl.md#extensions.", - statusCode = Some(400), - operation = Some("join") - ) - ) - ) + case s if relationalClosureRequired(s) => + Future.successful(ElasticFailure(RelationalClosureGuard.rejection(s))) case dql: DqlStatement => licenseManager match { @@ -150,18 +144,11 @@ class CoreDqlExtension extends ExtensionSpi { "SELECT ... FROM ... WHERE ... GROUP BY ... HAVING ... LIMIT ..." ) - /** True when the statement carries a cross-index JOIN (`from.enrichmentRequired`), including - * embedded in INSERT โ€ฆ SELECT / CREATE TABLE โ€ฆ AS SELECT. UNNEST joins are excluded by - * construction โ€” `joinedTables` collects `StandardJoin` sources only. - */ - private[this] def joinRequired(statement: Statement): Boolean = statement match { - case single: SingleSearch => single.from.enrichmentRequired - case multi: MultiSearch => multi.requests.exists(_.from.enrichmentRequired) - case select: SelectStatement => select.statement.exists(joinRequired) - case insert: Insert => insert.values.left.exists(joinRequired) - case create: CreateTable => create.ddl.left.exists(joinRequired) - case _ => false - } + // Story 22.1 โ€” `joinRequired` DELETED. The predicate now lives in the `sql` module + // (`app.softnetwork.elastic.sql.query.relationalClosureRequired`, folded over `closureSearches`) + // so core, the `searchAs` macro and the arrow venue read ONE definition with ONE list of + // statement arms. It covers the same shapes it always did โ€” a cross-index JOIN, embedded in + // INSERT โ€ฆ SELECT / CTAS, with UNNEST excluded by construction โ€” plus derived tables. // โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ•โ• // โœ… QUOTA CHECK LOGIC (in extension, not in core) diff --git a/core/src/test/scala/app/softnetwork/elastic/client/RelationalClosureGuardSpec.scala b/core/src/test/scala/app/softnetwork/elastic/client/RelationalClosureGuardSpec.scala new file mode 100644 index 000000000..ee16233b8 --- /dev/null +++ b/core/src/test/scala/app/softnetwork/elastic/client/RelationalClosureGuardSpec.scala @@ -0,0 +1,233 @@ +/* + * Copyright 2025 SOFTNETWORK + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package app.softnetwork.elastic.client + +import app.softnetwork.elastic.client.result._ +import app.softnetwork.elastic.sql.parser.Parser +import app.softnetwork.elastic.sql.query.{SearchStatement, SingleSearch} +import org.scalatest.flatspec.AnyFlatSpec +import org.scalatest.matchers.should.Matchers +import org.slf4j.{Logger, LoggerFactory} + +/** Story 22.1 โ€” the DIRECT client API seam (`SearchApi.resolveWithSchema`) and the sweep of every + * real `Parser(` dispatch site in core. + * + * Docker-free: `NopeClientApi` answers everything benignly, so a statement that is NOT refused + * comes back `ElasticSuccess`. That is what makes the assertions falsifiable โ€” the pre-story + * behaviour of the JOIN row below was a MEASURED `ElasticSuccess` over index list `[orders]` with + * a `match_all` body. + * + * ๐Ÿ”ด The 13 `Parser(` grep hits in this repository are SEVEN real dispatch sites: the four + * remaining hits are comments (`sql/package.scala`, `parser/time/package.scala`, + * `query/TemporalLiterals.scala`, `query/package.scala`) or unrelated APIs (Jackson's + * `createParser` in `file/package.scala`, JLine's `.parser` in `repl/Repl.scala`). The seven are + * `SQLImplicits`, `IndicesApi` ร—3 (update / delete / insert by query), `GatewayApi.run`, + * `PipelineApi.pipeline` and the `searchAs` macro โ€” the last two covered by `CoreDqlExtensionSpec` + * / `SQLQueryValidatorSpec`. + */ +class RelationalClosureGuardSpec extends AnyFlatSpec with Matchers { + + private val testLogger: Logger = LoggerFactory.getLogger(getClass) + + private def client(): ElasticClientApi = new NopeClientApi { + override protected def logger: Logger = testLogger + } + + private implicit val context: ConversionContext = NativeContext + + private def searchStatement(sql: String): SearchStatement = Parser(sql) match { + case Right(s: SearchStatement) => s + case Right(other) => fail(s"[$sql] expected a search statement, got $other") + case Left(e) => fail(s"[$sql] rejected: ${e.msg}") + } + + private def refusalOf(res: ElasticResult[_]): ElasticError = res match { + case ElasticFailure(e) => e + case ElasticSuccess(v) => fail(s"expected a refusal, got ElasticSuccess($v)") + } + + 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" + + // ---- the ONE seam ------------------------------------------------------------------------ + + behavior of "SearchApi.resolveWithSchema (the direct-API seam)" + + it should "refuse a derived-table statement before any router sees it" in { + val err = refusalOf(client().search(searchStatement(DerivedSelect))) + err.statusCode shouldBe Some(400) + err.operation shouldBe Some("search") + err.message should include(RelationalClosureGuard.ExtensionJar) + err.message should include("derived table") + } + + /** ๐Ÿ”ด The #157 residual on the raw client API, closed by this story on the lead's ruling. + * + * MEASURED on `origin/main` before the change: this returned `ElasticSuccess(ElasticResponse(โ€ฆ, + * {"query": {"match_all": {}}}, โ€ฆ))` over index list `[orders]` โ€” the JOIN leg silently dropped, + * HTTP 200, wrong answer. No sibling production code calls this API (every one of them goes + * through `gateway.run`), which is why it survived #157. โš ๏ธ Behaviour change, release-noted. + */ + it should "refuse a cross-index JOIN on the direct API (the pre-existing #157 residual)" in { + val err = refusalOf(client().search(searchStatement(JoinSelect))) + err.statusCode shouldBe Some(400) + err.operation shouldBe Some("search") + err.message should include(RelationalClosureGuard.ExtensionJar) + err.message should include("cross-index JOIN") + } + + /** The controls. `NopeClientApi` refuses every real search (it has no cluster), so the assertion + * is that the statement got PAST this guard and failed for the client's own reason โ€” never that + * it succeeded, which would be unobservable here. A guard that widened too far reddens these. + */ + it should "leave a plain statement alone" in { + val err = refusalOf(client().search(searchStatement("SELECT a FROM t LIMIT 5"))) + err.message should not include RelationalClosureGuard.ExtensionJar + } + + it should "leave a JOIN UNNEST alone โ€” it is not a cross-index JOIN" in { + val err = refusalOf( + client().search( + searchStatement("SELECT o.id, oi.q FROM orders o JOIN UNNEST(o.items) AS oi LIMIT 5") + ) + ) + err.message should not include RelationalClosureGuard.ExtensionJar + } + + it should "refuse a UNION ALL whose leg carries a derived table" in { + val err = refusalOf( + client().search(searchStatement(s"$DerivedSelect UNION ALL SELECT a FROM t")) + ) + err.statusCode shouldBe Some(400) + } + + /** ๐Ÿ”ด AC 6 in full. `search` is only ONE of the paths that cross the seam; `scroll`, + * `searchAsync` and `searchWithInnerHits` each call `resolveWithSchema` at their OWN call site + * (`ScrollApi:305`, `SearchApi:715/:741`, `SearchApi:1268/:1279`). Without a row each, deleting + * the guard line would redden the `search` rows alone โ€” and `scroll` is the documented path for + * large extractions (#238), i.e. the one an embedder is most likely to call directly. + */ + it should "refuse a derived-table statement on searchAsync too" in { + import scala.concurrent.Await + import scala.concurrent.duration._ + implicit val ec: scala.concurrent.ExecutionContext = scala.concurrent.ExecutionContext.global + val err = refusalOf( + Await.result(client().searchAsync(searchStatement(DerivedSelect)), 10.seconds) + ) + err.statusCode shouldBe Some(400) + err.message should include(RelationalClosureGuard.ExtensionJar) + } + + it should "refuse a derived-table statement on scroll too" in { + import akka.stream.scaladsl.Sink + import scala.concurrent.Await + import scala.concurrent.duration._ + implicit val system: akka.actor.ActorSystem = akka.actor.ActorSystem("closure-guard-scroll") + try { + val thrown = intercept[Throwable] { + Await.result( + client().scroll(searchStatement(DerivedSelect)).runWith(Sink.seq), + 20.seconds + ) + } + // the refusal reaches the stream as a failure carrying OUR message, never as an empty stream + // (an empty stream is what a silently-executed wrong query would look like) + Option(thrown.getMessage).getOrElse("") should include(RelationalClosureGuard.ExtensionJar) + } finally { + Await.result(system.terminate(), 20.seconds) + () + } + } + + it should "refuse a derived-table statement on searchWithInnerHits too" in { + import org.json4s.DefaultFormats + implicit val formats: org.json4s.Formats = DefaultFormats + val select = app.softnetwork.elastic.sql.query.SelectStatement(DerivedSelect) + select.statement.map(_.getClass.getSimpleName) shouldBe Some("SingleSearch") + val err = + refusalOf(client().searchWithInnerHits[Map[String, Any], Map[String, Any]](select, "x")) + err.statusCode shouldBe Some(400) + err.message should include(RelationalClosureGuard.ExtensionJar) + } + + // ---- the message ------------------------------------------------------------------------ + + behavior of "RelationalClosureGuard" + + it should "name the DERIVED shape when both shapes are present" in { + val stmt = Parser( + "SELECT o.id FROM orders o JOIN (SELECT cid FROM orders) AS d ON o.id = d.cid" + ).toOption.getOrElse(fail("rejected")) + stmt.asInstanceOf[SingleSearch].from.enrichmentRequired shouldBe true + RelationalClosureGuard.shapeOf(stmt) should include("derived table") + } + + 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") + } + + // ---- the dispatch-site sweep ------------------------------------------------------------- + + behavior of "every real Parser( dispatch site in core" + + /** `parseQueryForUpdate` parses ONLY strings that `startsWith("UPDATE")`; anything else is + * treated as a raw JSON body, so a SELECT string never reaches its `SingleSearch` arm. And + * `UPDATE` itself takes `identRef` and rejects any `FROM`, so no derived table can enter an + * UPDATE at all. Both halves are pinned, because a sweep test written as `updateByQuery(idx, + * "SELECT โ€ฆ")` would be VACUOUS. + */ + it should "refuse an UPDATE that names a FROM at all, and never parse a SELECT body" in { + val c = client().asInstanceOf[IndicesApi] + refusalOf( + c.updateByQuery("idx", "UPDATE idx SET a = 1 FROM (SELECT id FROM t) d") + ).statusCode shouldBe Some(400) + // not SQL to this site โ€” it is an invalid JSON body, never a parsed statement + refusalOf(c.updateByQuery("idx", DerivedSelect)).statusCode should not be None + } + + it should "refuse a DELETE targeting a derived table (the grammar's own err)" in { + val err = refusalOf( + client() + .asInstanceOf[IndicesApi] + .deleteByQuery("idx", "DELETE FROM (SELECT id FROM t) d WHERE id = 1") + ) + err.message should include("DELETE cannot target a derived table") + } + + it should "refuse an INSERT ... SELECT carrying a derived table" in { + import scala.concurrent.Await + import scala.concurrent.duration._ + implicit val system: akka.actor.ActorSystem = akka.actor.ActorSystem("closure-guard-spec") + try { + val res = Await.result( + client().asInstanceOf[IndicesApi].insertByQuery("idx", s"INSERT INTO idx $DerivedSelect"), + 10.seconds + ) + refusalOf(res).statusCode should not be None + } finally { + Await.result(system.terminate(), 10.seconds) + () + } + } + + it should "keep the pipeline DDL site loud for a SELECT" in { + refusalOf(client().asInstanceOf[PipelineApi].pipeline(DerivedSelect)).message should include( + "Unsupported pipeline DDL statement" + ) + } +} 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 428cea423..98dc5b124 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 @@ -405,6 +405,68 @@ class CoreDqlExtensionSpec extends AnyFlatSpec with Matchers { ext.canHandle(parsed("CREATE TABLE target AS SELECT id, name FROM old_users")) shouldBe false } + // ---- Story 22.1: the guard widens from "cross-index JOIN" to "relational closure" ---- + + behavior of "CoreDqlExtension relational-closure guard (story 22.1)" + + /** AC 6's proof, and the reason `returnsRows` needed no change: a derived-table statement reports + * `returnsRows == true` (it has no aggregate and no GROUP BY), yet NOTHING routes on that, + * because the closure arm runs before `checkQuotasAndExecute`. Asserted through the + * RecordingClient seam rather than argued. + */ + it should "never reach scroll or searchAsync for a FROM-derived statement" in { + val (client, res) = run("SELECT COL FROM (SELECT 1 AS COL) AS d", Quota.Community) + res shouldBe a[ElasticFailure] + val err = res.asInstanceOf[ElasticFailure].elasticError + err.statusCode shouldBe Some(400) + err.operation shouldBe Some("join") + err.message should include("softclient4es-arrow-extensions") + err.message should include("derived table") + client.scrolledStatement.get() shouldBe null + client.searchedStatement.get() shouldBe null + } + + it should "reject a JOIN-derived statement the same way" in { + val (client, res) = run( + "SELECT o.id, d.cid FROM orders o JOIN (SELECT cid FROM orders) AS d ON o.id = d.cid", + Quota.Community + ) + res shouldBe a[ElasticFailure] + res.asInstanceOf[ElasticFailure].elasticError.message should include("derived table") + 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", + "CREATE TABLE target AS SELECT COL FROM (SELECT 1 AS COL) AS d" + ).foreach { sql => + val (client, res) = run(sql, Quota.Community) + withClue(s"[$sql] ") { + res shouldBe a[ElasticFailure] + res.asInstanceOf[ElasticFailure].elasticError.statusCode shouldBe Some(400) + client.scrolledStatement.get() shouldBe null + client.searchedStatement.get() shouldBe null + } + } + val ext = new CoreDqlExtension() + def parsed(sql: String) = Parser(sql) match { + case Right(s) => s + case Left(e) => fail(s"parse failed: ${e.msg}") + } + ext.canHandle(parsed("INSERT INTO target SELECT COL FROM (SELECT 1 AS COL) AS d")) shouldBe true + ext.canHandle( + parsed("CREATE TABLE target AS SELECT COL FROM (SELECT 1 AS COL) AS d") + ) shouldBe true + } + + it should "still execute a plain statement โ€” the guard did not widen past closure shapes" in { + val (client, res) = run("SELECT a FROM t LIMIT 5", Quota.Community) + res shouldBe a[ElasticSuccess[_]] + client.searchedStatement.get() should not be null + } + // ---- Story P0.6: QueryResults cap-hit recorded on BOTH reject branches ---- behavior of "CoreDqlExtension cap-hit instrumentation (P0.6)" diff --git a/core/src/test/scala/app/softnetwork/elastic/client/help/HelpCorpusSpec.scala b/core/src/test/scala/app/softnetwork/elastic/client/help/HelpCorpusSpec.scala index 9b825f6b0..606aa66d5 100644 --- a/core/src/test/scala/app/softnetwork/elastic/client/help/HelpCorpusSpec.scala +++ b/core/src/test/scala/app/softnetwork/elastic/client/help/HelpCorpusSpec.scala @@ -796,8 +796,21 @@ class HelpCorpusSpec extends AnyFlatSpec with Matchers { // concrete leaves from a result-type scan, which is precisely how `MultiSearch` (`UNION ALL`) // slipped past `searchStatement`. A new entry here must be accompanied by a check that the // package walk below still reaches its leaves. + // Story 22.1 adds `derivedTableBodyInner`: the BODY of a derived table (`FROM (SELECT โ€ฆ) AS + // d`), typed `DqlStatement` because the body may be a FROM-less SELECT. It names no NEW + // statement leaf โ€” the body is `searchStatement | fromlessSelect`, both already enumerated โ€” + // and no user can type one as a statement of its own, so the package walk's coverage is + // unchanged. Verified by the superset assertion below and by this file's parser -> doc gate + // staying green with no new help document. val expectedAbstract = - Set("statement", "dqlStatement", "ddlStatement", "dmlStatement", "searchStatement") + Set( + "statement", + "dqlStatement", + "ddlStatement", + "dmlStatement", + "searchStatement", + "derivedTableBodyInner" + ) withClue( "the set of productions returning a SEALED TRAIT has changed. Every one of them hides its " + "concrete leaves from a result-type scan; confirm the package walk reaches them, then " + diff --git a/macros-tests/src/test/scala/app/softnetwork/elastic/sql/macros/SQLQueryValidatorSpec.scala b/macros-tests/src/test/scala/app/softnetwork/elastic/sql/macros/SQLQueryValidatorSpec.scala index 25da5e0d9..b51f85ccc 100644 --- a/macros-tests/src/test/scala/app/softnetwork/elastic/sql/macros/SQLQueryValidatorSpec.scala +++ b/macros-tests/src/test/scala/app/softnetwork/elastic/sql/macros/SQLQueryValidatorSpec.scala @@ -5,6 +5,41 @@ import org.scalatest.matchers.should.Matchers class SQLQueryValidatorSpec extends AnyFlatSpec with Matchers { + // ============================================================ + // Story 22.1 โ€” relational-closure statements cannot be typed at compile time + // ============================================================ + + // A derived table is a `SingleSearch`, NOT a new statement KIND, so story 20.9's + // `case Right(other)` abort does NOT catch it: without the new guarded arm the macro would + // ACCEPT this and type `Row` against nothing. + it should "REJECT a derived table at compile time (one index mapping cannot type it)" in { + assertDoesNotCompile(""" + import app.softnetwork.elastic.client.macros.TestElasticClientApi + import app.softnetwork.elastic.client.macros.TestElasticClientApi.defaultFormats + import app.softnetwork.elastic.sql.query.SelectStatement + + case class Row(COL: Int) + + TestElasticClientApi.searchAs[Row]( + "SELECT COL FROM (SELECT 1 AS COL) AS d" + )""") + } + + // โš ๏ธ BEHAVIOUR CHANGE, release-noted: this used to COMPILE and then run the FIRST index alone + // (the #157 silent-wrong-answer mode). It is a compile error now. + it should "REJECT a cross-index JOIN at compile time (this used to compile and run one table)" in { + assertDoesNotCompile(""" + import app.softnetwork.elastic.client.macros.TestElasticClientApi + import app.softnetwork.elastic.client.macros.TestElasticClientApi.defaultFormats + import app.softnetwork.elastic.sql.query.SelectStatement + + case class Row(id: Int, name: String) + + TestElasticClientApi.searchAs[Row]( + "SELECT o.id, c.name FROM orders o JOIN customers c ON o.cid = c.id" + )""") + } + // ============================================================ // Positive Tests (Should Compile) // ============================================================ diff --git a/macros/src/main/scala/app/softnetwork/elastic/sql/macros/SQLQueryValidator.scala b/macros/src/main/scala/app/softnetwork/elastic/sql/macros/SQLQueryValidator.scala index 11dab04ca..18308c325 100644 --- a/macros/src/main/scala/app/softnetwork/elastic/sql/macros/SQLQueryValidator.scala +++ b/macros/src/main/scala/app/softnetwork/elastic/sql/macros/SQLQueryValidator.scala @@ -19,7 +19,13 @@ package app.softnetwork.elastic.sql.macros import app.softnetwork.elastic.sql.`type`.{SQLType, SQLTypes, SQLVarchar} import app.softnetwork.elastic.sql.function.aggregate.{COUNT, WindowFunction} import app.softnetwork.elastic.sql.parser.Parser -import app.softnetwork.elastic.sql.query.{MultiSearch, SingleSearch} +import app.softnetwork.elastic.sql.query.{ + derivedTablesPresent, + relationalClosureRequired, + MultiSearch, + SingleSearch, + Statement +} import scala.language.experimental.macros import scala.reflect.macros.blackbox @@ -172,9 +178,20 @@ trait SQLQueryValidator { // ============================================================ private def parseSQLQuery(c: blackbox.Context)(sqlQuery: String): SingleSearch = { Parser(sqlQuery) match { + // Story 22.1 โ€” a derived table is a `SingleSearch`, NOT a new statement KIND, so the + // `case Right(other)` arm below (story 20.9's) does NOT catch it: without this guard the + // macro would ACCEPT the statement and type the OUTER select against nothing. + // `searchAs` / `scrollAs` bind ONE index mapping and can type neither a derived table's + // projection nor a JOIN's merged row. + case Right(request: SingleSearch) if relationalClosureRequired(request) => + c.abort(c.enclosingPosition, closureAbortMessage(request, sqlQuery)) + case Right(request: SingleSearch) => request + case Right(multi: MultiSearch) if relationalClosureRequired(multi) => + c.abort(c.enclosingPosition, closureAbortMessage(multi, sqlQuery)) + case Right(multi: MultiSearch) => multi.requests.headOption.getOrElse { c.abort(c.enclosingPosition, "โŒ Empty multi-search query") @@ -201,6 +218,18 @@ trait SQLQueryValidator { } } + /** โš ๏ธ Behaviour change (story 22.1, release note): a `searchAs[T]("โ€ฆ JOIN โ€ฆ")` that COMPILED + * before โ€” and then ran the FIRST index alone โ€” is a compile error now. That is the #157 + * silent-wrong-answer mode, moved from run time to compile time. + */ + private def closureAbortMessage(statement: Statement, sqlQuery: String): String = { + val shape = + if (derivedTablesPresent(statement)) "Derived tables (subqueries in FROM/JOIN)" + else "Cross-index JOINs" + s"โŒ $shape cannot be typed at compile time: searchAs/scrollAs bind one index mapping. " + + s"Run this statement through GatewayApi.run with the relational engine.\nQuery: $sqlQuery" + } + // ============================================================ // Reject SELECT * (incompatible with compile-time validation) // ============================================================ diff --git a/sql/src/main/scala/app/softnetwork/elastic/sql/package.scala b/sql/src/main/scala/app/softnetwork/elastic/sql/package.scala index b73cca9d7..f54ffecc7 100644 --- a/sql/src/main/scala/app/softnetwork/elastic/sql/package.scala +++ b/sql/src/main/scala/app/softnetwork/elastic/sql/package.scala @@ -1709,8 +1709,11 @@ package object sql { ) } } else { - // maybe from the main table or a subquery, not a JOIN - // here we only take into account the main table, not subqueries + // An UN-QUALIFIED name. It resolves against the MAIN table, whatever that is โ€” and since + // story 22.1 the main table may itself be a DERIVED table, in which case there is no + // schema to attach (a subquery has no mapping) and the name is checked against the derived + // table's PROJECTION instead, by `SingleSearch.derivedScopeCheck`. Nothing to do here: a + // derived table's `name` IS its correlation name, so every alias map already agrees. this .copy( fieldAlias = request.fieldAliases.get(identifierName).orElse(fieldAlias), diff --git a/sql/src/main/scala/app/softnetwork/elastic/sql/parser/FromParser.scala b/sql/src/main/scala/app/softnetwork/elastic/sql/parser/FromParser.scala index 0846704cb..0475bcf7e 100644 --- a/sql/src/main/scala/app/softnetwork/elastic/sql/parser/FromParser.scala +++ b/sql/src/main/scala/app/softnetwork/elastic/sql/parser/FromParser.scala @@ -19,6 +19,7 @@ package app.softnetwork.elastic.sql.parser import app.softnetwork.elastic.sql.GenericIdentifier import app.softnetwork.elastic.sql.query.{ CrossJoin, + DerivedTable, From, FullJoin, InnerJoin, @@ -79,6 +80,11 @@ trait FromParser { * `StandardJoin.sql` renders each `NamePart` as ONE lexeme instead (21.2 AD-5). */ def source: PackratParser[StandardJoin] = + derivedTable ^^ { dt => + // A derived JOIN leg owns its alias and its parts are empty โ€” the AD-1 invariant that + // `StandardJoin.validate()` ENFORCES. + StandardJoin(source = dt, joinType = None, on = None, alias = None, parts = Nil) + } | tableParts ~ alias.? ^^ { case ps ~ a => StandardJoin( source = GenericIdentifier(ps.last.value), @@ -104,8 +110,48 @@ trait FromParser { * no longer deletes a clause the statement carried. */ def table: PackratParser[Table] = - tableParts ~ alias.? ~ rep(join) ^^ { case ps ~ a ~ js => - Table(ps.last.value, a, js, parts = ps) + (derivedTable ^^ { dt => Table(dt.name, None, Nil, Nil, derived = Some(dt)) } | + tableParts ~ alias.? ^^ { case ps ~ a => Table(ps.last.value, a, Nil, parts = ps) }) ~ + rep(join) ^^ { case t ~ js => t.copy(joins = js) } + + /** `(SELECT โ€ฆ) [AS] alias` โ€” a derived table (SQL-92 ยง7.6 ``). + * + * DISJOINT from `tableParts` at the FIRST character: this production begins with the literal + * `(`, while `tableParts` begins with a name character or a quote (`qualifiedName` = + * `(quotedPart | bareFirstPart) ~ nameTail`). Neither can match a prefix of an input the other + * accepts whole, so the alternation order narrows nothing either way โ€” `derivedTable` is listed + * first only because a literal test fails faster than a 131-alternative reserved-word regex. + * + * The ALIAS is mandatory (SQL-92 requires a ``; MySQL 8.4 raises error 1248; + * 10 of the 11 captured BI statements carry one) and its absence is an `err`, never a `failure`: + * a `failure` would fall through `rep1sep(table, separator)` / `rep(join)` and report a position + * error naming neither the derived table nor the alias (the #213 mode). The emptiness test is on + * `alias.alias`, not on the `Option`: `regexAlias`'s character class is `*`, so `alias` can + * succeed with an EMPTY name and `alias.?` is not a reliable absence test. + * + * The body alternative carries its own `err` for a parenthesised non-SELECT (`FROM (t) x`, `FROM + * (SHOW TABLES) x`): without it the rejection is `tableParts`' identifier-regex failure, a + * grammar-internal message this project never pins. Raising an `err` this early in the input is + * safe: an `err` is discarded only by a SIBLING `Failure` that got FURTHER (story 21.4), and the + * only sibling here โ€” `tableParts` โ€” fails at the very same `(`. + * + * `alias.?` declines a following keyword by construction: `regexAliasRegex` carries the + * reserved-word negative lookahead, so `โ€ฆ ) WHERE ROWNUM โ€ฆ` yields `None` here and the alias + * `err` fires โ€” which is how the Oracle corpus row gets OUR message rather than a lexer's. + * + * Recursion (`table -> derivedTable -> searchStatement -> single -> from -> table`) runs through + * a CONSUMED `(`, so it is not left recursion; Packrat memoises it and nesting is unbounded. + */ + override def derivedTable: PackratParser[DerivedTable] = + (start ~> (derivedTableBodyInner | err( + "A derived table body must be a SELECT: write FROM (SELECT ...) AS " + )) <~ end) ~ alias.? >> { + case body ~ Some(a) if a.alias.nonEmpty => success(DerivedTable(body, a)) + case _ => + err( + "A derived table requires an alias (SQL-92 correlation name): " + + "write FROM (SELECT ...) AS or JOIN (SELECT ...) AS ON ..." + ) } def from: PackratParser[From] = From.regex ~ rep1sep(table, separator) ^^ { case _ ~ tables => 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 2868d7ccc..99e168fac 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 @@ -1029,34 +1029,44 @@ object Parser // error that names neither JOIN nor the watcher. def searchInput: PackratParser[SearchWatcherInput] = from ~ opt(where) ~ withinTimeout >> { case f ~ w ~ t => - f.joins match { - case Nil => - val criteria = resolveWhere(f, w).flatMap(_.criteria) - // `FROM a, b` stays a legitimate multi-index search; only a qualifier over it is - // unserviceable โ€” see `qualifiedOverManyIndices`. - if (qualifiedOverManyIndices(f, criteria)) - err( - s"A watcher input cannot qualify a column by table when it searches several " + - s"indices (${f.tables.map(_.name).mkString(", ")}): one Elasticsearch search " + - "applies one query to all of them, so it can neither join them nor scope a " + - "predicate to one. Watch a single index, drop the qualifiers, or pre-join the " + - "sources with a MATERIALIZED VIEW and watch the view." - ) - else - success( - SearchWatcherInput( - f.tables.map(_.name).distinct, - criteria, - t + // Story 22.1 โ€” a watcher input keeps only `tables.map(_.name)`, and a derived table's name + // is its ALIAS, so the watcher would silently watch an index named after the subquery. Same + // shape, same reason, as the JOIN refusal below (#191). + if (f.hasDerivedTables) + err( + "A watcher input cannot search a derived table (subquery in FROM/JOIN): a watcher " + + "searches indices. Watch the index directly, or pre-compute the subquery as a " + + "MATERIALIZED VIEW and watch the view." + ) + else + f.joins match { + case Nil => + val criteria = resolveWhere(f, w).flatMap(_.criteria) + // `FROM a, b` stays a legitimate multi-index search; only a qualifier over it is + // unserviceable โ€” see `qualifiedOverManyIndices`. + if (qualifiedOverManyIndices(f, criteria)) + err( + s"A watcher input cannot qualify a column by table when it searches several " + + s"indices (${f.tables.map(_.name).mkString(", ")}): one Elasticsearch search " + + "applies one query to all of them, so it can neither join them nor scope a " + + "predicate to one. Watch a single index, drop the qualifiers, or pre-join the " + + "sources with a MATERIALIZED VIEW and watch the view." + ) + else + success( + SearchWatcherInput( + f.tables.map(_.name).distinct, + criteria, + t + ) ) + case joins => + err( + s"JOIN is not supported in a watcher input (${joins.map(_.sql.trim).mkString(" ")}): " + + "a watcher input can only search one or more indices (FROM index1, index2). " + + "Pre-join the sources with a MATERIALIZED VIEW and have the watcher search the view." ) - case joins => - err( - s"JOIN is not supported in a watcher input (${joins.map(_.sql.trim).mkString(" ")}): " + - "a watcher input can only search one or more indices (FROM index1, index2). " + - "Pre-join the sources with a MATERIALIZED VIEW and have the watcher search the view." - ) - } + } } def httpInput: PackratParser[HttpInput] = @@ -1277,6 +1287,20 @@ object Parser RefreshLicense } + /** The body of a derived table, WITHOUT its parentheses (story 22.1 AD-3). + * + * The SAME pair, in the SAME order, as `dqlStatement` below, for the same reason: `|` commits to + * the first SUCCEEDING alternative and `searchStatement` FAILS (does not partially succeed) on a + * FROM-less body because `single` requires `from`, so `(SELECT 1 AS COL)` falls through to + * `fromlessSelect` and `(SELECT a FROM t)` never does. Putting `fromlessSelect` first would + * commit `(SELECT a FROM t)` to the prefix `SELECT a` and then fail on `FROM`. Do not reorder. + * + * The ascription is needed because `Parser[+T].|[U >: T]` cannot unify `SearchStatement` with + * `FromlessSelect`; their common supertype is `DqlStatement`. + */ + override def derivedTableBodyInner: PackratParser[DqlStatement] = + (searchStatement: PackratParser[DqlStatement]) | fromlessSelect + def dqlStatement: PackratParser[DqlStatement] = { searchStatement | // Issue #251 โ€” FROM-less SELECT. MUST stay immediately AFTER searchStatement: `|` commits @@ -1458,6 +1482,16 @@ object Parser (keyword("DELETE") ~ keyword("FROM")) ~> rep1sep(table, separator) ~ where.? >> { case tables ~ w => tables.flatMap(_.joins) match { + // Story 22.1 โ€” `DELETE FROM` shares `table` with the SELECT surface, so without this arm + // a derived table would parse and `Delete(tables.head, โ€ฆ)` would carry a `Table` whose + // name is the subquery's ALIAS: the delete-by-query would target an index that does not + // exist, or worse one that happens to. Nothing downstream re-checks it (#191's class). + case Nil if tables.exists(_.derived.isDefined) => + err( + "DELETE cannot target a derived table (subquery in FROM): Elasticsearch deletes by " + + "query over an index. Name the index and move the subquery's predicate into the " + + "WHERE clause." + ) case Nil if tables.size > 1 => err( s"DELETE targets a single table, got ${tables.map(_.name).mkString(", ")}: " + @@ -1677,6 +1711,22 @@ trait Parser protected def keyword(word: String): Parser[String] = s"(?i)$word\\b".r ^^ (_ => word) + /** Story 22.1 โ€” the derived-table productions, DECLARED here because `FromParser` (which owns + * `derivedTable`) sees this trait through its self-type while `searchStatement` / + * `fromlessSelect` live on `object Parser`. Implemented there and in `FromParser` respectively. + * + * Story 22.2 reuses `derivedTableBodyInner` for `IN (SELECT โ€ฆ)` / `EXISTS (SELECT โ€ฆ)` โ€” with its + * OWN `start`/`end`, and WITHOUT `derivedTable`'s `err` alternative, which would fire on `WHERE + * a = (b + 1)` before the alternation could fall back to `equality`. The paren-bearing + * `derivedTableBody = start ~> derivedTableBodyInner <~ end` the 22.1 spec named is deliberately + * NOT shipped here: nothing in this story calls it (`derivedTable` inlines the parentheses so + * that an UNTERMINATED body stays a plain `Failure` instead of taking the `err` branch), and an + * artifact ships no unreachable code. It is one line for 22.2 to add at its own call site. + */ + def derivedTableBodyInner: PackratParser[app.softnetwork.elastic.sql.query.DqlStatement] + + def derivedTable: PackratParser[app.softnetwork.elastic.sql.query.DerivedTable] + /** The pre-21.7 DDL/DML name regex. It is no longer used directly by any statement: story 21.7 * routed every one of its call sites through `identRef` (object references) or `identName` * (columns, option keys, struct-entry keys), which accept the same bare spelling PLUS both quote diff --git a/sql/src/main/scala/app/softnetwork/elastic/sql/parser/WhereParser.scala b/sql/src/main/scala/app/softnetwork/elastic/sql/parser/WhereParser.scala index f796d755a..a2b59676a 100644 --- a/sql/src/main/scala/app/softnetwork/elastic/sql/parser/WhereParser.scala +++ b/sql/src/main/scala/app/softnetwork/elastic/sql/parser/WhereParser.scala @@ -266,13 +266,17 @@ trait WhereParser { /** The token stream inside a relation predicate's parentheses. * - * `whereCriteria` is `rep1(allPredicate | allCriteria | start | or | and | end | then_case)`. - * This is that alternation minus THREE of those, and the differences are ENUMERATED rather than + * `whereCriteria` scans the alternation `allPredicate | allCriteria | start | or | and | end | + * then_case` (since story 22.1 with a depth rule, but the ITEM alternation is unchanged). This + * is that alternation minus THREE of those, and the differences are ENUMERATED rather than * summarised, because the correctness claim rests on exactly which ones are missing: * * - a bare `end` is omitted - the closing parenthesis belongs to the relation predicate, and - * `rep1` does NOT backtrack, so leaving it in would let the repetition swallow that `)` and - * everything after it, after which the enclosing end could never match; + * the scan does NOT backtrack, so leaving it in would let the repetition swallow that `)` + * and everything after it, after which the enclosing end could never match. (Story 22.1's + * depth rule solves the SAME problem one construct up, for a parenthesised derived-table + * body; it is not a substitute here, because a relation predicate's `)` is at depth 0 of a + * group this production opened itself through `relationGroup`.); * - a bare `start` is omitted for the mirror reason: an opening parenthesis must only ever be * consumed by `relationGroup`, together with its OWN closing one, which is what keeps the * two balanced and lets sub-groups nest; @@ -369,9 +373,59 @@ trait WhereParser { private def allCriteria: PackratParser[Token] = nestedCriteria | childCriteria | parentCriteria | criteria - def whereCriteria: PackratParser[List[Token]] = rep1( - allPredicate | allCriteria | start | or | and | end | then_case - ) + /** The token stream of a WHERE / HAVING / CASE-WHEN / JOIN-ON condition. + * + * Story 22.1 (AD-2b, specified with story 22.2): same alternatives as the `rep1` this replaces, + * ONE rule added โ€” a `)` is consumed only while an unmatched `(` is open in THIS clause. At + * depth 0 the scan stops and leaves the `)` to whoever opened it. + * + * ๐Ÿ”ด Why it had to change. `rep1` offers a bare `end` and never backtracks, so for every + * PARENTHESISED body whose last clause is a WHERE or a HAVING โ€” story 22.1's `FROM (SELECT a + * FROM t WHERE x = 1) d`, story 22.2's `IN (SELECT id FROM c WHERE r = 'EU')` โ€” the inner clause + * swallowed the subquery's own `)`, `processTokensHelper`'s top-level `EndDelimiter` arm + * answered `Left("Unbalanced parentheses")` and `where` raised it as a NON-backtracking `err` + * that killed the whole statement. It is the mechanism story 21.4 met inside relation predicates + * and fixed by giving them `relationTokens`, which omits `end` (`:264-293`) โ€” the same defect, + * one construct up. + * + * Depth is counted on `StartPredicate` / `EndPredicate` ONLY, because those are the only + * delimiters this alternation can emit: `start` produces `StartPredicate`, `end` produces + * `EndPredicate`, and `then_case` produces `ThenCase` โ€” which IS an `EndDelimiter` but must keep + * being consumed, since `processTokensHelper` reads it as end-of-tokens for a CASE-WHEN + * condition. A parenthesis that an ITEM consumes (a function call, a relation predicate's own + * group, `IN (1, 2)`) never reaches this counter: items are consumed atomically. + * + * What moves, and it is pinned: an unmatched OPENING paren is still consumed and still reaches + * `processTokens`, so `WHERE (b = 1` keeps its `"Unbalanced parentheses"` rejection. A STRAY `)` + * at depth 0 is no longer eaten โ€” the clause ends before it and `phrase` rejects the statement + * as trailing input, so `SELECT a FROM t WHERE a = 1)` stays a rejection but changes its wording + * (`ParserTotalitySpec` retargets those three contract pins). + * + * Written as a plain `Parser[List[Token]]` over the existing item parser: PackratParser memoises + * the items exactly as before, and the depth is a local of one scan, never parser state. + */ + def whereCriteria: PackratParser[List[Token]] = new self.Parser[List[Token]] { + + // The SAME alternation, in the SAME order, as the `rep1` this replaces. + private val item: self.Parser[Token] = + allPredicate | allCriteria | start | or | and | end | then_case + + @scala.annotation.tailrec + private def scan(rest: Input, depth: Int, acc: List[Token]): ParseResult[List[Token]] = + item(rest) match { + case Success(EndPredicate, _) if depth == 0 => + if (acc.isEmpty) Failure("criteria expected", rest) else Success(acc.reverse, rest) + case Success(EndPredicate, next) => scan(next, depth - 1, EndPredicate :: acc) + case Success(StartPredicate, next) => scan(next, depth + 1, StartPredicate :: acc) + case Success(t, next) => scan(next, depth, t :: acc) + // An item's own `err` (a relation predicate's, #250 / story 21.4) propagates unchanged. + case e: Error => e + case f: Failure => + if (acc.isEmpty) f else Success(acc.reverse, rest) + } + + override def apply(in: Input): ParseResult[List[Token]] = scan(in, 0, Nil) + } def where: PackratParser[Where] = Where.regex ~ whereCriteria >> { case _ ~ rawTokens => @@ -386,8 +440,10 @@ trait WhereParser { // A dangling `AND` / `OR` used to leave `Where(None)`, which renders as NO CLAUSE AT ALL: // `DELETE FROM orders WHERE id = 1 AND` parsed as `DELETE FROM orders` and emptied the // index (the #213 data-loss family, measured 2026-09-04). `where` runs only once the - // literal WHERE has matched and `whereCriteria` is `rep1`, so `None` here always means - // "a WHERE was written and nothing usable came of it". + // literal WHERE has matched, and `whereCriteria` yields at least one token or FAILS (story + // 22.1's scanner returns the item's own `Failure` when its accumulator is empty, exactly as + // `rep1` did), so `None` here always means "a WHERE was written and nothing usable came of + // it". case Right(None) => err("WHERE clause requires criteria") case Left(reason) => err(reason) } @@ -532,9 +588,10 @@ trait WhereParser { case unexpected :: _ => // #250 - this arm used to be `processTokensHelper(Nil, stack)`, which ABANDONED every // remaining token and returned whatever the stack happened to hold: a silent truncation of - // the clause the user wrote. It is believed unreachable - `whereCriteria` is - // `rep1(allPredicate | allCriteria | start | or | and | end | then_case)` and every one of - // those token kinds is matched by an arm above - and it was NEVER reached while + // the clause the user wrote. It is believed unreachable - `whereCriteria` scans + // `allPredicate | allCriteria | start | or | and | end | then_case` (story 22.1 added a + // depth rule, not a token kind) and every one of those token kinds is matched by an arm + // above - and it was NEVER reached while // instrumented across the sql, core, bridge and macros-tests suites (2026-09-05). That is // exactly why it must not silently truncate: an unreachable arm that loses data is one // grammar change away from being reachable. Same reasoning as the defensive arm in diff --git a/sql/src/main/scala/app/softnetwork/elastic/sql/query/From.scala b/sql/src/main/scala/app/softnetwork/elastic/sql/query/From.scala index 77be732b0..00cef988a 100644 --- a/sql/src/main/scala/app/softnetwork/elastic/sql/query/From.scala +++ b/sql/src/main/scala/app/softnetwork/elastic/sql/query/From.scala @@ -257,9 +257,15 @@ case class StandardJoin( * dot-separated part (story 21.2 AD-5). A JOIN source is a TABLE name, whose dots are literal, * so it takes `Table.render` โ€” the same whole-lexeme rule the FROM side uses. */ - override def sql: String = - s" ${asString(joinType)} $Join ${Table.render(parts, source.name)}" + - s"${asString(alias)}${asString(on)}" + override def sql: String = { + // Story 22.1 โ€” a DERIVED source carries its OWN alias and its own parenthesised render + // (`Table.render` would emit the bare correlation name and drop the whole subquery). + val ref = source match { + case d: DerivedTable => d.sql + case _ => s"${Table.render(parts, source.name)}${asString(alias)}" + } + s" ${asString(joinType)} $Join $ref${asString(on)}" + } override def update(request: SingleSearch): StandardJoin = { // The source of a JOIN is a TABLE name, not a column expression โ€” `Identifier.update` @@ -280,6 +286,17 @@ case class StandardJoin( */ override def validate(): Either[String, Unit] = { for { + // Story 22.1 AD-1, ENFORCED rather than narrated: a derived JOIN source OWNS its alias, so a + // programmatic `StandardJoin(derived, โ€ฆ, alias = Some(y))` would bind `y` in `joinReferences` + // while `sql` renders the derived table's own alias โ€” one reference, two names. + _ <- source match { + case d: DerivedTable if alias.nonEmpty || parts.nonEmpty => + Left( + s"A derived table owns its alias: the JOIN of '${d.name}' must carry no separate " + + "alias and no qualifier parts" + ) + case _ => Right(()) + } _ <- on match { case Some(o) => o.validate() case None if joinType.contains(CrossJoin) => Right(()) // CROSS JOIN needs no ON clause @@ -290,6 +307,90 @@ case class StandardJoin( } } +/** A derived table โ€” a parenthesised query used as a table reference (SQL-92 ยง7.6, ` + * ::= `, always followed by a ``). Story 22.1. + * + * `name` IS the correlation name. That is not a shortcut: in the OUTER scope a derived table is + * known ONLY by its alias, so `From.tableAliases`' `table -> alias` binding collapses to `alias -> + * alias`, `Identifier.table` becomes the alias, and every consumer that keys on a source name + * (`aliasKey`, `joinSourceKeys`, `aliasesToTable`, `Identifier.update`'s resolution, + * `TemporalLiterals`' join-leg guard) works with no new arm. A synthetic or empty name was + * rejected: an empty `Identifier.name` already means "a literal" in this AST, and a synthesised + * alias would make the rendered text differ from the input. + * + * `query` is a `DqlStatement` rather than a `SearchStatement` because the body may be a FROM-less + * SELECT (`FromlessSelect` is a `DqlStatement`, #251) โ€” Tableau's connection probe is exactly + * `FROM (SELECT 1 AS COL) AS SUBQUERY`. The grammar produces only `SearchStatement | + * FromlessSelect`; `validate()` rejects any other kind a programmatic construction could inject. + * + * ๐Ÿ”ด `SingleSearch.sources` yields this ALIAS for a derived table. It is NOT an index name, and + * nothing in core may hand it to Elasticsearch: `SearchApi.resolveWithSchema` refuses a + * closure-shaped statement before any router sees it, and `CoreDqlExtension` refuses it at the + * gateway. A consumer OUTSIDE core that reads `sources` / `from.tables.head.name` as an index โ€” + * arrow's `JoinPlanner` does today โ€” is the story-22.4 hand-off, guarded meanwhile by the loud arm + * this story adds there. + */ +case class DerivedTable(query: DqlStatement, alias: Alias) extends Source { + + override val name: String = alias.alias + + /** `() AS ` โ€” the body through its OWN `.sql`, so nesting, `UNION ALL` and the + * FROM-less form all round-trip by construction. `Alias.sql` already carries the leading ` AS ` + * and re-quotes a quoted alias with the canonical double quote (story 21.1 AD-1). + */ + override def sql: String = s"(${query.sql})$alias" + + /** The body is its OWN scope. Nothing in it is resolved against the enclosing statement here: + * that would be SQL:1999 `LATERAL`, which `SingleSearch.validate()` rejects by name. The inner + * statement was already `.update()`-d by `Parser.single` when it was parsed. + */ + def update(request: SingleSearch): DerivedTable = this + + /** The output column names an OUTER reference may name โ€” `Field.outputName` per SELECT item (the + * alias when there is one, else the source field), i.e. the SAME derivation `SearchApi` uses for + * a result row. + * + * `None` means the projection cannot be enumerated and the derived table is OPAQUE: a bare + * `SELECT *` (no schema is attached at parse time and this story never guesses a field), or a + * body kind the grammar cannot build. A `UNION ALL` body takes the FIRST branch's names, which + * is what SQL specifies for a set operation's column names. + */ + lazy val outputNames: Option[Seq[String]] = query match { + case s: SingleSearch => DerivedTable.projected(s) + case m: MultiSearch => m.requests.headOption.flatMap(DerivedTable.projected) + case f: FromlessSelect => Some(f.columnNames) + case _ => None + } + + override def validate(): Either[String, Unit] = + for { + _ <- + if (alias.alias.isEmpty) Left("A derived table requires a non-empty alias") + else Right(()) + // `Parser.single` runs `.update()` inside its action but `Parser.apply` validates the + // TOP-LEVEL statement only, so an inner GROUP BY / HAVING / ORDER BY rule would be silently + // skipped one nesting level down โ€” the #253 family, one level in. + _ <- query match { + case s: SearchStatement => s.validate() + case f: FromlessSelect => f.validate() + case other => + Left(s"A derived table body must be a SELECT, got ${other.getClass.getSimpleName}") + } + } yield () +} + +object DerivedTable { + + /** A bare `SELECT *` projects an un-enumerable list; anything else projects its items' output + * names. The `*` test is `identifierName` with no functions โ€” the same spelling + * `SearchApi.extractOutputFieldNames` uses. + */ + private[query] def projected(s: SingleSearch): Option[Seq[String]] = + if (s.select.fields.exists(f => f.identifier.name == "*" && f.identifier.functions.isEmpty)) + None + else Some(s.select.fields.map(_.outputName)) +} + object Table { /** The full dotted reference, un-quoted, qualifier parts included. Equals `name` whenever the @@ -328,7 +429,12 @@ case class Table( * an error: after story 20.3 the JDBC driver advertises the cluster name as the schema, so a BI * tool's qualifier is routinely a name no registry knows. */ - parts: Seq[NamePart] = Nil + parts: Seq[NamePart] = Nil, + /** Story 22.1 โ€” set when this FROM item is `(SELECT โ€ฆ) [AS] alias`. Then `name == derived.name` + * (the alias), `tableAlias` is `None` and `parts` is `Nil`: the alias has ONE owner, not two + * (story 21.3's one-key-two-derivations lesson). The invariant is ENFORCED in `validate()`. + */ + derived: Option[DerivedTable] = None ) extends Source { /** The full dotted reference (qualifier parts included), un-quoted โ€” the alias-map key when a @@ -336,13 +442,30 @@ case class Table( */ lazy val qualifiedName: String = Table.qualifiedName(parts, name) - override def sql: String = - s"${Table.render(parts, name)}${asString(tableAlias)} ${joins.map(_.sql).mkString(" ")}".trim + override def sql: String = { + val ref = derived match { + case Some(d) => d.sql // carries its own parentheses and its own alias + case None => s"${Table.render(parts, name)}${asString(tableAlias)}" + } + s"$ref ${joins.map(_.sql).mkString(" ")}".trim + } def update(request: SingleSearch): Table = this.copy(joins = joins.map(_.update(request))) override def validate(): Either[String, Unit] = for { + // Story 22.1 AD-1, ENFORCED rather than narrated: a programmatic + // `Table("x", Some(y), derived = Some(dt))` would bind `x -> y` in `tableAliases` while the + // render drops `y` entirely. + _ <- derived match { + case Some(d) if tableAlias.nonEmpty || parts.nonEmpty || name != d.name => + Left( + s"A derived table owns its alias: Table.name must be '${d.name}', tableAlias None, " + + "parts Nil" + ) + case Some(d) => d.validate() + case None => Right(()) + } _ <- tableAlias match { case Some(a) if a.alias.isEmpty => Left(s"Table $name alias cannot be empty") case _ => Right(()) @@ -532,11 +655,73 @@ case class From(tables: Seq[Table]) extends Updateable { def update(request: SingleSearch): From = this.copy(tables = tables.map(_.update(request))) + /** Story 22.1 โ€” a DERIVED table's correlation name must name nothing else in the same FROM. + * + * ๐Ÿ”ด Built over the SEQUENCE of addressable names, never over `tableAliases`: that `ListMap` is + * keyed by TABLE and has ALREADY collapsed a colliding key by the time anyone reads it (story + * 21.2 AD-6), so a check written over the map is blind to the very case it exists to catch. + * MEASURED: `FROM bi_events b JOIN (SELECT category FROM bi_events) bi_events` binds `bi_events + * -> b` from the table and `bi_events -> bi_events` from the join, the `++` overwrites, alias + * `b` is silently GONE, and `b.amount` then resolves against the DERIVED table. Tableau aliases + * a derived table with the inner table's OWN name, so this is the default spelling, not a corner + * case. + * + * A plain source contributes BOTH the KEY its index is filed under (`aliasKey`) and the alias + * somebody wrote, because `Identifier.update` resolves a qualifier through `aliasesToTable` and + * lands on the key: two sources sharing a key are indistinguishable downstream whatever they are + * called. That is why this is not simply folded into the explicit-alias check above, which + * compares written aliases only. + * + * SCOPE, deliberately narrow (epic 22 OQ-4, lead ruling NARROW): only a collision INVOLVING a + * derived table is rejected. `FROM orders JOIN orders` collapses today and PARSES; making the + * rule uniform is a breaking change on existing input that belongs to whoever schedules one. + */ + private lazy val derivedNameCollision: Either[String, Unit] = { + val derivedSeq: Seq[String] = + tables.flatMap(_.derived.map(_.name)) ++ joins.collect { + case sj: StandardJoin if sj.source.isInstanceOf[DerivedTable] => sj.source.name + } + if (derivedSeq.isEmpty) Right(()) + else { + val plainNames: Seq[String] = + tables + .filter(_.derived.isEmpty) + .flatMap(t => + Seq(aliasKey(t.name, t.qualifiedName)) ++ t.tableAlias.map(_.alias).filter(_.nonEmpty) + ) ++ + joins + .collect { case sj: StandardJoin if !sj.source.isInstanceOf[DerivedTable] => sj } + .flatMap(sj => + Seq(aliasKey(sj.source.name, sj.qualifiedName)) ++ + sj.alias.map(_.alias).filter(_.nonEmpty) + ) ++ + unnestAliases.keys + derivedSeq + .diff(derivedSeq.distinct) + .headOption + .orElse(derivedSeq.find(plainNames.contains)) match { + case Some(n) => + Left( + s"Alias '$n' is used by more than one source in FROM: a derived table's correlation " + + "name must name nothing else in the same FROM. Rename the subquery's alias." + ) + case None => Right(()) + } + } + } + override def validate(): Either[String, Unit] = { if (tables.isEmpty) { Left("At least one table is required in FROM clause") } else if (tables.count(_.joins.nonEmpty) > 1) { Left("Only one table with joins is supported in FROM clause") + } else if (derivedNameCollision.isLeft) { + // Story 22.1 โ€” FIRST, ahead of the BIDC-8 self-join arm below. `FROM t x, (SELECT a FROM u) t` + // has two `Table`s whose `qualifiedName` is `t` under DIFFERENT effective aliases (`x`, `t`), + // so that arm would fire and advise *"Write a self-join as FROM t x JOIN t t ON โ€ฆ"* โ€” advice + // that makes no sense for a subquery, and a DIFFERENT message from the one the alias-less + // spelling of the same mistake gets. + derivedNameCollision } else { // ๐Ÿ”ด Story BIDC-8, tripwire 2 (lead ruling: keep the behaviour or reject LOUDLY, never // change it silently). A comma-separated FROM is a MULTI-INDEX SEARCH โ€” one query over every @@ -575,6 +760,7 @@ case class From(tables: Seq[Table]) extends Updateable { // keep their multi-index-search acceptance (21.2 preserves, it does not interpret), and // the alias-less `FROM t JOIN t` is left to the join planner's own guard. The comparison // is case-INSENSITIVE (review NEW-3): DuckDB's catalog folds `sq_A` and `sq_a`. + // val explicitAliases: Seq[(String, String)] = tables.flatMap(t => t.tableAlias.map(_.alias).filter(_.nonEmpty).map(_ -> t.qualifiedName) @@ -609,6 +795,32 @@ case class From(tables: Seq[Table]) extends Updateable { lazy val joinedTables: Seq[String] = tables.flatMap(_.joinedTables) lazy val enrichmentRequired: Boolean = joinedTables.nonEmpty + + /** correlation name -> derived table, for every FROM item and every standard JOIN leg that is a + * subquery (story 22.1). The key IS `Source.name` for a `DerivedTable`, so it is the same + * language as `tableAliases`' keys and as `Identifier.table`. + */ + lazy val derivedTables: ListMap[String, DerivedTable] = ListMap( + (tables.flatMap(t => t.derived.map(d => d.name -> d)) ++ + joins + .collect { case sj: StandardJoin => + sj.source + } + .collect { case d: DerivedTable => d.name -> d }): _* + ) + + lazy val hasDerivedTables: Boolean = derivedTables.nonEmpty + + /** Epic 22 AD-4 โ€” THE predicate every venue routes on: this FROM needs the relational engine + * (DuckDB, shipped in `softclient4es-arrow-extensions`) because it carries a cross-index JOIN + * leg (`enrichmentRequired`, meaning UNCHANGED) or a derived table. + * + * Read it through `SingleSearch.relationalClosureRequired` or the package-level + * `relationalClosureRequired(statement)` rather than directly: later stories widen the + * STATEMENT-level value with constructs a `From` cannot see (22.3's correlated subqueries, + * 22.5's CTEs), and a consumer reading `from.` would silently miss them. + */ + lazy val relationalClosureRequired: Boolean = enrichmentRequired || hasDerivedTables } case class NestedElement( 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 new file mode 100644 index 000000000..73c3a3b1a --- /dev/null +++ b/sql/src/main/scala/app/softnetwork/elastic/sql/query/SubqueryScope.scala @@ -0,0 +1,125 @@ +/* + * Copyright 2025 SOFTNETWORK + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package app.softnetwork.elastic.sql.query + +import app.softnetwork.elastic.sql.Identifier + +/** Does a nested statement read a correlation name of an ENCLOSING statement? + * + * ONE detector, structural, on the AST the parser already built (story 22.1, consumed by stories + * 22.2 and 22.3 โ€” a second walk is the defect class this object exists to prevent). + * + * `GenericIdentifier.update` resolves a qualifier against the statement's OWN `aliasesToTable` + * (FROM aliases, index names and UNNEST aliases); a qualifier that is NOT one of them is LEFT IN + * THE NAME โ€” `o.cid` keeps `name = "o.cid"`, `tableAlias = None`, `table = None`. That signature + * IS the annotation: Elasticsearch would read such a name as an object path and match nothing, + * with HTTP 200. So after the inner `update()`: + * - qualified-and-resolved (`tableAlias = Some(q)`): inner-scoped, never correlated; + * - qualified-and-unresolved: correlated iff the HEAD segment is a correlation name of an + * enclosing statement โ€” innermost wins, a name the inner statement also declares shadows the + * outer one (SQL scoping); + * - bare (`cid`): NOT decided here. SQL resolves a bare name innermost-first, so it is assumed + * to be the inner column; story 22.2's seam re-checks it against both mappings. + * + * ๐Ÿ”ด A dotted path into an OBJECT field whose head happens to equal an enclosing alias is reported + * as correlated. That is loud and the message says how to disambiguate โ€” strictly better than the + * alternative, which is reading an enclosing alias as an object path and answering zero rows with + * HTTP 200. + */ +object SubqueryScope { + + /** Every name a statement's FROM makes addressable: the alias-map keys AND values (index names, + * aliases, qualified keys) plus the UNNEST aliases. + */ + def correlationNames(s: SingleSearch): Set[String] = + (s.from.tableAliases.keys ++ s.from.tableAliases.values ++ s.from.unnestAliases.keys).toSet + + def correlatedReferences(body: DqlStatement, outer: SingleSearch): Seq[Identifier] = + correlatedReferences(body, correlationNames(outer)) + + /** The Set-taking overload story 22.2 keeps (one detector, never two): the enclosing scopes are + * ACCUMULATED as the walk descends, so a reference from two levels in to the OUTERMOST alias is + * caught at the outermost statement too. + */ + private[query] def correlatedReferences( + body: DqlStatement, + outerScopes: Set[String] + ): Seq[Identifier] = + body match { + case inner: SingleSearch => + val innerNames = correlationNames(inner) + val outerOnly = outerScopes -- innerNames // innermost wins + val direct = + if (outerOnly.isEmpty) Nil + else + inner.referencedIdentifiers.filter { id => + id.tableAlias.isEmpty && !id.nested && id.name.contains(".") && + outerOnly.contains(id.name.split("\\.", 2)(0)) + } + // A derived table NESTED in this body is walked with this statement's names added. + direct ++ inner.from.derivedTables.values.toSeq.flatMap(d => + correlatedReferences(d.query, outerScopes ++ innerNames) + ) + case multi: MultiSearch => multi.requests.flatMap(r => correlatedReferences(r, outerScopes)) + case _ => Nil // a FROM-less body names no source and can reference nothing + } + + /** LATERAL-shaped references: a DERIVED body (FROM or JOIN position) naming an ENCLOSING + * correlation name. + * + * SQL-92 ยง7.6 gives a derived table NO access to the enclosing scope; only SQL:1999 `LATERAL` + * does, and `lateral` is not a word this dialect knows. Without this arm the shape PARSES and + * the relational engine (story 22.4) would execute the body as written โ€” `WHERE cid = c.id` + * reaching Elasticsearch as an object path, zero rows, HTTP 200 โ€” which is exactly the + * silent-wrong-answer mode epic 22 exists to close. + * + * Story 22.2/22.3 widen the walk to bodies nested inside WHERE subqueries; the shape of the + * recursion is already here. + */ + def lateralReferences(s: SingleSearch, outerScopes: Set[String] = Set.empty): Seq[Identifier] = + lateralOffenders(s, outerScopes).map(_._2) + + /** The same walk, keeping the correlation name of the derived table whose body names each + * offending identifier. + * + * ๐Ÿ”ด The pairing is not cosmetic. `correlatedReferences` recurses into derived tables NESTED in + * a body, so a reference two levels in belongs to the INNER derived table, not to the outer one + * a `find` over the top-level map would return โ€” and the alias is the whole point of the + * message. Returning the pair also removes the `getOrElse("")` that could render `derived table + * ''`. + */ + def lateralOffenders( + s: SingleSearch, + outerScopes: Set[String] = Set.empty + ): Seq[(String, Identifier)] = { + val here = outerScopes ++ correlationNames(s) + s.from.derivedTables.values.toSeq.flatMap { d => + val inner: Seq[(String, Identifier)] = d.query match { + // attribute a nested body's references to the NESTED derived table + case body: SingleSearch => lateralOffenders(body, here) + case _ => Nil + } + val innerIds = inner.map(_._2).toSet + correlatedReferences(d.query, here).filterNot(innerIds.contains).map(d.name -> _) ++ inner + } + } + + 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." +} 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 2a4d3c7ea..ee1fa4956 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 @@ -55,6 +55,37 @@ import java.time.{Duration, Instant} import scala.collection.immutable.ListMap package object query { + + /** The `SingleSearch`es a statement EMBEDS โ€” the ONE place these arms are spelled. + * + * Every closure-shaped question ("does this statement need the relational engine?", "does it + * carry a derived table?") is a fold over this, never a second `match`: a second list of arms is + * the desync class story 21.3 paid for. `Delete`, `CreateMaterializedView` and the watcher are + * deliberately NOT arms โ€” they reject closure shapes in the grammar or in `validate()`, so they + * never reach a router carrying one. + */ + def closureSearches(statement: Statement): Seq[SingleSearch] = statement match { + case s: SingleSearch => Seq(s) + case m: MultiSearch => m.requests + case sel: SelectStatement => sel.statement.toSeq.flatMap(closureSearches) + case ins: Insert => ins.values.left.toOption.toSeq.flatMap(closureSearches) + case ct: CreateTable => ct.ddl.left.toOption.toSeq.flatMap(closureSearches) + case _ => Nil + } + + /** Epic 22 AD-4 โ€” the statement needs the relational engine (a cross-index JOIN or a derived + * table today; stories 22.3/22.5/22.6 widen the per-statement value this reads). + */ + def relationalClosureRequired(statement: Statement): Boolean = + closureSearches(statement).exists(_.relationalClosureRequired) + + /** Narrower than [[relationalClosureRequired]]: a derived table specifically. Read by the + * rejection message (the remedy differs โ€” a JOIN can be rewritten by hand, a derived table is + * usually emitted by a BI tool the user does not control) and by the MATERIALIZED VIEW guard. + */ + def derivedTablesPresent(statement: Statement): Boolean = + closureSearches(statement).exists(_.hasDerivedTables) + sealed trait Statement extends Token sealed trait DqlStatement extends Statement @@ -119,6 +150,40 @@ package object query { lazy val fieldAliases: ListMap[String, String] = select.fieldAliases lazy val tableAliases: ListMap[String, String] = from.tableAliases + /** correlation name -> derived table (story 22.1). */ + lazy val derivedTables: ListMap[String, DerivedTable] = from.derivedTables + + /** Read THIS, not `from.hasDerivedTables`, for the same reason as + * [[relationalClosureRequired]]: story 22.5 makes a CTE reference a derived table that a + * `From` alone cannot see. + */ + 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. + */ + lazy val relationalClosureRequired: Boolean = from.relationalClosureRequired + + /** 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. + * + * ONE list, several consumers (`derivedScopeCheck` here, `SubqueryScope`'s correlation walk, + * story 22.2's subquery detector): a second enumeration of the clauses is the + * one-key-two-derivations drift story 21.3 paid for four times. + */ + lazy val referencedIdentifiers: Seq[Identifier] = + select.fields.flatMap(f => FunctionUtils.funIdentifiers(f.identifier)) ++ + where.flatMap(_.criteria).map(_.referencedIdentifiers).getOrElse(Nil) ++ + having.flatMap(_.criteria).map(_.referencedIdentifiers).getOrElse(Nil) ++ + groupBy.map(_.buckets.map(_.identifier)).getOrElse(Nil) ++ + orderBy.map(_.sorts.map(_.field)).getOrElse(Nil) ++ + from.joins + .collect { case sj: StandardJoin => sj } + .flatMap(_.on.toSeq) + .flatMap(_.criteria.referencedIdentifiers) + /** alias -> table KEY, lossless (story BIDC-8): the map to consult when resolving a qualifier. * `tableAliases` (table -> alias) cannot hold two aliases of one table. */ @@ -401,6 +466,15 @@ package object query { * The guard sits on the aggregation arm ONLY because `windowRowQuery` already carries * `groupBy.isEmpty`, so hoisting it out would be equivalent today and would make correctness * depend on that internal. Do not "simplify" it. + * + * ๐Ÿ”ด Story 22.1 โ€” for a statement whose FROM is [[relationalClosureRequired]] (a cross-index + * JOIN or a derived table) this value describes the OUTER shape only: what the relational + * engine's result will look like once story 22.4 executes it. It is NOT a routing licence. No + * core router may act on it for such a statement, because every one of them runs BEHIND a + * guard that fires first โ€” `SearchApi.resolveWithSchema` precedes the `this match` on both the + * sync and async search paths, and `CoreDqlExtension`'s closure arm precedes + * `checkQuotasAndExecute`. `RelationalClosureGuardSpec` proves it rather than asserting it. + * The EXPRESSION is deliberately untouched. */ lazy val returnsRows: Boolean = windowRowQuery || (sqlAggregations.isEmpty && groupBy.isEmpty) @@ -498,9 +572,101 @@ package object query { lazy val buckets: Seq[Bucket] = bucketTree.allBuckets.flatten + /** Story 22.1 AD-4 โ€” an OUTER reference into a derived table must name a column the derived + * table PROJECTS (its `outputNames`: the SELECT alias when there is one, else the bare column, + * so `SELECT amount AS total` exposes `total`, NOT `amount`). + * + * Checked HERE, not in `Identifier.update`: `update` runs inside `Parser.single`'s combinator + * action and cannot report. That is the house pattern โ€” record in `update`, format in + * `validate` โ€” and this arm needs no recording, because everything it reads survives on the + * AST. + * + * Scope rules, deliberately narrow (epic 22: never guess a field): + * - a QUALIFIED reference (`d.x`) is checked against the derived table `d` names โ€” + * `Identifier.table` is the alias-map key and for a derived table that key IS the alias; + * - an UN-QUALIFIED reference is checked ONLY when the FROM has exactly ONE source and it is + * a derived table (Tableau's `SELECT COL FROM (SELECT 1 AS COL) AS SUBQUERY`). With + * several sources a bare name is ambiguous today for plain tables too, and resolving it is + * story 22.3's scope model, not this story's; + * - an outer SELECT alias (`SELECT COL AS c โ€ฆ ORDER BY c`), an ordinal or literal (empty + * `name`), `*` and `COUNT(*)` are never derived-table references; + * - a derived table whose projection is OPAQUE (`outputNames == None`, i.e. a bare `SELECT + * *`) accepts EVERY reference: rejecting one would mean inventing the schema, and DuckDB's + * binder rejects a wrong one loudly once story 22.4 executes it. + * + * A dotted remainder (`d.items.name`) is struct/nested access INTO a projected column, so the + * HEAD segment is what is compared, never the whole path. + */ + private lazy val derivedScopeCheck: Either[String, Unit] = { + 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 + } + // `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 != "*") + .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 head = id.name.split("\\.", 2)(0) + scope.flatMap(d => + d.outputNames.filterNot(_.contains(head)).map(names => (id, d, names)) + ) + } + .toSeq + .headOption match { + case Some((id, d, names)) => + Left( + s"Column '${id.name}' is not projected by derived table '${d.name}' " + + s"(it projects: ${names.mkString(", ")})" + ) + case None => Right(()) + } + } + } + + /** Story 22.1 (amendment from story 22.3's review) โ€” a derived body that reads an ENCLOSING + * correlation name is SQL:1999 `LATERAL`, which this dialect does not support. Without this + * arm the shape parses `Right` and story 22.4 would execute the body as written: the outer + * qualifier reaches Elasticsearch as an object path and the statement answers ZERO rows with + * HTTP 200. The detector is `SubqueryScope`, shared with stories 22.2 / 22.3. + */ + private lazy val lateralCheck: Either[String, Unit] = + if (!from.hasDerivedTables) Right(()) + else + SubqueryScope.lateralOffenders(this).headOption match { + // The offender is carried WITH the derived table whose body names it, so the message can + // never name the wrong alias (or, worse, an empty one) for a reference two levels in. + case Some((alias, id)) => Left(SubqueryScope.lateralMessage(id, alias)) + case None => Right(()) + } + override def validate(): Either[String, Unit] = { for { _ <- from.validate() + // AFTER `from.validate()` so a derived table's OWN body is validated first, and BEFORE + // every clause rule so the scope message wins over a downstream symptom. + _ <- derivedScopeCheck + _ <- lateralCheck _ <- select.validate() _ <- where.map(_.validate()).getOrElse(Right(())) _ <- groupBy.map(_.validate()).getOrElse(Right(())) @@ -1120,7 +1286,19 @@ package object query { s"DELETE FROM ${Table.render(table.parts, table.name)}${asString(where)}" // `DELETE FROM t WHERE COUNT(x) > 5` used to become `match_all` and WIPE the index (S2-2). - override def validate(): Either[String, Unit] = where.map(_.validate()).getOrElse(Right(())) + override def validate(): Either[String, Unit] = + // Story 22.1 โ€” the grammar already rejects `DELETE FROM (SELECT โ€ฆ) d`, but that is a PARSER + // guard: `GatewayApi.run(statement: Statement)` accepts a programmatically built `Delete`, + // and `Table.validate()`'s AD-1 arm is SATISFIED by a well-formed derived table. Without + // this the delete-by-query would target an index named after the subquery's alias โ€” or, if + // one of that name happens to exist, the wrong index entirely. Every other AD-1 invariant in + // this story is enforced in `validate()`; this is the same belt for the DML side. + if (table.derived.isDefined) + Left( + "DELETE cannot target a derived table (subquery in FROM): Elasticsearch deletes by " + + "query over an index." + ) + else where.map(_.validate()).getOrElse(Right(())) } sealed trait FileFormat extends Token { @@ -1312,8 +1490,23 @@ package object query { /** Same reasoning as `CreateTable.validate()` (story BIDC-8): the view's query is validated by * the rules that govern any SELECT โ€” this statement used to inherit the no-op default. + * + * Story 22.1 adds the derived-table refusal, and it is ordered FIRST so the DERIVED message + * wins whenever both apply. A materialized view is an Elasticsearch TRANSFORM, and the + * extension that plans it (`MaterializedViewExtension`, softclient4es-extensions) keys on + * `from.tables` by INDEX NAME: a derived table's name is its ALIAS, so the transform would be + * planned over an index that does not exist โ€” silently (the extensions#45/#46 class). A + * cross-index JOIN in a materialized view IS supported by that extension, so the guard is on + * the derived half only. */ - override def validate(): Either[String, Unit] = dql.validate() + override def validate(): Either[String, Unit] = + if (derivedTablesPresent(dql)) + Left( + "MATERIALIZED VIEW over a derived table (subquery in FROM/JOIN) is not supported: an " + + "Elasticsearch transform reads indices. Materialize the subquery as its own view and " + + "reference it." + ) + else dql.validate() override def sql: String = { // The leading space belongs HERE, not to `Frequency.sql`: `TransformConfig` renders the same diff --git a/sql/src/test/resources/corpus/epic-21-attribution.csv b/sql/src/test/resources/corpus/epic-21-attribution.csv index 7e0f0fbb8..d5163572a 100644 --- a/sql/src/test/resources/corpus/epic-21-attribution.csv +++ b/sql/src/test/resources/corpus/epic-21-attribution.csv @@ -3,15 +3,15 @@ "superset.flightsql.w2.002","parses","pre_epic21","pre_epic21","parsed before Epic 21 (re-measured at ac54a079, Task 0) -- Epic 21 may not claim it" "superset.flightsql.w3.003","parses","pre_epic21","pre_epic21","parsed before Epic 21 (re-measured at ac54a079, Task 0) -- Epic 21 may not claim it" "superset.flightsql.w4.004","parses","pre_epic21","pre_epic21","parsed before Epic 21 (re-measured at ac54a079, Task 0) -- Epic 21 may not claim it" -"superset.flightsql.w5.005","rejected","residual","epic22a_derived_table","a derived table (a SELECT in FROM or JOIN position); Epic 22 owns relational closure." +"superset.flightsql.w5.005","parses","residual","epic22a_derived_table","a derived table (a SELECT in FROM or JOIN position); Epic 22 owns relational closure. Story 22.1 made it PARSE (2026-09-14); it still scores residual because Epic 21 did not fix it, and it EXECUTES only through the relational engine." "superset.flightsql.w6.006","rejected","residual","epic22b_cte","a WITH ... AS (...) common table expression; Epic 22 owns CTEs. authorship=analyst by design -- never cite it as a shape Superset emits (19.4 G9)" -"superset.flightsql.w7.007","rejected","residual","epic22a_derived_table","a derived table (a SELECT in FROM or JOIN position); Epic 22 owns relational closure." +"superset.flightsql.w7.007","parses","residual","epic22a_derived_table","a derived table (a SELECT in FROM or JOIN position); Epic 22 owns relational closure. Story 22.1 made it PARSE (2026-09-14); it still scores residual because Epic 21 did not fix it, and it EXECUTES only through the relational engine." "superset.flightsql.w8.008","parses","pre_epic21","pre_epic21","parsed before Epic 21 (re-measured at ac54a079, Task 0) -- Epic 21 may not claim it" "superset.flightsql.w1.009","parses","pre_epic21","pre_epic21","parsed before Epic 21 (re-measured at ac54a079, Task 0) -- Epic 21 may not claim it" "superset.flightsql.w1.010","parses","pre_epic21","pre_epic21","parsed before Epic 21 (re-measured at ac54a079, Task 0) -- Epic 21 may not claim it" "tableau.mysql.wx.001","parses","capability_open","capability_open","Tableau temp-table capability probe (CREATE TABLE, backticked name); parses since story 21.7's uniform Parser.ident quoting -- excluded from scoring, the capability answer is an open product decision tracked outside Epic 21" "tableau.mysql.wx.002","parses","capability_open","capability_open","Tableau temp-table capability probe (DROP TABLE, quoted name); parses since story 21.7's uniform Parser.ident quoting -- excluded from scoring, the capability answer is an open product decision tracked outside Epic 21" -"tableau.mysql.wx.003","rejected","residual","epic22a_derived_table","a derived table (a SELECT in FROM or JOIN position); Epic 22 owns relational closure." +"tableau.mysql.wx.003","parses","residual","epic22a_derived_table","a derived table (a SELECT in FROM or JOIN position); Epic 22 owns relational closure. Story 22.1 made it PARSE (2026-09-14); it still scores residual because Epic 21 did not fix it, and it EXECUTES only through the relational engine." "tableau.mysql.wx.004","parses","pre_epic21","pre_epic21","parsed before Epic 21 (re-measured at ac54a079, Task 0) -- Epic 21 may not claim it" "tableau.mysql.wx.005","parses","capability_open","capability_open","Tableau temp-table capability probe (CREATE TABLE, backticked name); parses since story 21.7's uniform Parser.ident quoting -- excluded from scoring, the capability answer is an open product decision tracked outside Epic 21" "tableau.mysql.wx.006","parses","capability_open","capability_open","Tableau temp-table capability probe (DROP TABLE, quoted name); parses since story 21.7's uniform Parser.ident quoting -- excluded from scoring, the capability answer is an open product decision tracked outside Epic 21" @@ -26,8 +26,8 @@ "tableau.mysql.w1.015","parses","pre_epic21","pre_epic21","parsed before Epic 21 (re-measured at ac54a079, Task 0) -- Epic 21 may not claim it" "tableau.mysql.w1.016","parses","capability_open","capability_open","Tableau temp-table capability probe (CREATE TABLE, backticked name); parses since story 21.7's uniform Parser.ident quoting -- excluded from scoring, the capability answer is an open product decision tracked outside Epic 21" "tableau.mysql.w1.017","parses","capability_open","capability_open","Tableau temp-table capability probe (DROP TABLE, quoted name); parses since story 21.7's uniform Parser.ident quoting -- excluded from scoring, the capability answer is an open product decision tracked outside Epic 21" -"tableau.mysql.w1.018","rejected","residual","epic22a_derived_table","a derived table (a SELECT in FROM or JOIN position); Epic 22 owns relational closure." -"tableau.mysql.w1.019","rejected","residual","epic22a_derived_table","a derived table (a SELECT in FROM or JOIN position); Epic 22 owns relational closure." +"tableau.mysql.w1.018","parses","residual","epic22a_derived_table","a derived table (a SELECT in FROM or JOIN position); Epic 22 owns relational closure. Story 22.1 made it PARSE (2026-09-14); it still scores residual because Epic 21 did not fix it, and it EXECUTES only through the relational engine." +"tableau.mysql.w1.019","parses","residual","epic22a_derived_table","a derived table (a SELECT in FROM or JOIN position); Epic 22 owns relational closure. Story 22.1 made it PARSE (2026-09-14); it still scores residual because Epic 21 did not fix it, and it EXECUTES only through the relational engine." "tableau.mysql.w1.020","parses","fixed","epic21","rejected before Epic 21, parses now. Blocked ONLY by identifier quoting (21.1/21.2); it belongs to no silent-wrong-answer family, so PD-3 adds no obligation beyond the parse verdict." "tableau.mysql.w1.021","parses","fixed","epic21","rejected before Epic 21, parses now. PD-3 family: aggregate-free GROUP BY with no LIMIT (#253) -- would otherwise have flipped from a loud rejection to a SILENT wrong answer; an ORDINAL GROUP BY / ORDER BY (#298) -- ORDER BY parsed before the epic and was silently DISCARDED. Correctness discharged by MERGED tests on real ES 6.8/7.17/8.18/9.0: GroupByCompletenessSpec ""GROUP BY with no aggregate"" (21.3) + the four ""corpus shape:"" tests (21.6); GroupByCompletenessSpec ""resolve an ordinal GROUP BY / ORDER BY to the n-th SELECT item (OQ-1)"" (21.3) + ""corpus shape: ... ORDER BY 1 ASC"" (21.6)." "tableau.mysql.w1.022","parses","fixed","epic21","rejected before Epic 21, parses now. PD-3 family: aggregate-free GROUP BY with no LIMIT (#253) -- would otherwise have flipped from a loud rejection to a SILENT wrong answer; GROUP BY names a SELECT alias (#296, 21.3 FOLD-IN 2) -- the pre-fix mode was ZERO buckets with HTTP 200. Correctness discharged by MERGED tests on real ES 6.8/7.17/8.18/9.0: GroupByCompletenessSpec ""GROUP BY with no aggregate"" (21.3) + the four ""corpus shape:"" tests (21.6); GroupByCompletenessSpec ""resolve a GROUP BY select-alias to the aliased field (FOLD-IN 2)"" (21.3)." @@ -51,8 +51,8 @@ "tableau.mysql.w7.040","parses","pre_epic21","pre_epic21","parsed before Epic 21 (re-measured at ac54a079, Task 0) -- Epic 21 may not claim it" "tableau.mysql.w7.041","parses","capability_open","capability_open","Tableau temp-table capability probe (CREATE TABLE, backticked name); parses since story 21.7's uniform Parser.ident quoting -- excluded from scoring, the capability answer is an open product decision tracked outside Epic 21" "tableau.mysql.w7.042","parses","capability_open","capability_open","Tableau temp-table capability probe (DROP TABLE, quoted name); parses since story 21.7's uniform Parser.ident quoting -- excluded from scoring, the capability answer is an open product decision tracked outside Epic 21" -"tableau.mysql.w7.043","rejected","residual","epic22a_derived_table","a derived table (a SELECT in FROM or JOIN position); Epic 22 owns relational closure." -"tableau.mysql.w7.044","rejected","residual","epic22a_derived_table","a derived table (a SELECT in FROM or JOIN position); Epic 22 owns relational closure." +"tableau.mysql.w7.043","parses","residual","epic22a_derived_table","a derived table (a SELECT in FROM or JOIN position); Epic 22 owns relational closure. Story 22.1 made it PARSE (2026-09-14); it still scores residual because Epic 21 did not fix it, and it EXECUTES only through the relational engine." +"tableau.mysql.w7.044","parses","residual","epic22a_derived_table","a derived table (a SELECT in FROM or JOIN position); Epic 22 owns relational closure. Story 22.1 made it PARSE (2026-09-14); it still scores residual because Epic 21 did not fix it, and it EXECUTES only through the relational engine." "tableau.mysql.w7.045","parses","fixed","epic21","rejected before Epic 21, parses now. Blocked ONLY by identifier quoting (21.1/21.2); it belongs to no silent-wrong-answer family, so PD-3 adds no obligation beyond the parse verdict." "tableau.mysql.w7.046","parses","fixed","epic21","rejected before Epic 21, parses now. PD-3 family: aggregate-free GROUP BY with no LIMIT (#253) -- would otherwise have flipped from a loud rejection to a SILENT wrong answer; an ORDINAL GROUP BY / ORDER BY (#298) -- ORDER BY parsed before the epic and was silently DISCARDED. Correctness discharged by MERGED tests on real ES 6.8/7.17/8.18/9.0: GroupByCompletenessSpec ""GROUP BY with no aggregate"" (21.3) + the four ""corpus shape:"" tests (21.6); GroupByCompletenessSpec ""resolve an ordinal GROUP BY / ORDER BY to the n-th SELECT item (OQ-1)"" (21.3) + ""corpus shape: ... ORDER BY 1 ASC"" (21.6)." "tableau.mysql.w7.047","parses","fixed","epic21","rejected before Epic 21, parses now. PD-3 family: aggregate-free GROUP BY with no LIMIT (#253) -- would otherwise have flipped from a loud rejection to a SILENT wrong answer; GROUP BY names a SELECT alias (#296, 21.3 FOLD-IN 2) -- the pre-fix mode was ZERO buckets with HTTP 200. Correctness discharged by MERGED tests on real ES 6.8/7.17/8.18/9.0: GroupByCompletenessSpec ""GROUP BY with no aggregate"" (21.3) + the four ""corpus shape:"" tests (21.6); GroupByCompletenessSpec ""resolve a GROUP BY select-alias to the aliased field (FOLD-IN 2)"" (21.3)." @@ -68,13 +68,13 @@ "tableau.mysql.w8.057","parses","fixed","epic21","rejected before Epic 21, parses now. Blocked ONLY by identifier quoting (21.1/21.2); it belongs to no silent-wrong-answer family, so PD-3 adds no obligation beyond the parse verdict." "tableau.sql92.wx.001","rejected","rejected_pending_policy","rejected_pending_policy","Tableau temp-table capability probe; deliberately refused AT PARSE TIME by the recognise-to-reject CREATE [LOCAL | GLOBAL] TEMPORARY TABLE production, which names the construct and the reason instead of an unrelated combinator. MUST STAY REJECTED -- a flip means a fix went too far" "tableau.sql92.wx.002","parses","capability_open","capability_open","Tableau temp-table capability probe (DROP TABLE, quoted name); parses since story 21.7's uniform Parser.ident quoting -- excluded from scoring, the capability answer is an open product decision tracked outside Epic 21" -"tableau.sql92.wx.003","rejected","residual","epic22a_derived_table","a derived table (a SELECT in FROM or JOIN position); Epic 22 owns relational closure." +"tableau.sql92.wx.003","parses","residual","epic22a_derived_table","a derived table (a SELECT in FROM or JOIN position); Epic 22 owns relational closure. Story 22.1 made it PARSE (2026-09-14); it still scores residual because Epic 21 did not fix it, and it EXECUTES only through the relational engine." "tableau.sql92.wx.004","parses","pre_epic21","pre_epic21","parsed before Epic 21 (re-measured at ac54a079, Task 0) -- Epic 21 may not claim it" "tableau.sql92.wx.005","rejected","rejected_pending_policy","rejected_pending_policy","Tableau temp-table capability probe; deliberately refused AT PARSE TIME by the recognise-to-reject CREATE [LOCAL | GLOBAL] TEMPORARY TABLE production, which names the construct and the reason instead of an unrelated combinator. MUST STAY REJECTED -- a flip means a fix went too far" "tableau.sql92.wx.006","parses","capability_open","capability_open","Tableau temp-table capability probe (DROP TABLE, quoted name); parses since story 21.7's uniform Parser.ident quoting -- excluded from scoring, the capability answer is an open product decision tracked outside Epic 21" "tableau.sql92.wx.007","rejected","rejected_pending_policy","rejected_pending_policy","Tableau temp-table capability probe; deliberately refused AT PARSE TIME by the recognise-to-reject CREATE [LOCAL | GLOBAL] TEMPORARY TABLE production, which names the construct and the reason instead of an unrelated combinator. MUST STAY REJECTED -- a flip means a fix went too far" "tableau.sql92.wx.008","parses","capability_open","capability_open","Tableau temp-table capability probe (DROP TABLE, quoted name); parses since story 21.7's uniform Parser.ident quoting -- excluded from scoring, the capability answer is an open product decision tracked outside Epic 21" -"tableau.sql92.wx.009","rejected","residual","epic22a_derived_table","a derived table (a SELECT in FROM or JOIN position); Epic 22 owns relational closure." +"tableau.sql92.wx.009","parses","residual","epic22a_derived_table","a derived table (a SELECT in FROM or JOIN position); Epic 22 owns relational closure. Story 22.1 made it PARSE (2026-09-14); it still scores residual because Epic 21 did not fix it, and it EXECUTES only through the relational engine." "tableau.sql92.wx.010","rejected","residual","local:select-top-n","bisected: the T-SQL TOP n clause is not in the grammar -- the same statement with LIMIT 1 parses" "tableau.sql92.wx.011","parses","fixed","epic21","rejected before Epic 21, parses now. Blocked ONLY by identifier quoting (21.1/21.2); it belongs to no silent-wrong-answer family, so PD-3 adds no obligation beyond the parse verdict." "tableau.sql92.wx.012","rejected","residual","epic22a_derived_table","a derived table (a SELECT in FROM position); Epic 22 owns relational closure. CORRECTION, re-measured 2026-09-13: ROWNUM is NOT a second blocker -- SELECT * FROM ""elastic"".""bi_events"" WHERE ROWNUM <= 1 PARSES and round-trips, because ROWNUM resolves as an ordinary identifier. That is the HAZARD: once Epic 22 adds derived tables this statement will parse and range-filter a field that does not exist, returning ZERO rows with HTTP 200 (the #205/#209/#224/#253 silent-wrong-answer family). Epic 22 must not score it fixed on the parse verdict alone." diff --git a/sql/src/test/scala/app/softnetwork/elastic/sql/census/CorpusReplaySpec.scala b/sql/src/test/scala/app/softnetwork/elastic/sql/census/CorpusReplaySpec.scala index f53d2ee21..15911afad 100644 --- a/sql/src/test/scala/app/softnetwork/elastic/sql/census/CorpusReplaySpec.scala +++ b/sql/src/test/scala/app/softnetwork/elastic/sql/census/CorpusReplaySpec.scala @@ -238,6 +238,29 @@ class CorpusReplaySpec extends AnyFlatSpec with Matchers { } TempTableProbeIds should have size 24 RejectedPendingPolicyIds should have size 3 + // Story 22.1 โ€” the derived-table partition, pinned in code and checked both ways. + DerivedTableParsesIds should have size 9 + DerivedTableRejectedIds should have size 2 + val derivedDeclared = + attribution.values.filter(_.owner == "epic22a_derived_table").map(_.captureId).toSet + withClue("rows the table owns as epic22a_derived_table that the CODE does not partition: ") { + (derivedDeclared -- DerivedTableParsesIds -- DerivedTableRejectedIds) shouldBe empty + } + withClue("derived-table rows the CODE pins that the table no longer owns: ") { + (DerivedTableParsesIds ++ DerivedTableRejectedIds -- derivedDeclared) shouldBe empty + } + checkAll( + (DerivedTableParsesIds ++ DerivedTableRejectedIds).toList.sorted, + "derived-table verdicts (pinned in code, never in the table)" + )(identity) { id => + val want = if (DerivedTableParsesIds.contains(id)) "parses" else "rejected" + if (byId(id).verdict != want) { + sys.error(s"expected $want, measured ${byId(id).verdict}") + } + if (attributionOf(attribution, id).scored != "residual") { + sys.error("a derived-table row must score residual โ€” Epic 21 did not fix it") + } + } CapabilityOpenIds should have size 21 val pending = corpus.filter(r => RejectedPendingPolicyIds.contains(r.captureId)) checkAll(pending, "policy-pending DDL probes (must STAY rejected)")(_.captureId) { row => @@ -391,9 +414,49 @@ object CorpusReplay { */ def expectedFor(owner: String): Option[String] = if (owner == "epic21" || owner == "pre_epic21") Some("parses") - else if (isIssueOwner(owner) || isLocalOwner(owner) || owner == "capability_open") None + // ๐Ÿ”ด Story 22.1 โ€” `epic22a_derived_table` ALONE stopped implying `rejected`, because that epic + // landed. It is NOT `owner.startsWith("epic22")`: `epic22b_cte` must keep implying `rejected`, + // or the single CTE row (`superset.flightsql.w6.006`) would be asserted by NOTHING โ€” neither + // by an implication nor by a code pin โ€” and the day a grammar change makes `WITH โ€ฆ AS (` + // parse by accident the gate would go green and the 21.6 headline would move in silence. + // What replaces the implication for the derived rows is the code-pinned partition below. + else if ( + isIssueOwner(owner) || isLocalOwner(owner) || owner == "capability_open" || + owner == "epic22a_derived_table" + ) None else Some("rejected") + /** Story 22.1 โ€” the derived-table rows, PINNED IN CODE for the same reason the temp-table probe + * ids are (G4): a gate whose expectations live in the file it guards can be silenced by editing + * that file. + * + * An `epic22*` owner stopped implying `rejected` when story 22.1 landed โ€” an epic in flight may + * have shipped, and the verdict is MEASURED, not assumed. What replaces the implication is this + * explicit partition of the eleven derived-table statements, asserted against the attribution + * table in BOTH directions. Their `scored` stays `residual`: Epic 21 did not fix them, and the + * 21.6 headline (56/99) must not move when a later epic lands. + */ + val DerivedTableParsesIds: Set[String] = Set( + "tableau.mysql.wx.003", + "tableau.mysql.w1.018", + "tableau.mysql.w7.043", + "tableau.sql92.wx.003", + "tableau.mysql.w1.019", + "tableau.mysql.w7.044", + "tableau.sql92.wx.009", + "superset.flightsql.w5.005", + "superset.flightsql.w7.007" + ) + + /** The two declared residuals, with their SECOND blocker โ€” `wx.012` has no correlation name (the + * alias is mandatory, SQL-92 ยง7.6) AND carries Oracle `ROWNUM`; `w8.054` carries the MySQL + * null-safe `<=>` in its ON. Both are out of epic 22's scope and must STAY rejected. + */ + val DerivedTableRejectedIds: Set[String] = Set( + "tableau.sql92.wx.012", + "tableau.mysql.w8.054" + ) + /** The 24 Tableau temp-table capability probes, PINNED HERE and not in the CSV. * * Parsing one of these answers Tableau's "temp tables supported?" probe - honouring temp tables diff --git a/sql/src/test/scala/app/softnetwork/elastic/sql/parser/DerivedTableCorpusSpec.scala b/sql/src/test/scala/app/softnetwork/elastic/sql/parser/DerivedTableCorpusSpec.scala new file mode 100644 index 000000000..6e8fae6ec --- /dev/null +++ b/sql/src/test/scala/app/softnetwork/elastic/sql/parser/DerivedTableCorpusSpec.scala @@ -0,0 +1,148 @@ +package app.softnetwork.elastic.sql.parser + +import org.scalatest.flatspec.AnyFlatSpec +import org.scalatest.matchers.should.Matchers + +/** Story 22.1 โ€” the ELEVEN derived-table statements of the Epic 19 BI corpus, each with its + * EXPECTED verdict and, for a rejection, its declared owner. + * + * Table-driven so the attribution is machine-checked rather than argued: story 21.6 AD-3's rule is + * that a verdict is never attributed by reading the error message, because `Parser.apply` reports + * ONE combinator failure and a statement with several blockers reports whichever alternative got + * furthest. Nine parse after this story; two are declared residuals with a named owner, and one of + * those (`w8.054`) carries a blocker epic 22 does not own at all. + * + * Statements are pasted VERBATIM from `epic-19-bi-corpus.csv` column `captured_statement` + * (whitespace collapsed; a trailing `;` is what `Parser.apply` strips anyway). + */ +class DerivedTableCorpusSpec extends AnyFlatSpec with Matchers { + + sealed trait Expected + case object Parses extends Expected + final case class Residual(owner: String, reasonIncludes: String) extends Expected + + behavior of "The epic-19 derived-table corpus rows" + + private val rows: Seq[(String, String, Expected)] = Seq( + ("tableau.mysql.wx.003", "SELECT `COL` FROM (SELECT 1 AS `COL`) AS `SUBQUERY`", Parses), + ("tableau.mysql.w1.018", "SELECT `COL` FROM (SELECT 1 AS `COL`) AS `SUBQUERY`", Parses), + ("tableau.mysql.w7.043", "SELECT `COL` FROM (SELECT 1 AS `COL`) AS `SUBQUERY`", Parses), + ( + "tableau.sql92.wx.003", + """SELECT "COL" FROM (SELECT 1 AS "COL") AS "SUBQUERY"""", + Parses + ), + ( + "tableau.mysql.w1.019", + "SELECT MAX(1) AS `TblMax` FROM ( SELECT * FROM `elastic`.`bi_events` `bi_events` ) " + + "`bi_events`", + Parses + ), + ( + "tableau.mysql.w7.044", + "SELECT MAX(1) AS `TblMax` FROM ( SELECT * FROM `bi_category_dim` ) `bi_category_dim`", + Parses + ), + ( + "tableau.sql92.wx.009", + """SELECT MAX(1) AS "TblMax" FROM ( SELECT * FROM "elastic"."bi_events" "bi_events" ) """ + + """"bi_events"""", + Parses + ), + ( + "superset.flightsql.w5.005", + """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""", + Parses + ), + ( + "superset.flightsql.w7.007", + """SELECT category_label AS category_label, sum(amount) AS "SUM(amount)" FROM """ + + """(SELECT e.category, e.amount, e.qty, e.country, d.category_label, d.category_group """ + + """FROM bi_events AS e JOIN bi_category_dim AS d ON e.category = d.category) AS """ + + """virtual_table GROUP BY category_label ORDER BY "SUM(amount)" DESC LIMIT 10000""", + Parses + ), + ( + "tableau.sql92.wx.012", + """SELECT * FROM (SELECT * FROM "elastic"."bi_events") WHERE ROWNUM <= 1""", + // The alias is mandatory (PD-1) AND the statement carries Oracle `ROWNUM`, which epic 22 + // does not own. The parser reports the FIRST blocker; the attribution is this table's. + Residual( + "PD-1 (no correlation name) + Oracle ROWNUM (epic 22 out of scope)", + "requires an alias" + ) + ), + ( + "tableau.mysql.w8.054", + "SELECT `t0`.`category_label` AS `category_label`, SUM(`bi_events`.`amount`) AS " + + "`sum_amount_ok` FROM `elastic`.`bi_events` `bi_events` INNER JOIN ( SELECT " + + "`bi_events`.`category` AS `category`, `bi_category_dim`.`category_label` AS " + + "`category_label` FROM `elastic`.`bi_events` `bi_events` LEFT JOIN `bi_category_dim` ON " + + "(`bi_events`.`category` = `bi_category_dim`.`category`) GROUP BY `bi_events`.`category`, " + + "`bi_category_dim`.`category_label` ) `t0` ON (`bi_events`.`category` <=> " + + "`t0`.`category`) GROUP BY `t0`.`category_label`", + // The DERIVED TABLE itself parses; the MySQL null-safe `<=>` in the ON does not. The reason + // is grammar-internal, so it is deliberately NOT pinned. + Residual("MySQL <=> in the JOIN ON (epic 22 out of scope)", "") + ) + ) + + rows.foreach { case (id, sql, expected) => + // The label is computed OUTSIDE the interpolation: a string literal inside an `s"${โ€ฆ}"` block + // is a Scala 2.12 lexer risk not worth taking in a cross-compiled test source. + val label = expected match { + case Parses => "PARSE" + case Residual(o, _) => s"stay rejected - $o" + } + it should s"$id: $label" in { + noException should be thrownBy Parser(sql) + expected match { + case Parses => + val stmt = Parser(sql).toOption.getOrElse( + fail(s"[$id] rejected: ${Parser(sql).swap.toOption.map(_.msg).getOrElse("")}") + ) + withClue(s"[$id] render is not a fixed point: ${stmt.sql} ") { + Parser(stmt.sql) shouldBe Right(stmt) + } + case Residual(_, reason) => + val msg = Parser(sql).swap.toOption.map(_.msg).getOrElse("") + withClue(s"[$id] ") { Parser(sql).isLeft shouldBe true } + // Without this a restored `throw` would still yield a Left carrying the same reason. + withClue(s"[$id] msg=[$msg] ") { msg should not startWith Parser.InternalParseFailure } + if (reason.nonEmpty) withClue(s"[$id] msg=[$msg] ") { msg should include(reason) } + } + } + } + + /** ๐Ÿ”ด The control that makes `w8.054`'s attribution machine-checked rather than prose. + * + * Its `Residual` row carries an EMPTY `reasonIncludes` (the rejection is grammar-internal and + * this project never pins such a message), so on its own it cannot tell "rejected by `<=>`" from + * "rejected because derived tables regressed" โ€” the 21.6 AD-3 failure mode. The SAME statement + * with `=` in place of `<=>` must PARSE; then `<=>` is the only variable. + */ + it should "w8.054 with = instead of <=>: PARSE โ€” so <=> is the only blocker left" in { + val withEq = rows + .find(_._1 == "tableau.mysql.w8.054") + .map(_._2.replace("<=>", "=")) + .getOrElse(fail("the w8.054 row is gone")) + Parser(withEq).isRight shouldBe true + // ๐Ÿ”ด Deliberately NOT a render fixed point, and the reason is a PRE-EXISTING defect this story + // neither causes nor fixes: a QUOTED table name in JOIN position is rejected by the grammar + // (`FROM "b"` parses, `JOIN "u"` does not). MEASURED on unmodified `origin/main` b7f20f40: + // `SELECT k FROM b INNER JOIN "u" AS "t0" ON b.k = "t0"."k"` is `Left("end of input + // expected")` there too. Since story 21.1 canonicalises backticks to double quotes, the render + // of any statement whose JOIN leg carries a quoted name cannot be re-parsed. It does not touch + // the nine rows above โ€” every JOIN they carry is a DERIVED table, which renders parenthesised + // and re-parses (asserted for each). Reported to the lead, not fixed here. + () + } + + it should "report 9 parsing and 2 declared residuals โ€” the number story 22.7 publishes" in { + rows.count(_._3 == Parses) shouldBe 9 + rows.size shouldBe 11 + } +} 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 new file mode 100644 index 000000000..a8775e961 --- /dev/null +++ b/sql/src/test/scala/app/softnetwork/elastic/sql/parser/DerivedTableSpec.scala @@ -0,0 +1,335 @@ +package app.softnetwork.elastic.sql.parser + +import app.softnetwork.elastic.sql.Alias +import app.softnetwork.elastic.sql.query._ +import org.scalatest.flatspec.AnyFlatSpec +import org.scalatest.matchers.should.Matchers + +/** Story 22.1 โ€” derived tables in FROM and JOIN: grammar, AST shape, column scope, the LATERAL + * refusal, the render fixed point WITH its rendered text, and the neighbour productions that must + * not move. + * + * Every rendered string below was MEASURED against the implementation, never guessed. + */ +class DerivedTableSpec extends AnyFlatSpec with Matchers { + + private def parse(sql: String): SingleSearch = Parser(sql) match { + case Right(s: SingleSearch) => s + case Right(other) => fail(s"[$sql] expected SingleSearch, got ${other.getClass.getSimpleName}") + case Left(e) => fail(s"[$sql] rejected: ${e.msg}") + } + + private def reasonOf(sql: String): String = Parser(sql).swap.toOption.map(_.msg).getOrElse("") + + /** `ParserTotalitySpec`'s `rejects`, copied rather than shared (it is private there): no throw, + * THEN `Left`, THEN not the boundary catch, THEN the reason. + * + * ๐Ÿ”ด The third assertion is what makes every case falsifiable. Once `Parser.apply` carries a + * `NonFatal` boundary catch (#250) a restored `throw` still yields a `Left` CONTAINING the same + * reason, so `isLeft` + `include` alone can never go red for it. + */ + private def rejects(sql: String, reasons: String*): Unit = { + withClue(s"[$sql] ") { noException should be thrownBy Parser(sql) } + withClue(s"[$sql] ") { Parser(sql).isLeft shouldBe true } + val msg = reasonOf(sql) + withClue(s"[$sql] msg=[$msg] ") { msg should not startWith Parser.InternalParseFailure } + reasons.foreach(r => withClue(s"[$sql] msg=[$msg] ") { msg should include(r) }) + () + } + + // โ”€โ”€ grammar + AST โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ + + "FROM (SELECT ...) AS alias" should "build a Table whose derived table IS its name" in { + val s = parse("SELECT COL FROM (SELECT 1 AS COL) AS SUBQUERY") + val t = s.from.mainTable + t.name shouldBe "SUBQUERY" + t.tableAlias shouldBe None + t.parts shouldBe Nil + t.derived.map(_.alias) shouldBe Some(Alias("SUBQUERY")) + t.derived.map(_.query.getClass.getSimpleName) shouldBe Some("FromlessSelect") + t.derived.flatMap(_.outputNames) shouldBe Some(Seq("COL")) + s.from.tableAliases shouldBe scala.collection.immutable.ListMap("SUBQUERY" -> "SUBQUERY") + s.from.aliasesToTable shouldBe scala.collection.immutable.ListMap("SUBQUERY" -> "SUBQUERY") + s.from.hasDerivedTables shouldBe true + s.from.enrichmentRequired shouldBe false + s.from.relationalClosureRequired shouldBe true + s.relationalClosureRequired shouldBe true + relationalClosureRequired(s) shouldBe true + derivedTablesPresent(s) shouldBe true + // ๐Ÿ”ด the ALIAS, never an index โ€” see the guards in core (AD-5) + s.sources shouldBe Seq("SUBQUERY") + } + + it should "accept the alias without AS, quoted and backticked" in { + parse("SELECT a FROM (SELECT a FROM t) d").from.mainTable.name shouldBe "d" + parse("""SELECT a FROM (SELECT a FROM t) "d"""").from.mainTable.name shouldBe "d" + parse("SELECT a FROM (SELECT a FROM t) `d`").from.mainTable.name shouldBe "d" + } + + it should "take a SingleSearch, a UNION ALL and a FROM-less body" in { + parse("SELECT a FROM (SELECT a FROM t WHERE a > 1) d").from.mainTable.derived + .map(_.query.getClass.getSimpleName) shouldBe Some("SingleSearch") + parse("SELECT a FROM (SELECT a FROM t UNION ALL SELECT a FROM u) d").from.mainTable.derived + .map(_.query.getClass.getSimpleName) shouldBe Some("MultiSearch") + parse("SELECT n FROM (SELECT 1 AS n) d").from.mainTable.derived + .map(_.query.getClass.getSimpleName) shouldBe Some("FromlessSelect") + } + + /** ๐Ÿ”ด AD-2b's own row. Without the depth-aware `whereCriteria` these two are rejected with + * "Unbalanced parentheses": the inner clause's bare `end` swallows the body's own `)`. + */ + it should "accept a body whose LAST clause is WHERE or HAVING (AD-2b)" in { + parse("SELECT a FROM (SELECT a FROM t WHERE x = 1) d") + parse("SELECT a FROM (SELECT a, COUNT(*) AS c FROM t GROUP BY a HAVING c > 1) d") + } + + "JOIN (SELECT ...) AS alias ON ..." should "put the derived table on StandardJoin.source" in { + val s = parse( + "SELECT o.id, d.total FROM orders o JOIN " + + "(SELECT cid, SUM(amount) AS total FROM orders GROUP BY cid) AS d ON o.id = d.cid" + ) + val sj = s.from.mainTable.joins.head.asInstanceOf[StandardJoin] + sj.source shouldBe a[DerivedTable] + sj.source.name shouldBe "d" + sj.alias shouldBe None + sj.parts shouldBe Nil + s.from.joinSourceKeys should contain("d") + s.from.tableAliases("d") shouldBe "d" + s.from.aliasesToTable("d") shouldBe "d" + // `enrichmentRequired` keeps its meaning โ€” "a JOIN leg exists" โ€” with no edit + s.from.enrichmentRequired shouldBe true + s.from.relationalClosureRequired shouldBe true + s.from.derivedTables.keySet shouldBe Set("d") + } + + Seq("INNER JOIN", "LEFT JOIN", "LEFT OUTER JOIN", "RIGHT JOIN", "FULL OUTER JOIN").foreach { jt => + it should s"accept every join type: $jt" in { + parse( + s"SELECT o.id FROM orders o $jt (SELECT cid FROM x) d ON o.id = d.cid" + ).from.mainTable.joins.head + .asInstanceOf[StandardJoin] + .source shouldBe a[DerivedTable] + } + } + + it should "accept CROSS JOIN (SELECT ...) d without ON (story 21.2 AD-7)" in { + parse("SELECT o.id FROM orders o CROSS JOIN (SELECT cid FROM x) d") + () + } + + it should "nest to any depth and mix with UNNEST and a comma list" in { + parse("SELECT a FROM (SELECT a FROM (SELECT a FROM (SELECT a FROM t) x) y) z") + parse("SELECT a FROM t, (SELECT a FROM u) d") + parse("SELECT a FROM (SELECT a, items FROM t) d JOIN UNNEST(d.items) i") + () + } + + // โ”€โ”€ column scope (AD-4) โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ + + "An outer reference" should "resolve against the derived table's output names" in { + val s = parse("SELECT d.total FROM (SELECT amount AS total FROM t) d WHERE d.total > 1") + val id = s.select.fields.head.identifier + id.name shouldBe "total" + id.table shouldBe Some("d") + id.tableAlias shouldBe Some("d") + } + + it should "be rejected when the derived table does not project it (qualified)" in { + rejects( + "SELECT d.amount FROM (SELECT amount AS total FROM t) d", + "Column 'amount' is not projected by derived table 'd'", + "it projects: total" + ) + } + + it should "be rejected when un-qualified over a single derived source" in { + rejects( + "SELECT nope FROM (SELECT 1 AS COL) AS d", + "Column 'nope' is not projected by derived table 'd'" + ) + } + + it should "be checked in a JOIN ON clause too, and accept struct access into a projection" in { + rejects( + "SELECT o.id FROM orders o JOIN (SELECT cid FROM x) d ON o.id = d.nope", + "Column 'nope' is not projected by derived table 'd'" + ) + // the HEAD segment `items` IS projected โ€” a dotted remainder is struct access, not a column + parse("SELECT d.items.name FROM (SELECT items FROM t) d") + () + } + + it should "enforce the AD-1 invariant on a programmatic construction" in { + val dt = DerivedTable(parse("SELECT a FROM t"), Alias("d")) + Table("d", derived = Some(dt)).validate() shouldBe Right(()) + Table("x", derived = Some(dt)).validate().isLeft shouldBe true + Table("d", Some(Alias("y")), derived = Some(dt)).validate().isLeft shouldBe true + // ๐Ÿ”ด The JOIN row needs an ON, or it is UNFALSIFIABLE: with `on = None` and + // `joinType = None` the NEXT arm of `StandardJoin.validate` answers "requires an ON clause" + // whether or not the AD-1 arm exists. The message is pinned for the same reason. + val on = parse("SELECT a FROM t JOIN u ON t.a = u.a").from.mainTable.joins.head + .asInstanceOf[StandardJoin] + .on + on should not be empty + StandardJoin(dt, None, on, alias = Some(Alias("y"))).validate() match { + case Left(msg) => msg should include("A derived table owns its alias") + case Right(_) => fail("a derived JOIN source carrying its own alias must be rejected") + } + StandardJoin(dt, None, on).validate() shouldBe Right(()) + } + + it should "not trip on an outer SELECT alias, a literal, an ordinal, * or COUNT(*)" in { + parse("SELECT COL AS c FROM (SELECT 1 AS COL) AS d ORDER BY c") + parse("SELECT 2 AS two FROM (SELECT 1 AS COL) AS d") + parse("SELECT COL FROM (SELECT 1 AS COL) AS d ORDER BY 1") + parse("SELECT * FROM (SELECT a FROM t) d") + parse("SELECT COUNT(*) AS n FROM (SELECT a FROM t) d") + () + } + + it should "treat a SELECT * body as opaque (PD-3)" in { + val s = parse("SELECT MAX(1) AS TblMax FROM (SELECT * FROM bi_events bi_events) bi_events") + s.from.mainTable.derived.flatMap(_.outputNames) shouldBe None + // never checked, because nothing here knows the mapping โ€” a wrong name fails loudly in the + // relational engine's binder instead of being guessed at + parse("SELECT anything FROM (SELECT * FROM t) d") + // a QUALIFIED star must be opaque too. `Identifier.update` normalises `e.*` to `name = "*"` + // with `tableAlias = Some("e")`, so `projected`'s `name == "*"` test sees it โ€” MEASURED. Were + // it ever to keep the dotted spelling, the projection would read `Seq("e.*")` and EVERY outer + // reference would be rejected, which is why this is pinned rather than assumed. + val q = parse("SELECT anything FROM (SELECT e.* FROM t e) d") + q.from.mainTable.derived.flatMap(_.outputNames) shouldBe None + () + } + + it should "reject a duplicate correlation name โ€” including the key-collapsing shape" in { + rejects( + "SELECT a FROM t d JOIN (SELECT a FROM u) d ON d.a = d.a", + "Alias 'd' is used by more than one source in FROM" + ) + // ๐Ÿ”ด the shape `tableAliases` CANNOT see: the ListMap has already collapsed the key, so alias + // `b` is silently gone by the time anyone reads the map. Tableau aliases a derived table with + // the inner table's OWN name, so this is the default spelling, not a corner case. + rejects( + "SELECT b.amount FROM bi_events b JOIN (SELECT category FROM bi_events) bi_events " + + "ON b.category = bi_events.category", + "Alias 'bi_events' is used by more than one source in FROM" + ) + } + + // โ”€โ”€ the LATERAL refusal (amendment from story 22.3's review) โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ + + "A derived body reading an enclosing alias" should "be rejected as LATERAL, in JOIN position" in { + 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", + "'c.id'", + "derived table 'd'", + "LATERAL" + ) + } + + 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" + ) + } + + it should "not fire for a body naming only its OWN sources" in { + parse("SELECT d.total FROM (SELECT amount AS total FROM t WHERE t.amount > 1) d") + () + } + + // โ”€โ”€ rejections that MUST be ours (AD-3 / AD-6) โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ + + "A derived table" should "require an alias (PD-1)" in { + rejects("SELECT * FROM (SELECT * FROM t)", "A derived table requires an alias") + rejects( + """SELECT * FROM (SELECT * FROM "elastic"."bi_events") WHERE ROWNUM <= 1""", + "requires an alias" + ) + } + + it should "require a SELECT body โ€” FROM (t) x stays rejected, with OUR message" in { + rejects("SELECT a FROM (t) x", "A derived table body must be a SELECT") + rejects("SELECT a FROM (SHOW TABLES) x", "A derived table body must be a SELECT") + } + + it should "be refused by DELETE and by a watcher input" in { + rejects( + "DELETE FROM (SELECT id FROM t) d WHERE id = 1", + "DELETE cannot target a derived table" + ) + rejects( + """CREATE WATCHER my_watcher AS + | EVERY 5 MINUTES + | FROM (SELECT a FROM t) d WITHIN 2 MINUTES + | ALWAYS DO + | log_action AS LOG "Watcher triggered" AT INFO + | END""".stripMargin, + "A watcher input cannot search a derived table" + ) + } + + it should "be refused by CREATE MATERIALIZED VIEW (AD-6)" in { + rejects( + "CREATE MATERIALIZED VIEW v AS SELECT a FROM (SELECT a FROM t) d", + "MATERIALIZED VIEW over a derived table" + ) + } + + it should "validate its body (an inner rule is not skipped one level down)" in { + rejects("SELECT a FROM (SELECT a, b FROM t GROUP BY a) d", "GROUP BY") + } + + // โ”€โ”€ render: fixed point AND text โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ + + private val renders = Seq( + "SELECT COL FROM (SELECT 1 AS COL) AS SUBQUERY" -> + "SELECT COL FROM (SELECT 1 AS COL) AS SUBQUERY", + "SELECT `COL` FROM (SELECT 1 AS `COL`) AS `SUBQUERY`" -> + """SELECT "COL" FROM (SELECT 1 AS "COL") AS "SUBQUERY"""", + "SELECT a FROM (SELECT a FROM t) d" -> + "SELECT a FROM (SELECT a FROM t) AS d", + "SELECT MAX(1) AS TblMax FROM ( SELECT * FROM `elastic`.`bi_events` `bi_events` ) `bi_events`" -> + """SELECT MAX(1) AS TblMax FROM (SELECT * FROM "elastic"."bi_events" AS "bi_events") AS "bi_events"""", + "SELECT o.id FROM orders o LEFT JOIN (SELECT cid FROM x) d ON o.id = d.cid" -> + "SELECT o.id FROM orders AS o LEFT JOIN (SELECT cid FROM x) AS d ON o.id = d.cid", + "SELECT a FROM (SELECT a FROM (SELECT a FROM t) x) y" -> + "SELECT a FROM (SELECT a FROM (SELECT a FROM t) AS x) AS y", + "SELECT a FROM (SELECT a FROM t UNION ALL SELECT a FROM u) d" -> + "SELECT a FROM (SELECT a FROM t UNION ALL SELECT a FROM u) AS d", + "SELECT a FROM (SELECT a FROM t WHERE x = 1) d" -> + "SELECT a FROM (SELECT a FROM t WHERE x = 1) AS d", + "SELECT a FROM t, (SELECT a FROM u) d" -> + "SELECT a FROM t,(SELECT a FROM u) AS d", + "SELECT a FROM (SELECT a, items FROM t) d JOIN UNNEST(d.items) i" -> + "SELECT a FROM (SELECT a, items FROM t) AS d JOIN UNNEST(d.items) AS i" + ) + + renders.foreach { case (in, text) => + it should s"render [$in] as its canonical text and re-parse to an equal AST" in { + val stmt = Parser(in).toOption.getOrElse(fail(s"[$in] rejected: ${reasonOf(in)}")) + stmt.sql shouldBe text + Parser(stmt.sql) shouldBe Right(stmt) + } + } + + // โ”€โ”€ neighbour pins โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ + + "Neighbouring productions" should "not move" in { + Seq( + "SELECT (a + 1) AS s FROM t", + "SELECT a FROM t WHERE (a = 1 AND b = 2)", + "SELECT a FROM t JOIN UNNEST(t.items) i", + "SELECT a FROM t1, t2", + """SELECT category FROM "elastic"."bi_events" "bi_events"""" + ).foreach { sql => + val stmt = Parser(sql).toOption.getOrElse(fail(s"[$sql] rejected: ${reasonOf(sql)}")) + withClue(s"[$sql] ") { Parser(stmt.sql) shouldBe Right(stmt) } + } + } +} diff --git a/sql/src/test/scala/app/softnetwork/elastic/sql/parser/ParserTotalitySpec.scala b/sql/src/test/scala/app/softnetwork/elastic/sql/parser/ParserTotalitySpec.scala index 194a9d2fa..d55f09d26 100644 --- a/sql/src/test/scala/app/softnetwork/elastic/sql/parser/ParserTotalitySpec.scala +++ b/sql/src/test/scala/app/softnetwork/elastic/sql/parser/ParserTotalitySpec.scala @@ -127,8 +127,10 @@ class ParserTotalitySpec extends AnyFlatSpec with Matchers { // `Where(None)` renders as no clause at all, so `DELETE FROM orders WHERE id = 1 AND` parsed as // `DELETE FROM orders` and emptied the index. Same #213 data-loss family as the `phrase` change. // Lead ruling 2026-09-05: fold the fix into this story. `where` runs only after the literal - // WHERE matched and `whereCriteria` is `rep1`, so `None` always means "a WHERE was written and - // nothing usable came of it" - no valid statement can be lost. + // WHERE matched, and `whereCriteria` yields at least one token or FAILS (story 22.1's depth-aware + // scanner keeps that contract - it returns the item's own Failure on an empty accumulator), so + // `None` always means "a WHERE was written and nothing usable came of it" - no valid statement + // can be lost. it should "reject a dangling AND in a SELECT instead of dropping the WHERE" in { rejects("SELECT a FROM t WHERE a = 1 AND", "WHERE clause requires criteria") rejects("SELECT a FROM t WHERE a = 1 OR", "WHERE clause requires criteria") @@ -155,10 +157,27 @@ class ParserTotalitySpec extends AnyFlatSpec with Matchers { // the `)` consumed by `whereCriteria` and then ignored by processTokensHelper's EndDelimiter // arm. A closing delimiter reaching that scan is unmatched by construction - a balanced group is // consumed whole by `extractSubTokens` - so rejecting it cannot lose a valid statement. + // + // ๐Ÿ”ด RETARGETED by story 22.1 (AD-2b), never deleted: these are CONTRACT pins ("a stray `)` is + // rejected, totally") and the contract is unchanged. What moved is the REASON. `whereCriteria` + // is now a depth-aware scanner that leaves a depth-0 `)` to whoever opened it - that is what + // lets `FROM (SELECT a FROM t WHERE x = 1) d` parse at all - so a stray `)` is no longer eaten + // by the clause and reaches `phrase` as trailing input instead of `processTokens` as an + // unbalanced delimiter. The reason text is dropped rather than re-pinned because it is now a + // grammar-internal message, which this project never pins. The unmatched OPENING-paren pins + // below keep `"Unbalanced parentheses"` byte-for-byte. it should "reject a stray closing parenthesis instead of swallowing it" in { - rejects("SELECT a FROM t WHERE a = 1)", "Unbalanced parentheses") - rejects("SELECT a FROM t WHERE a = 1))", "Unbalanced parentheses") - rejects("SELECT a FROM t HAVING COUNT(a) > 1)", "Unbalanced parentheses") + rejects("SELECT a FROM t WHERE a = 1)") + rejects("SELECT a FROM t WHERE a = 1))") + rejects("SELECT a FROM t HAVING COUNT(a) > 1)") + } + + // The OTHER branch of story 22.1's `if (acc.isEmpty)`: a depth-0 `)` as the clause's FIRST token + // makes the scanner fail rather than return an empty token list. Both branches are pinned, or a + // regression in one of them would be invisible. + it should "reject a clause that is nothing but a closing parenthesis" in { + rejects("SELECT a FROM t WHERE )") + rejects("SELECT a, COUNT(a) c FROM t GROUP BY a HAVING )") } // --- an `err` competing with a sibling alternative that consumed FURTHER -------------------- 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 c3b46b3fb..d55f603b9 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 @@ -1284,6 +1284,39 @@ trait ReplGatewayIntegrationSpec extends ReplIntegrationTestKit { rows shouldBe Seq(Map("1" -> 1)) } + // ========================================================================= + // 6c. Derived tables โ€” story 22.1: they PARSE, then fail LOUDLY without the relational engine + // ========================================================================= + + behavior of "REPL - derived tables without the relational engine" + + it should "refuse FROM (SELECT ...) AS alias with HTTP 400 naming the extension" in { + val res = executeSync("SELECT COL FROM (SELECT 1 AS COL) AS SUBQUERY") + res shouldBe a[ExecutionFailure] + val error = res.asInstanceOf[ExecutionFailure].error + error.statusCode shouldBe Some(400) + error.message should include("softclient4es-arrow-extensions") + error.message should include("derived table") + } + + it should "refuse JOIN (SELECT ...) AS alias the same way โ€” never run the first index alone" in { + // `dql_orders` is created by section 5, so a 404 can never satisfy this in place of the guard. + val res = executeSync( + "SELECT o.id, d.cid FROM dql_orders o JOIN (SELECT id AS cid FROM dql_orders) AS d " + + "ON o.id = d.cid" + ) + res shouldBe a[ExecutionFailure] + val error = res.asInstanceOf[ExecutionFailure].error + error.statusCode shouldBe Some(400) + error.message should include("softclient4es-arrow-extensions") + } + + it should "still answer the un-nested statement โ€” the guard did not widen" in { + // the 6b handshake pin, repeated as the control + val rows = assertQueryRows(System.nanoTime(), executeSync("SELECT 1")) + rows shouldBe Seq(Map("1" -> 1)) + } + // ========================================================================= // 7. Error handling // =========================================================================