diff --git a/core/src/main/resources/help/commands/dql/select.json b/core/src/main/resources/help/commands/dql/select.json index 4a206267..51260266 100644 --- a/core/src/main/resources/help/commands/dql/select.json +++ b/core/src/main/resources/help/commands/dql/select.json @@ -3,6 +3,7 @@ "category": "DQL", "shortDescription": "Retrieve data from one or more tables", "syntax": [ + "WITH name AS (SELECT ...) [, name2 AS (SELECT ...)]", "SELECT [DISTINCT] columns", "FROM table [AS alias]", "FROM (SELECT ...) [AS] alias", @@ -31,6 +32,11 @@ ], "description": "The SELECT statement retrieves rows from Elasticsearch indices. It supports most standard SQL features including joins, aggregations, and subqueries.", "clauses": [ + { + "name": "WITH", + "description": "Common table expressions: name a subquery once and reference it in FROM/JOIN like a table. Non-recursive; each CTE may reference the CTEs defined before it.", + "optional": true + }, { "name": "DISTINCT", "description": "Remove duplicate rows from results", @@ -131,6 +137,12 @@ "description": "Filter by a set computed by another query: the subquery runs first and its values are pushed into the outer query", "sql": "SELECT id, amount FROM orders WHERE customer_id IN (SELECT id FROM customers WHERE region = 'EU')", "output": null + }, + { + "title": "Common table expression", + "description": "Aggregate in a named subquery, then select from it by name", + "sql": "WITH monthly AS (SELECT category, SUM(amount) AS total FROM bi_events GROUP BY category) SELECT * FROM monthly", + "output": null } ], "notes": [ @@ -140,12 +152,16 @@ "A FROM-less SELECT is evaluated against a hidden softclient4es_handshake index created on first use, so a read-only account needs it pre-created", "A derived table (a SELECT in FROM or JOIN) must carry an alias; a SELECT * body exposes an unchecked projection", "A WHERE subquery that reads an outer alias (a correlated subquery) executes through the relational engine shipped in softclient4es-arrow-extensions, and is refused with HTTP 400 at every venue without it. An outer reference must be QUALIFIED with the outer table's alias - a bare column name inside the subquery is read as the subquery's own column - and an object path inside the body should be qualified with the body's own alias", - "NOT IN follows SQL: when the subquery's values contain NULL, no row matches. ANY/SOME over an empty subquery is false and ALL over an empty subquery is true" + "NOT IN follows SQL: when the subquery's values contain NULL, no row matches. ANY/SOME over an empty subquery is false and ALL over an empty subquery is true", + "A CTE name shadows an index of the same name for the duration of the statement, so a CTE cannot be named after the index it reads: write WITH orders_f AS (SELECT ... FROM orders ...) rather than WITH orders AS (SELECT ... FROM orders ...). A CTE referenced twice is executed twice (inline semantics)" ], "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", "A WHERE subquery may return at most 65536 distinct values (Elasticsearch index.max_terms_count); a larger set must be written as a JOIN", - "Subqueries are accepted in WHERE only (SELECT, DELETE, UPDATE): not in HAVING, CASE, JOIN ON, MATERIALIZED VIEW or WATCHER definitions; UNION ALL and FROM-less subquery bodies are rejected, and NOT IN over a GROUP BY subquery cannot see a NULL group" + "Subqueries are accepted in WHERE only (SELECT, DELETE, UPDATE): not in HAVING, CASE, JOIN ON, MATERIALIZED VIEW or WATCHER definitions; UNION ALL and FROM-less subquery bodies are rejected, and NOT IN over a GROUP BY subquery cannot see a NULL group", + "WITH RECURSIVE and CTE column lists (WITH a (x, y) AS ...) are not supported; a WITH clause is accepted only at the top of a SELECT, not inside a subquery body, CTAS, INSERT ... SELECT or MATERIALIZED VIEW", + "A statement with a WITH clause executes only through the relational engine (softclient4es-arrow-extensions); without it the statement is refused with HTTP 400", + "A CTE body may not name the CTE itself: unlike PostgreSQL and other engines, which bind such a name to the base table, this engine rejects it. Rename the CTE (WITH orders_f AS (SELECT ... FROM orders ...)). WITH RECURSIVE is a separate unsupported feature." ], "seeAlso": [ "INSERT", diff --git a/core/src/main/scala/app/softnetwork/elastic/client/RelationalClosureGuard.scala b/core/src/main/scala/app/softnetwork/elastic/client/RelationalClosureGuard.scala index e9a3d719..01709b26 100644 --- a/core/src/main/scala/app/softnetwork/elastic/client/RelationalClosureGuard.scala +++ b/core/src/main/scala/app/softnetwork/elastic/client/RelationalClosureGuard.scala @@ -17,7 +17,12 @@ package app.softnetwork.elastic.client import app.softnetwork.elastic.client.result.ElasticError -import app.softnetwork.elastic.sql.query.{closureSearches, derivedTablesPresent, Statement} +import app.softnetwork.elastic.sql.query.{ + closureSearches, + ctesPresent, + derivedTablesPresent, + Statement +} /** The ONE rejection every venue WITHOUT the relational engine emits for a closure-shaped statement * β€” a cross-index JOIN or a derived table (epic 22 AD-5; #157's discipline, widened). @@ -38,10 +43,21 @@ object RelationalClosureGuard { * did not choose. */ def shapeOf(statement: Statement): String = - if (closureSearches(statement).exists(_.hasCorrelatedSubqueries)) CorrelatedShape + // Story 22.5 β€” FIRST: a CTE reference IS a derived table, so a WITH statement would otherwise + // be reported as "a derived table (subquery in FROM/JOIN)" β€” a shape the user did not write. + // Naming what was actually typed is the whole point of this method. + if (ctesPresent(statement)) CteShape + else if (closureSearches(statement).exists(_.hasCorrelatedSubqueries)) CorrelatedShape else if (derivedTablesPresent(statement)) "A derived table (subquery in FROM/JOIN)" else "A cross-index JOIN" + /** Story 22.5. πŸ”΄ The words `WITH clause` sit in the first 25 characters on purpose: + * `GatewayApi.excerpt` caps a rejection at 200 characters and elides the MIDDLE (head 120 + tail + * 77), so a term that must reach the user has to fit in the FIRST 120 (story 22.3's measured + * `LATERAL` incident). + */ + private val CteShape = "A WITH clause (common table expression)" + /** Story 22.3 β€” reported FIRST, for the same reason the derived table outranks the JOIN: it is * the construct with the narrowest remedy (qualify differently, or rewrite as a JOIN), and a * statement that carries both is refused for the one the analyst is least likely to have chosen. diff --git a/core/src/test/scala/app/softnetwork/elastic/client/RelationalClosureGuardSpec.scala b/core/src/test/scala/app/softnetwork/elastic/client/RelationalClosureGuardSpec.scala index 29b745ad..c7214bbf 100644 --- a/core/src/test/scala/app/softnetwork/elastic/client/RelationalClosureGuardSpec.scala +++ b/core/src/test/scala/app/softnetwork/elastic/client/RelationalClosureGuardSpec.scala @@ -18,7 +18,12 @@ package app.softnetwork.elastic.client import app.softnetwork.elastic.client.result._ import app.softnetwork.elastic.sql.parser.Parser -import app.softnetwork.elastic.sql.query.{relationalClosureRequired, SearchStatement, SingleSearch} +import app.softnetwork.elastic.sql.query.{ + ctesPresent, + relationalClosureRequired, + SearchStatement, + SingleSearch +} import org.scalatest.flatspec.AnyFlatSpec import org.scalatest.matchers.should.Matchers import org.slf4j.{Logger, LoggerFactory} @@ -65,6 +70,11 @@ class RelationalClosureGuardSpec extends AnyFlatSpec with Matchers { private val CorrelatedSelect = "SELECT c.id FROM customers c WHERE EXISTS (SELECT 1 FROM orders o WHERE o.customer_id = c.id)" + /** Story 22.5's corpus witness, `superset.flightsql.w6.006`, verbatim. */ + private val CteSelect = + "WITH monthly AS (SELECT category, SUM(amount) AS total FROM bi_events GROUP BY category) " + + "SELECT * FROM monthly" + // ---- the ONE seam ------------------------------------------------------------------------ behavior of "SearchApi.resolveWithSchema (the direct-API seam)" @@ -233,6 +243,53 @@ class RelationalClosureGuardSpec extends AnyFlatSpec with Matchers { err.message should include("A correlated subquery") } + /** Story 22.5 β€” the CTE shape is reported FIRST, and the test that matters is the one below it: a + * CTE reference IS a derived table, so WITHOUT the arm this statement is refused as "a derived + * table (subquery in FROM/JOIN)" β€” a construct the analyst never wrote. + */ + it should "name the CTE shape, and prefer it over the derived table it is made of" in { + val cte = searchStatement(CteSelect) + relationalClosureRequired(cte) shouldBe true + ctesPresent(cte) shouldBe true + RelationalClosureGuard.shapeOf(cte) should include("WITH clause") + // The falsifiable half: it must NOT fall through to the derived-table wording. + RelationalClosureGuard.shapeOf(cte) should not include "derived table" + // ... and the plain derived table must still get its own name (the arm did not swallow it). + RelationalClosureGuard.shapeOf(searchStatement(DerivedSelect)) should include("derived table") + } + + it should "refuse a CTE statement on the direct API at the seam" in { + val err = refusalOf(client().search(searchStatement(CteSelect))) + err.statusCode shouldBe Some(400) + err.operation shouldBe Some("search") + err.message should include("WITH clause") + err.message should include(RelationalClosureGuard.ExtensionJar) + } + + it should "refuse a CTE statement even when NO CTE is referenced" in { + // The classifier cannot count references, and the AST predicate must agree with it. + val err = + refusalOf(client().search(searchStatement("WITH u AS (SELECT 1 AS x) SELECT a FROM t"))) + err.statusCode shouldBe Some(400) + err.message should include("WITH clause") + } + + /** Story 22.5, Task 0 row 13. `IndicesApi.parseQueryForDeletion` ALREADY sniffed a leading `WITH` + * as SQL before this story, so a CTE statement handed to `deleteByQuery` used to die on a lexer + * error. It now PARSES, and both routes must still refuse it β€” never a delete-by-query over an + * index named after the CTE's alias. + */ + it should "refuse a CTE statement handed to deleteByQuery, on both routes" in { + val sql = "WITH c AS (SELECT id FROM t) SELECT * FROM c" + // alias != index: the SingleSearch arm's index check fires first (loud, 400). + val mismatch = refusalOf(client().asInstanceOf[IndicesApi].deleteByQuery("t", sql)) + mismatch.statusCode shouldBe Some(400) + // alias == index: the arm reaches `resolveDmlWithSchema` and the seam's closure term refuses. + val seam = refusalOf(client().asInstanceOf[IndicesApi].deleteByQuery("c", sql)) + seam.statusCode shouldBe Some(400) + seam.message should include("WITH clause") + } + it should "say the statement was refused rather than executed against the first index (PD-2)" in { val msg = RelationalClosureGuard.rejection(searchStatement(JoinSelect)).message msg should include("refused rather than executed against the first index it names") diff --git a/core/src/test/scala/app/softnetwork/elastic/client/SplitStatementsSpec.scala b/core/src/test/scala/app/softnetwork/elastic/client/SplitStatementsSpec.scala index 9cbf4d9b..8e46ba80 100644 --- a/core/src/test/scala/app/softnetwork/elastic/client/SplitStatementsSpec.scala +++ b/core/src/test/scala/app/softnetwork/elastic/client/SplitStatementsSpec.scala @@ -31,6 +31,15 @@ class SplitStatementsSpec extends AnyFlatSpec with Matchers { GatewayApi.splitStatements("SELECT 1; SELECT 2") shouldBe List("SELECT 1", "SELECT 2") } + /** Story 22.5 β€” `splitStatements` is a quote-aware `;` scanner keyed on `'`, `"`, backtick and + * `--`. A `WITH` token is none of those, and the parentheses of a CTE body are not either, so + * the splitter needs no change: this pin is what says so rather than assuming it. + */ + it should "split a CTE statement from a following statement on the `;`" in { + GatewayApi.splitStatements("WITH a AS (SELECT 1 AS x) SELECT * FROM a; SELECT 1") shouldBe + List("WITH a AS (SELECT 1 AS x) SELECT * FROM a", "SELECT 1") + } + it should "return a single statement unchanged" in { GatewayApi.splitStatements("SELECT * FROM t WHERE a = 1") shouldBe List("SELECT * FROM t WHERE a = 1") 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 fb1df024..caf1542f 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 @@ -477,6 +477,42 @@ class CoreDqlExtensionSpec extends AnyFlatSpec with Matchers { client.searchedStatement.get() shouldBe null } + /** Story 22.5 β€” a CTE statement reaches the SAME guard through the SAME predicate, with zero edit + * in this extension: `relationalClosureRequired` gained one disjunct and every venue inherited + * it. + * + * The falsifiable half is the pair of `null` seam assertions. A CTE statement reports + * `returnsRows == true` (no aggregate in the OUTER select), so had it slipped past the closure + * arm it would have been SCROLLED against an index named after the CTE's alias β€” `monthly` is + * not an index, and the alias is what `sources` yields. + */ + it should "reject a CTE statement, naming the WITH clause, and never reach scroll or search" in { + val (client, res) = run( + "WITH monthly AS (SELECT category, SUM(amount) AS total FROM bi_events GROUP BY category) " + + "SELECT * FROM monthly", + Quota.Community + ) + res shouldBe a[ElasticFailure] + val err = res.asInstanceOf[ElasticFailure].elasticError + err.statusCode shouldBe Some(400) + err.message should include("WITH clause") + err.message should include("softclient4es-arrow-extensions") + // It must be named as the construct the analyst WROTE, not as the derived table it becomes. + err.message should not include "derived table" + client.scrolledStatement.get() shouldBe null + client.searchedStatement.get() shouldBe null + } + + it should "reject a CTE statement even when no CTE is referenced" in { + // The arrow regex classifier keys on the leading token and cannot count references; the AST + // predicate must agree with it or the two venues disagree about who owns the statement. + val (client, res) = run("WITH u AS (SELECT 1 AS x) SELECT a FROM t", Quota.Community) + res shouldBe a[ElasticFailure] + res.asInstanceOf[ElasticFailure].elasticError.message should include("WITH clause") + client.scrolledStatement.get() shouldBe null + client.searchedStatement.get() shouldBe null + } + it should "reject INSERT ... SELECT and CTAS carrying a derived table, and claim them" in { Seq( "INSERT INTO target SELECT COL FROM (SELECT 1 AS COL) AS d", diff --git a/core/src/test/scala/app/softnetwork/elastic/client/help/HelpCorpusSpec.scala b/core/src/test/scala/app/softnetwork/elastic/client/help/HelpCorpusSpec.scala index 1d28d46b..55a29344 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 @@ -810,6 +810,17 @@ class HelpCorpusSpec extends AnyFlatSpec with Matchers { // `start ~> derivedTableBodyInner <~ end`, i.e. the already-enumerated `derivedTableBodyInner` // in parentheses, so the package walk's coverage is unchanged (the superset assertion below // stays green and no new help document is required). + // Story 22.5 adds `cteBody` and `withQuery`, and NEITHER names a new statement leaf, which is + // the check this comment block exists to record: + // - `cteBody` is `start ~> (derivedTableBodyInner | err(...)) <~ end`, i.e. the + // already-enumerated `derivedTableBodyInner` in parentheses with a message of its own. Its + // leaves are `SingleSearch` / `MultiSearch` / `FromlessSelect`, all already walked. + // - `withQuery` is `withClause ~ searchStatement`, typed `SearchStatement` because the WITH + // list is a FIELD on `SingleSearch` (`ctes`) rather than a new `Statement` kind β€” which is + // precisely why story 22.5 needs no new help document. A `WITH …` statement IS a + // `SingleSearch` (or a `MultiSearch` for a `UNION ALL` outer), both already enumerated. + // Both facts are re-checked mechanically by the superset assertion below and by this file's + // parser -> doc gate, which stays green with no new document. val expectedAbstract = Set( "statement", @@ -818,6 +829,8 @@ class HelpCorpusSpec extends AnyFlatSpec with Matchers { "dmlStatement", "searchStatement", "derivedTableBodyInner", + "cteBody", + "withQuery", "app$softnetwork$elastic$sql$parser$WhereParser$$subqueryBody" ) withClue( 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 516cbe20..ab3143fc 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 @@ -40,6 +40,68 @@ class SQLQueryValidatorSpec extends AnyFlatSpec with Matchers { )""") } + // Story 22.5 β€” a CTE statement is also a `SingleSearch`, so it reaches the SAME guarded arm with + // no new macro branch. + // + // πŸ”΄ The SQL is an UNREFERENCED CTE, and that choice is the whole test. A statement that + // REFERENCES its CTE carries a derived table, so story 22.1's `from.relationalClosureRequired` + // arm already aborts it β€” MEASURED: with such a statement, deleting `|| ctes.nonEmpty` from + // `SingleSearch.relationalClosureRequired` left this suite 25/25 GREEN, i.e. the row could not + // fail for the reason its title named. `WITH u AS (...) SELECT a FROM t` has NO derived table, + // so only the new disjunct can reject it. + it should "REJECT a CTE at compile time, even when no CTE is referenced" 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(a: Int) + + TestElasticClientApi.searchAs[Row]( + "WITH u AS (SELECT a FROM t) SELECT a FROM t" + )""") + } + + // …and the REFERENCED form too, which 22.1's arm would also catch β€” kept as the neighbour, not + // as the guard. + it should "REJECT a referenced CTE at compile time" 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(a: Int) + + TestElasticClientApi.searchAs[Row]( + "WITH m AS (SELECT a FROM t) SELECT a FROM m" + )""") + } + + /** πŸ”΄ `assertDoesNotCompile` reports only THAT a snippet failed, never WHY β€” so the two rows + * above cannot see the shape naming at all: disabling the `ctesPresent` branch in + * `closureAbortMessage` left them GREEN. The message is asserted DIRECTLY, the way + * `RelationalClosureGuardSpec` asserts `RelationalClosureGuard.shapeOf` for the runtime guard. + */ + it should "name the WITH clause in the abort message, not the derived table it becomes" in { + def parsed(sql: String): app.softnetwork.elastic.sql.query.Statement = + app.softnetwork.elastic.sql.parser.Parser(sql) match { + case Right(s) => s + case Left(e) => fail(s"[$sql] rejected: ${e.msg}") + } + val cte = "WITH m AS (SELECT a FROM t) SELECT a FROM m" + val msg = SQLQueryValidator.closureAbortMessage(parsed(cte), cte) + msg should include("WITH clauses (common table expressions)") + // The falsifiable half: it must NOT fall through to the construct the author never wrote. + msg should not include "Derived tables" + // …and the neighbours must keep their own names (the arm did not swallow them). + val derived = "SELECT COL FROM (SELECT 1 AS COL) AS d" + SQLQueryValidator.closureAbortMessage(parsed(derived), derived) should include( + "Derived tables (subqueries in FROM/JOIN)" + ) + val join = "SELECT o.id, c.name FROM orders o JOIN customers c ON o.cid = c.id" + SQLQueryValidator.closureAbortMessage(parsed(join), join) should include("Cross-index JOINs") + } + // ============================================================ // 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 18308c32..aef80078 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 @@ -20,6 +20,7 @@ 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.{ + ctesPresent, derivedTablesPresent, relationalClosureRequired, MultiSearch, @@ -184,13 +185,13 @@ trait SQLQueryValidator { // `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)) + c.abort(c.enclosingPosition, SQLQueryValidator.closureAbortMessage(request, sqlQuery)) case Right(request: SingleSearch) => request case Right(multi: MultiSearch) if relationalClosureRequired(multi) => - c.abort(c.enclosingPosition, closureAbortMessage(multi, sqlQuery)) + c.abort(c.enclosingPosition, SQLQueryValidator.closureAbortMessage(multi, sqlQuery)) case Right(multi: MultiSearch) => multi.requests.headOption.getOrElse { @@ -218,18 +219,6 @@ 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) // ============================================================ @@ -846,6 +835,27 @@ trait SQLQueryValidator { object SQLQueryValidator { val DEBUG: Boolean = sys.props.get("sql.macro.debug").contains("true") + /** ⚠️ 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[macros]` rather than `private` so `SQLQueryValidatorSpec` can assert the MESSAGE. + * `assertDoesNotCompile` reports only THAT a snippet failed, never why, so the shape naming + * below was asserted by nothing β€” the story-22.4 "name the mechanism, then ask which input + * distinguishes it" rule, caught by review. + */ + private[macros] def closureAbortMessage(statement: Statement, sqlQuery: String): String = { + val shape = + // Story 22.5 β€” FIRST, the same reason `RelationalClosureGuard.shapeOf` names the CTE first: + // a CTE reference IS a derived table, so without this arm the compile error would name a + // construct the author never wrote. + if (ctesPresent(statement)) "WITH clauses (common table expressions)" + else 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" + } + // βœ… Cache to avoid redundant validations private val validationCache = scala.collection.mutable.Map[String, Boolean]() diff --git a/sql/src/main/scala/app/softnetwork/elastic/sql/SQLKeywords.scala b/sql/src/main/scala/app/softnetwork/elastic/sql/SQLKeywords.scala index 157e99d6..410e7f26 100644 --- a/sql/src/main/scala/app/softnetwork/elastic/sql/SQLKeywords.scala +++ b/sql/src/main/scala/app/softnetwork/elastic/sql/SQLKeywords.scala @@ -433,6 +433,9 @@ object SQLKeywords { "PROCESSOR", "PROCESSORS", "RANGE", + // Story 22.5 - the `keyword("RECURSIVE")` literal of `Parser.withClause`. A REPL completion + // word, NOT a parser-reserved one: `SELECT recursive FROM t` still parses. + "RECURSIVE", "REFRESH", "RENAME", "REPLACE", 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 994defc3..41130d00 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 @@ -1313,7 +1313,96 @@ object Parser override lazy val derivedTableBodyInner: PackratParser[DqlStatement] = (searchStatement: PackratParser[DqlStatement]) | fromlessSelect + /** Story 22.5 β€” the NAME of a CTE: ONE part, bare or quoted (`identRef` normalises a single bare + * part to `parts = Nil`, so `WITH "My CTE" AS (…)` and `WITH my_cte AS (…)` spell exactly what + * `FROM "My CTE"` / `FROM my_cte` reference). A QUALIFIED spelling is an `err`: a CTE lives in + * the statement, not in a namespace, and reading `"s"."a"` as the CTE `a` would silently accept + * a qualifier the statement can never honour. + */ + lazy val cteName: PackratParser[NamePart] = identRef >> { case (n, ps) => + if (ps.size > 1) + err(s"A CTE name must be a single unqualified name, got ${renderName(ps, n)}") + else success(NamePart(n, ps.headOption.exists(_.quoted))) + } + + /** The parenthesised body of a CTE. + * + * πŸ”΄ NOT `derivedTableBodyInner` alone, and NOT `FromParser.derivedTable`: the first carries no + * `err`, so `WITH a AS (t) …` would surface a grammar-internal identifier failure; the second + * carries a derived-table-specific message and lives behind the FROM surface. A CTE body owns + * its OWN `err` β€” safe for the same reason 22.1's is: after `AS (` nothing else can match, so + * the `err` cannot steal an input a sibling would have taken. + */ + lazy val cteBody: PackratParser[DqlStatement] = + start ~> (derivedTableBodyInner | err( + "A CTE body must be a SELECT: write WITH AS (SELECT ...)" + )) <~ end + + /** ` [(col, …)] AS ()`. + * + * The column list is ACCEPTED by the grammar so it can be REFUSED with a message saying what to + * write instead; without the optional arm the rejection would be a grammar-internal `AS + * expected` at the `(`, which names neither the construct nor the remedy. + */ + lazy val cteDefinition: PackratParser[Cte] = + cteName ~ opt(start ~> rep1sep(identName, separator) <~ end) ~ (keyword("AS") ~> cteBody) >> { + case n ~ Some(cols) ~ _ => + err( + s"CTE '${n.value}' declares a column list (${cols.mkString(", ")}), which is not " + + "supported: alias the columns in the CTE's SELECT list instead " + + s"(WITH ${n.value} AS (SELECT expr AS ${cols.head}, ...))" + ) + case n ~ None ~ body => success(Cte(n, body)) + } + + /** `WITH [RECURSIVE] [, ]*`. + * + * `RECURSIVE` is refused BY NAME (epic 22 out of scope) rather than left to fail as an + * identifier, so the user is told what is unsupported instead of where the parser stopped. The + * `err` fires only when the literal follows `WITH`, so a CTE NAMED `recursive` in FIRST position + * is refused too β€” an accepted, documented cost: the same name in a later position and the + * QUOTED spelling (`WITH "recursive" AS (…)`, which `keyword("RECURSIVE")` cannot match) are + * both accepted, and the quoted form is the documented escape hatch. Neither `with` nor + * `recursive` is RESERVED by this story: rejecting a table or alias named `with` that parses + * today would be a breaking change (`feedback_alternation_order_declines`). + */ + lazy val withClause: PackratParser[Seq[Cte]] = + keyword("WITH") ~> ( + (keyword("RECURSIVE") ~> err( + "WITH RECURSIVE is not supported: only non-recursive common table expressions are " + + "accepted (a CTE may reference the CTEs defined before it, never itself)" + )) | + rep1sep(cteDefinition, separator) + ) + + /** Story 22.5 β€” a search statement prefixed by a `WITH` clause. + * + * Substitution runs HERE, in the parser action, on the FULLY PARSED statement β€” ONCE. It is not + * in `update()` because `update()` re-runs on every executed statement + * (`SearchApi.resolveWithSchema` is `copy(schema).update()`), so a rewrite there would need an + * idempotency latch and the WITH list would have to be reachable from every branch and every + * body at update time. See [[app.softnetwork.elastic.sql.query.CteSubstitution]]. + * + * Its rejections (duplicate name, forward/self reference) are `err`s: raised at the END of a + * fully consumed input they sit FURTHER than every sibling alternative's offset-0 failure, so + * `Failure.append` keeps them and the user sees our message rather than the terminal + * `dmlStatement` alternative's regex complaint. + */ + lazy val withQuery: PackratParser[SearchStatement] = + withClause ~ searchStatement >> { case ctes ~ body => + CteSubstitution(ctes, body) match { + case Right(stmt) => success(stmt) + case Left(reason) => err(reason) + } + } + lazy val dqlStatement: PackratParser[DqlStatement] = { + // Story 22.5 β€” FIRST, and it narrows nothing: `withQuery` begins with the literal `(?i)WITH\b` + // while every other alternative of `dqlStatement`, `ddlStatement` and `dmlStatement` begins + // with a DIFFERENT keyword, so they are disjoint at the first token. First is chosen because a + // `\bWITH\b` test is cheaper than trying the SELECT regex on every statement, and because it + // keeps `searchStatement` and `fromlessSelect` adjacent (#251). + withQuery | searchStatement | // Issue #251 β€” FROM-less SELECT. MUST stay immediately AFTER searchStatement: `|` commits // to the first SUCCEEDING alternative, and searchStatement FAILS (not partially succeeds) diff --git a/sql/src/main/scala/app/softnetwork/elastic/sql/query/Cte.scala b/sql/src/main/scala/app/softnetwork/elastic/sql/query/Cte.scala new file mode 100644 index 00000000..87ff2c5a --- /dev/null +++ b/sql/src/main/scala/app/softnetwork/elastic/sql/query/Cte.scala @@ -0,0 +1,319 @@ +/* + * 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.{renderName, Alias, NamePart, Token} + +/** One common table expression of a `WITH` clause (story 22.5): ` AS ()`. + * + * `name` is the part exactly as written (bare or quoted -- ONE lexeme, never split, #85's rule); + * `query` is what the grammar's `cteBody` produced (`SearchStatement | FromlessSelect`, already + * `.update()`-d by `Parser.single`), with every EARLIER CTE of the same WITH list already + * substituted into it by [[CteSubstitution.resolve]]. + * + * Non-recursive by construction: a CTE body may reference the CTEs that PRECEDE it and nothing + * else -- a reference to itself or to a later CTE is rejected at parse time. + */ +case class Cte(name: NamePart, query: DqlStatement) extends Token { + + override def sql: String = s"${renderName(Seq(name), name.value)} AS (${query.sql})" + + override def validate(): Either[String, Unit] = query match { + case s: SearchStatement => s.validate() + case f: FromlessSelect => f.validate() + case other => + Left(s"CTE '${name.value}' body must be a SELECT, got ${other.getClass.getSimpleName}") + } +} + +/** Story 22.5 -- turns `WITH ...` plus a parsed search statement into the statement stories 22.1 + * and 22.4 already know how to validate, route and plan: every bare, single-part FROM/JOIN + * reference whose name is a CTE name becomes `Table(name = , derived = + * Some(DerivedTable(body, alias, cte = Some(part))))`, recursively through literal derived-table + * bodies and through every statement a WHERE / HAVING / JOIN `ON` criteria embeds. + * + * ONE owner of the rule: the parser calls it from the `withQuery` action, ONCE, on the fully + * parsed statement, and `SingleSearch.validate()` only CHECKS that it ran + * ([[SingleSearch.unsubstitutedCteReference]]). Substitution deliberately does NOT live in + * `update()`, which re-runs on every executed statement (`SearchApi.resolveWithSchema` is + * `copy(schema).update()`, `Table.mergeWithSearch` re-updates too): a rewrite there would need an + * idempotency latch AND the WITH list would have to be present on every branch and every body at + * update time. After this pass `ctes` is render/carry only and nothing in `update()` reads it. + * + * Rules (SQL-92 Clause 7.12 / 6.3, the non-recursive subset): + * 1. CTE names are matched EXACTLY (case-sensitive, like every alias map in this AST) against + * `Table.name` / `StandardJoin.source.name` when the reference has AT MOST ONE part -- a + * qualified reference (`"sch".monthly`) is never a CTE reference (a qualifier names a venue's + * namespace, #85). 2. A CTE name SHADOWS an index of the same name for the statement's + * duration: the substitution is unconditional, so `FROM bi_events` reads the CTE `bi_events` + * if one is declared. Loud, not silent: the render re-emits the WITH list, so the user sees + * it. 3. Scope is LEFT-TO-RIGHT and non-recursive: CTE k's body sees CTEs 1..k-1. A body + * naming itself or a later CTE is REJECTED, never read as an index of that name. 4. Duplicate + * CTE names are rejected. An UNREFERENCED CTE is allowed (its body still validates). A CTE + * referenced twice yields two marked derived tables sharing ONE body value (inline semantics + * -- executed twice by the planner). 5. A reference that carries its own alias keeps it as the + * derived table's correlation name (`FROM monthly m` => `DerivedTable(body, Alias("m"), cte = + * Some(monthly))`); a reference whose alias EQUALS the CTE name is normalised to the canonical + * `Alias(part.value, part.quoted)` so `FROM monthly AS monthly`, `FROM "monthly" AS monthly` and + * `FROM monthly` are ONE AST and render as `FROM monthly` -- the fixed point needs a canonical + * form. + */ +object CteSubstitution { + + /** Resolve the WITH list: duplicates and forward/self references rejected, earlier CTEs + * substituted into later bodies, each rewritten body re-`update()`-d. + */ + def resolve(ctes: Seq[Cte]): Either[String, Seq[Cte]] = { + val names = ctes.map(_.name.value) + names.diff(names.distinct).distinct.headOption match { + case Some(dup) => Left(s"CTE '$dup' is defined more than once in the WITH clause") + case None => + ctes.zipWithIndex + .foldLeft(Right(Vector.empty[Cte]): Either[String, Vector[Cte]]) { + case (left @ Left(_), _) => left + case (Right(done), (cte, k)) => + val laterOrSelf = names.drop(k).toSet + referencedNames(cte.query).find(laterOrSelf.contains) match { + case Some(n) if n == cte.name.value => + // πŸ”΄ Say what HAPPENED, not what it resembles. "recursive CTEs are not + // supported" is FALSE for the commonest analyst idiom that lands here β€” + // `WITH orders AS (SELECT id FROM orders WHERE id > 1) SELECT * FROM orders` + // contains no recursion at all; the author meant the BASE TABLE, which is what + // ANSI/PostgreSQL/DuckDB bind it to (a non-recursive CTE is not in scope inside + // its own body). This engine takes rule 3 instead and refuses it, so the message + // has to name the real cause and the real remedy. The load-bearing terms sit in + // the first 120 characters because `GatewayApi.excerpt` elides the MIDDLE. + Left( + s"A CTE body may not name the CTE itself: '$n' is referenced inside its own " + + s"definition. Rename the CTE, e.g. WITH ${n}_f AS (SELECT ... FROM $n ...). " + + "(Recursive CTEs, WITH RECURSIVE, are a separate unsupported feature.)" + ) + case Some(n) => + Left( + s"CTE '${cte.name.value}' references CTE '$n', which is defined later in the " + + "WITH clause; a CTE may only reference the CTEs before it" + ) + case None => + val scope = done.map(c => c.name.value -> c).toMap + val rewritten = substituteStatement(cte.query, scope) + Right( + done :+ (if (rewritten eq cte.query) cte + else cte.copy(query = updated(rewritten))) + ) + } + } + .map(_.toSeq) + } + } + + /** Substitute the (already resolved) WITH list into a top-level search statement. The list is + * attached to the FIRST branch (a `UNION ALL` statement's WITH clause is textually attached to + * its first SELECT and `MultiSearch.sql` concatenates branch renders); every branch is rewritten + * and re-`update()`-d. + * + * `requests` is non-empty by construction (`rep1sep`); the `headOption` guard is for a + * programmatic `MultiSearch(Nil)`, which is returned untouched rather than thrown on. + */ + def apply(ctes: Seq[Cte], body: SearchStatement): Either[String, SearchStatement] = + resolve(ctes).flatMap { resolved => + val scope = resolved.map(c => c.name.value -> c).toMap + body match { + case s: SingleSearch => + Right(substituteSingle(s, scope).copy(ctes = resolved).update()) + case m: MultiSearch => + val branches = m.requests.map(b => substituteSingle(b, scope)) + branches.headOption match { + case Some(h) => + Right( + m.copy(requests = h.copy(ctes = resolved).update() +: branches.tail.map(_.update())) + ) + // `requests` is non-empty by construction (`rep1sep`); this guards a programmatic + // `MultiSearch(Nil)`, which has no branch 0 to carry the list -- so it is refused for + // the same reason the arm below is, rather than returned with the list dropped. + case None => + Left("A WITH clause cannot be attached to a MultiSearch with no branches") + } + case other => + // πŸ”΄ NEVER `case other => other`. This method is PUBLIC, and + // `SingleSearch.unsubstitutedCteReference` names it in the very message it gives an + // embedder ("build the statement through Parser or CteSubstitution"), so a caller can + // and will reach this arm. Returning the statement unchanged DROPS the WITH list BEFORE + // it is ever attached, and every safety net this story has keys on `ctes.nonEmpty` β€” the + // closure guard, `unsubstitutedCteReference`, `RelationalClosureGuard.shapeOf` and the + // render all go quiet at once. MEASURED on the `case other => other` form: + // `CteSubstitution(Seq(Cte("a", …)), SelectStatement("SELECT x FROM a"))` answered + // `Right`, `validate()` passed, the render lost the WITH clause, and `sources` was + // `List(a)` β€” i.e. the statement would have executed against an INDEX named after the + // CTE. Loud beats silent (story 22.3b's rule). + // + // `SelectStatement` cannot carry a substitution at all: its `query` is a SQL STRING and + // its `statement` is a LAZY RE-PARSE of that string, so a `WITH` written inside it is + // already substituted by `Parser` on the way through and never needs this method. + Left( + s"A WITH clause cannot be attached to a ${other.getClass.getSimpleName}: it carries " + + "the statement as SQL text, which is re-parsed on use. Write the WITH clause inside " + + "that text and let Parser substitute it." + ) + } + } + + /** Bare single-part table names a statement references in FROM/JOIN position, recursively through + * literal derived-table bodies AND through every statement embedded in a WHERE / HAVING / JOIN + * `ON` criteria ([[Criteria.embeddedStatements]]) -- the input of the forward-reference check. + * + * ONE walk feeds both that check and [[SingleSearch.unsubstitutedCteReference]], so the rewrite + * and the guard cannot disagree about where a reference may hide. + */ + private[query] def referencedNames(q: DqlStatement): Seq[String] = q match { + case s: SingleSearch => + s.from.tables.flatMap(tableNames) ++ embeddedStatements(s).flatMap(referencedNames) + case m: MultiSearch => m.requests.flatMap(referencedNames) + case _ => Nil + } + + /** Every statement a `SingleSearch` embeds in a CRITERIA position: WHERE, HAVING and each JOIN + * `ON`. Reads the structural hook on `Criteria`, so a criteria kind that carries a statement is + * walked here by construction. + */ + def embeddedStatements(s: SingleSearch): Seq[DqlStatement] = + s.where.flatMap(_.criteria).toSeq.flatMap(_.embeddedStatements) ++ + s.having.flatMap(_.criteria).toSeq.flatMap(_.embeddedStatements) ++ + s.from.joins + .collect { case sj: StandardJoin => sj } + .flatMap(_.on.toSeq) + .flatMap(_.criteria.embeddedStatements) + + private def tableNames(t: Table): Seq[String] = { + val own = t.derived match { + case Some(d) if d.cte.isEmpty => referencedNames(d.query) + case Some(_) => Nil // an already-substituted reference + case None if t.parts.size <= 1 => Seq(t.name) + case None => Nil + } + own ++ t.joins.flatMap { + case sj: StandardJoin => + sj.source match { + case d: DerivedTable if d.cte.isEmpty => referencedNames(d.query) + case _: DerivedTable => Nil + case s if sj.parts.size <= 1 => Seq(s.name) + case _ => Nil + } + case _ => Nil // UNNEST names a column path, not a table + } + } + + private def substituteStatement(q: DqlStatement, scope: Map[String, Cte]): DqlStatement = + q match { + case s: SingleSearch => substituteSingle(s, scope) + case m: MultiSearch => + val branches = m.requests.map(b => substituteSingle(b, scope)) + if (branches.zip(m.requests).forall { case (a, b) => a eq b }) m + else m.copy(requests = branches.map(_.update())) + case other => other + } + + /** Substitute the FROM/JOIN tree AND every statement embedded in a criteria (WHERE / HAVING / + * JOIN `ON`). The result is NOT yet updated -- the callers decide when (`apply` updates the top + * level; a rewritten BODY is updated by the caller that rewrote it). + * + * `eq`-preserving: a statement in which nothing matched is returned AS IS, so a body that + * references no CTE is never touched and keeps its parse-time update. + */ + private def substituteSingle(s: SingleSearch, scope: Map[String, Cte]): SingleSearch = + if (scope.isEmpty) s + else { + val tables = s.from.tables.map(t => substituteTable(t, scope)) + val where = s.where.map(w => w.copy(criteria = w.criteria.map(mapCriteria(_, scope)))) + val having = s.having.map(h => h.copy(criteria = h.criteria.map(mapCriteria(_, scope)))) + val untouched = + tables.zip(s.from.tables).forall { case (a, b) => a eq b } && + same(where.flatMap(_.criteria), s.where.flatMap(_.criteria)) && + same(having.flatMap(_.criteria), s.having.flatMap(_.criteria)) + if (untouched) s + else s.copy(from = s.from.copy(tables = tables), where = where, having = having) + } + + private def same(a: Option[Criteria], b: Option[Criteria]): Boolean = (a, b) match { + case (Some(x), Some(y)) => x eq y + case (None, None) => true + case _ => false + } + + private def mapCriteria(c: Criteria, scope: Map[String, Cte]): Criteria = + c.mapEmbeddedStatements(q => substituteStatement(q, scope)) + + /** Rule 5's canonical form: an alias that merely repeats the CTE name is NOT information, and + * keeping the written spelling would make `FROM "m" AS m` and `FROM "m"` two different ASTs with + * the same render -- a fixed point that fails for one of them. + */ + private def canonicalAlias(part: NamePart, written: Option[Alias]): Alias = + written match { + case Some(a) if a.alias.nonEmpty && a.alias != part.value => a + case _ => Alias(part.value, part.quoted) + } + + private def substituteTable(t: Table, scope: Map[String, Cte]): Table = { + val joins = t.joins.map(j => substituteJoin(j, scope)) + val joinsUntouched = joins.zip(t.joins).forall { case (a, b) => a eq b } + t.derived match { + case Some(d) if d.cte.isEmpty => + // A LITERAL derived table: its body may reference a CTE of the enclosing scope. + val body = substituteStatement(d.query, scope) + if ((body eq d.query) && joinsUntouched) t + else t.copy(joins = joins, derived = Some(d.copy(query = updated(body)))) + case Some(_) => if (joinsUntouched) t else t.copy(joins = joins) + case None if t.parts.size <= 1 && scope.contains(t.name) => + val cte = scope(t.name) + val alias = canonicalAlias(cte.name, t.tableAlias) + Table(alias.alias, None, joins, Nil, Some(DerivedTable(cte.query, alias, Some(cte.name)))) + case None => if (joinsUntouched) t else t.copy(joins = joins) + } + } + + private def substituteJoin(j: Join, scope: Map[String, Cte]): Join = j match { + case sj: StandardJoin => + // The ON criteria may embed a statement that references a CTE -- walk it too. + val on = sj.on.map(o => o.copy(criteria = mapCriteria(o.criteria, scope))) + val onUntouched = on.zip(sj.on).forall { case (a, b) => a.criteria eq b.criteria } + val withOn = if (onUntouched) sj else sj.copy(on = on) + sj.source match { + case d: DerivedTable if d.cte.isEmpty => + val body = substituteStatement(d.query, scope) + if (body eq d.query) withOn else withOn.copy(source = d.copy(query = updated(body))) + case _: DerivedTable => withOn + case src if sj.parts.size <= 1 && scope.contains(src.name) => + val cte = scope(src.name) + val alias = canonicalAlias(cte.name, sj.alias) + withOn.copy( + source = DerivedTable(cte.query, alias, Some(cte.name)), + alias = None, + parts = Nil + ) + case _ => withOn + } + case other => other + } + + /** Re-resolve a body whose FROM tree was rewritten: its `tableAliases` changed, so every + * identifier that resolved against the old source has to resolve again. + */ + private def updated(q: DqlStatement): DqlStatement = q match { + case s: SingleSearch => s.update() + case m: MultiSearch => m.update() + case other => other + } +} 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 00cef988..19371246 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 @@ -330,15 +330,39 @@ case class StandardJoin( * 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 { +case class DerivedTable( + query: DqlStatement, + alias: Alias, + /** Story 22.5 β€” `Some(part)` when this derived table is a CTE REFERENCE substituted out of the + * statement's WITH list; `part` is the CTE's name exactly as written. `None` for a literal + * `(SELECT …) AS x`. + * + * A RENDER MARKER and nothing more: every resolver, scope check, guard and planner reads `query` + * / `alias` / `outputNames` exactly as it does for a literal derived table β€” that is the whole + * point of "a CTE reference IS a derived table" and is why this story adds no consumer arm + * anywhere. + */ + cte: Option[NamePart] = None +) 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). + * + * For a CTE REFERENCE the render is the CTE NAME, plus ` AS ` only when the statement + * gave the reference an alias of its own. `SingleSearch.sql` re-emits the WITH list, so this is + * what makes `Parser(stmt.sql) == Right(stmt)` hold WITHOUT expanding the statement β€” an + * expanded render would be persisted by `MaterializedViewExtension` and re-parsed as a DIFFERENT + * statement (an unreferenced CTE beside a literal derived table). */ - override def sql: String = s"(${query.sql})$alias" + override def sql: String = cte match { + case Some(part) => + val ref = renderName(Seq(part), part.value) + if (alias.alias == part.value) ref else s"$ref$alias" + case None => 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 @@ -385,10 +409,24 @@ object DerivedTable { * 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)) + private[query] def projected(s: SingleSearch): Option[Seq[String]] = { + def bareStar(f: Field): Boolean = f.identifier.name == "*" && f.identifier.functions.isEmpty + if (!s.select.fields.exists(bareStar)) Some(s.select.fields.map(_.outputName)) + else + // Story 22.5 β€” `SELECT * FROM ` (a CTE chained on a CTE, `WITH a AS (…), + // b AS (SELECT * FROM a)`) projects exactly what that source projects: KNOWN when the + // source's own projection is known, opaque otherwise. This never guesses β€” it forwards a + // projection another derived table already declared (story 22.1 PD-3 stands). + // + // Only the shape whose select list is EXACTLY the bare star over a SOLE derived source with + // no JOIN and no EXCEPT is claimed: `SELECT *, 1 AS extra FROM a` would otherwise be typed + // WITHOUT `extra` and a legal `d.extra` REJECTED. + (s.select.fields, s.from.tables) match { + case (Seq(f), Seq(t)) if bareStar(f) && t.joins.isEmpty && s.select.except.isEmpty => + t.derived.flatMap(_.outputNames) + case _ => None + } + } } object Table { diff --git a/sql/src/main/scala/app/softnetwork/elastic/sql/query/Where.scala b/sql/src/main/scala/app/softnetwork/elastic/sql/query/Where.scala index 0a365f3d..a2647915 100644 --- a/sql/src/main/scala/app/softnetwork/elastic/sql/query/Where.scala +++ b/sql/src/main/scala/app/softnetwork/elastic/sql/query/Where.scala @@ -98,6 +98,54 @@ sealed trait Criteria extends Updateable with PainlessScript { case _ => Nil } + /** Story 22.5 -- every statement this criteria EMBEDS, in statement order. + * + * DERIVED from [[subqueries]] rather than written as a second `this match`: the two would be arm + * lists over the SAME sealed hierarchy and could silently disagree about where a statement can + * hide -- the one-key-two-derivations drift story 21.3 paid for four times. `subqueries` already + * recurses through `Predicate` and the three `ElasticRelation`s and stops AT a subquery node + * (its own body is a separate scope, walked by whoever owns that scope), which is exactly the + * boundary [[CteSubstitution.referencedNames]] wants. + */ + final def embeddedStatements: Seq[DqlStatement] = subqueries.map(_.query) + + /** The WRITE half of [[embeddedStatements]]: rebuild this criteria with `f` applied to every + * statement it embeds. `eq`-preserving -- a subtree in which nothing changed is returned AS IS, + * which is what makes [[CteSubstitution]] 's "never touch a body that references no CTE" test + * meaningful. + * + * The structural arms are the same three shapes `subqueries` recurses through; the leaf arm + * delegates to [[SubqueryCriteria.withQuery]], which is ABSTRACT. + * + * πŸ”΄ What that buys, stated exactly: a new `SubqueryCriteria` cannot forget to take part -- the + * compiler refuses it. It does NOT extend to a new `Criteria` subtype that is not a + * `SubqueryCriteria`; such a type falls to `case _ => this` SILENTLY, and if it ever carries a + * statement the failure mode is a CTE name inside `IN (SELECT ... FROM cte)` read as an INDEX + * (`index_not_found` when absent, a wrong answer with HTTP 200 when an index of that name + * exists). `Criteria` is sealed, so that would be an edit to THIS file -- which is the whole of + * the protection, and it is a convention rather than a compiler guarantee. + */ + def mapEmbeddedStatements(f: DqlStatement => DqlStatement): Criteria = this match { + case p: Predicate => + val l = p.leftCriteria.mapEmbeddedStatements(f) + val r = p.rightCriteria.mapEmbeddedStatements(f) + if ((l eq p.leftCriteria) && (r eq p.rightCriteria)) p + else p.copy(leftCriteria = l, rightCriteria = r) + case n: ElasticNested => + val c = n.criteria.mapEmbeddedStatements(f) + if (c eq n.criteria) n else n.copy(criteria = c) + case c: ElasticChild => + val x = c.criteria.mapEmbeddedStatements(f) + if (x eq c.criteria) c else c.copy(criteria = x) + case p: ElasticParent => + val x = p.criteria.mapEmbeddedStatements(f) + if (x eq p.criteria) p else p.copy(criteria = x) + case s: SubqueryCriteria => + val q = f(s.query) + if (q eq s.query) s else s.withQuery(q) + case _ => this + } + def nested: Boolean = false def nestedElement: Option[NestedElement] @@ -1302,6 +1350,17 @@ sealed trait SubqueryCriteria extends Criteria with ElasticFilter { def maybeNot: Option[NOT.type] def correlatedRefs: Seq[Identifier] + /** This node carrying a REWRITTEN body (story 22.5) β€” the write half of + * [[Criteria.embeddedStatements]], which `Criteria.mapEmbeddedStatements` dispatches to. + * + * ABSTRACT on purpose. A defaulted `this` would let a new subquery kind silently opt out of + * every rewrite that descends through criteria (today: the CTE substitution), and the failure + * mode is a CTE name inside `IN (SELECT ... FROM cte)` resolved as an INDEX β€” `index_not_found` + * when absent, a WRONG ANSWER with HTTP 200 when an index of that name exists. Declared here, + * the compiler refuses the omission. + */ + def withQuery(q: DqlStatement): SubqueryCriteria + /** The identifiers of the OUTER statement this node names directly (its left operand) β€” what * `referencedIdentifiers` / `derivedScopeCheck` / the aggregate-in-WHERE rejection must see. * `EXISTS` names none. @@ -1392,6 +1451,8 @@ case class InSubquery( maybeNot: Option[NOT.type] = None, correlatedRefs: Seq[Identifier] = Nil ) extends SubqueryCriteria { + override def withQuery(q: DqlStatement): InSubquery = this.copy(query = q) + override def operator: Operator = IN override def sql: String = s"$identifier $notAsString$operator (${query.sql})" @@ -1432,6 +1493,8 @@ case class ExistsSubquery( maybeNot: Option[NOT.type] = None, correlatedRefs: Seq[Identifier] = Nil ) extends SubqueryCriteria { + override def withQuery(q: DqlStatement): ExistsSubquery = this.copy(query = q) + override def operator: Operator = EXISTS override def sql: String = s"$notAsString$operator (${query.sql})" @@ -1462,6 +1525,8 @@ case class ScalarSubquery( maybeNot: Option[NOT.type] = None, correlatedRefs: Seq[Identifier] = Nil ) extends SubqueryCriteria { + override def withQuery(q: DqlStatement): ScalarSubquery = this.copy(query = q) + override def sql: String = s"$notAsString$identifier $operator (${query.sql})" /** πŸ”΄ Required by `PainlessOperandFormSpec`, and it is not bookkeeping: without it a `NOT` @@ -1531,6 +1596,8 @@ case class QuantifiedSubquery( maybeNot: Option[NOT.type] = None, correlatedRefs: Seq[Identifier] = Nil ) extends SubqueryCriteria { + override def withQuery(q: DqlStatement): QuantifiedSubquery = this.copy(query = q) + override def sql: String = s"$notAsString$identifier $operator $quantifier (${query.sql})" 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 649743a3..a38faee8 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 @@ -86,6 +86,13 @@ package object query { def derivedTablesPresent(statement: Statement): Boolean = closureSearches(statement).exists(_.hasDerivedTables) + /** Story 22.5 β€” the statement carries a `WITH` clause. Narrower than + * [[relationalClosureRequired]] and read for the SAME reason as [[derivedTablesPresent]]: the + * rejection message names the shape the user actually wrote, and the remedy differs. + */ + def ctesPresent(statement: Statement): Boolean = + closureSearches(statement).exists(_.hasCtes) + /** Story 22.2 β€” the statement carries a WHERE subquery somewhere. Read by the MATERIALIZED VIEW * and WATCHER guards, which must refuse one: both render the SELECT into an Elasticsearch * artefact (a transform, a watcher input) WITHOUT crossing `SearchApi.resolveWithSchema`, so the @@ -148,10 +155,26 @@ package object query { onConflict: Option[OnConflict] = None, schema: Option[Schema] = None, explodeNested: Boolean = true, - schemas: Map[String, Schema] = Map.empty + schemas: Map[String, Schema] = Map.empty, + /** Story 22.5 β€” the statement's WITH list, in order, each body already resolved against the + * CTEs before it. Carried for the RENDER (`sql` re-emits it), for `validate()` (an + * UNREFERENCED CTE's body is still validated) and for the routing predicate + * ([[relationalClosureRequired]]). The FROM/JOIN tree carries the REFERENCES as marked derived + * tables (`DerivedTable.cte`), substituted ONCE at parse time by [[CteSubstitution]] β€” never + * here, never in `update()`. + * + * For a `UNION ALL` statement the list lives on the FIRST branch only (the WITH clause is + * textually attached to the first SELECT and `MultiSearch.sql` concatenates branch renders); + * every branch's tree carries the substituted references. `MultiSearch`'s arity is unchanged. + */ + ctes: Seq[Cte] = Nil ) extends SearchStatement { - override def sql: String = - s"$select$from${asString(where)}${asString(groupBy)}${asString(having)}${asString(orderBy)}${asString(limit)}${asString(onConflict)}" + override def sql: String = { + val withPrefix = if (ctes.isEmpty) "" else ctes.map(_.sql).mkString("WITH ", ", ", " ") + s"$withPrefix$select$from${asString(where)}${asString(groupBy)}${asString(having)}${asString( + orderBy + )}${asString(limit)}${asString(onConflict)}" + } override def withoutNestedExplosion: SingleSearch = this.copy(explodeNested = false) @@ -177,7 +200,17 @@ package object query { * disjunction somewhere else is the story-21.3 desync class. */ lazy val relationalClosureRequired: Boolean = - from.relationalClosureRequired || hasCorrelatedSubqueries + from.relationalClosureRequired || hasCorrelatedSubqueries || ctes.nonEmpty + + /** Story 22.5 β€” `ctes.nonEmpty` is a disjunct of [[relationalClosureRequired]] even when NO CTE + * is referenced, and that is deliberate rather than lazy: the regex classifier + * (`JoinDetector.CtePattern`, arrow) keys on the statement's leading token and CANNOT count + * references, while `JoinDetectorSpec`'s anti-drift property asserts that the classifier and + * this AST predicate agree on every row. A statement whose CTEs are all unreferenced plans at + * the engine as ONE plain leg β€” correct, and rare enough that a second rule would cost more + * than the round trip it saves. + */ + lazy val hasCtes: Boolean = ctes.nonEmpty /** Every WHERE-subquery node this statement carries, in statement order (story 22.2). * @@ -724,8 +757,39 @@ package object query { case None => Right(()) } + /** Story 22.5 β€” a bare single-part FROM/JOIN reference that names one of THIS statement's CTEs + * but is not a marked derived table can only come from a PROGRAMMATIC construction that + * bypassed [[CteSubstitution]] (the grammar always substitutes). It would execute against an + * INDEX of the CTE's name β€” silently. Reject it by name. + * + * It walks the SAME surface the substitution walks β€” FROM/JOIN, literal derived bodies, and + * every statement embedded in a WHERE / HAVING / ON criteria β€” because it IS + * `CteSubstitution.referencedNames`, so the check and the rewrite cannot disagree about where + * a reference may hide. (`referencedNames` already skips marked derived tables and qualified + * references, so a substituted statement reports NO name: that is the invariant checked here.) + */ + private lazy val unsubstitutedCteReference: Either[String, Unit] = { + val names = ctes.map(_.name.value).toSet + if (names.isEmpty) Right(()) + else + CteSubstitution.referencedNames(this).find(names.contains) match { + case Some(n) => + Left( + s"CTE '$n' is referenced but was not substituted: build the statement through " + + "Parser or CteSubstitution" + ) + case None => Right(()) + } + } + override def validate(): Either[String, Unit] = { for { + // Story 22.5 β€” an UNREFERENCED CTE's body reaches `DerivedTable.validate()` through no + // path at all (nothing in the FROM tree points at it), so its own GROUP BY / HAVING rules + // would be silently skipped. A REFERENCED body is validated twice (here and through + // `Table.validate` -> `d.validate()`), which is idempotent and cheap. + _ <- ctes.map(_.validate()).collectFirst { case l @ Left(_) => l }.getOrElse(Right(())) + _ <- unsubstitutedCteReference _ <- 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. diff --git a/sql/src/test/resources/corpus/epic-21-attribution.csv b/sql/src/test/resources/corpus/epic-21-attribution.csv index d5163572..4e0106c7 100644 --- a/sql/src/test/resources/corpus/epic-21-attribution.csv +++ b/sql/src/test/resources/corpus/epic-21-attribution.csv @@ -4,7 +4,7 @@ "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","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.w6.006","parses","residual","epic22b_cte","a WITH ... AS (...) common table expression; Epic 22 owns CTEs. Story 22.5 made it PARSE (2026-09-16); it still scores residual because Epic 21 did not fix it, and it EXECUTES only through the relational engine. authorship=analyst by design -- never cite it as a shape Superset emits (19.4 G9)" "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" 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 15911afa..573b62a4 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 @@ -261,6 +261,28 @@ class CorpusReplaySpec extends AnyFlatSpec with Matchers { sys.error("a derived-table row must score residual β€” Epic 21 did not fix it") } } + // Story 22.5 β€” the CTE partition, checked BOTH ways against the table exactly as the + // derived-table one is. + CteParsesIds should have size 1 + val cteDeclared = + attribution.values.filter(_.owner == "epic22b_cte").map(_.captureId).toSet + withClue("rows the table owns as epic22b_cte that the CODE does not pin: ") { + (cteDeclared -- CteParsesIds) shouldBe empty + } + withClue("CTE rows the CODE pins that the table no longer owns: ") { + (CteParsesIds -- cteDeclared) shouldBe empty + } + checkAll( + CteParsesIds.toList.sorted, + "CTE verdicts (pinned in code, never in the table)" + )(identity) { id => + if (byId(id).verdict != "parses") { + sys.error(s"expected parses, measured ${byId(id).verdict}") + } + if (attributionOf(attribution, id).scored != "residual") { + sys.error("the CTE 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 => @@ -414,15 +436,15 @@ object CorpusReplay { */ def expectedFor(owner: String): Option[String] = if (owner == "epic21" || owner == "pre_epic21") Some("parses") - // πŸ”΄ 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. + // πŸ”΄ Stories 22.1 / 22.5 β€” each `epic22*` owner stops implying `rejected` ONE AT A TIME, on the + // day its story lands, and is replaced by a code-pinned partition below. It is deliberately NOT + // `owner.startsWith("epic22")`: an owner whose story has NOT landed must keep implying + // `rejected`, or its rows would be asserted by NOTHING β€” neither an implication nor a code pin + // β€” and the day a grammar change makes them parse by accident the gate would go green and the + // 21.6 headline would move in silence. `epic22b_cte` joined the list with story 22.5. else if ( isIssueOwner(owner) || isLocalOwner(owner) || owner == "capability_open" || - owner == "epic22a_derived_table" + owner == "epic22a_derived_table" || owner == "epic22b_cte" ) None else Some("rejected") @@ -457,6 +479,19 @@ object CorpusReplay { "tableau.mysql.w8.054" ) + /** Story 22.5 β€” the CTE partition, the exact shape story 22.1 gave the derived-table rows and for + * the identical reason: once `epic22b_cte` stops implying `rejected` (the story landed), the + * only thing left asserting this row is a pin that lives in CODE, where editing the CSV cannot + * silence it. + * + * The corpus carries exactly ONE `WITH` statement, which is also story 22.5's entire corpus + * credit. Its `scored` stays `residual`: Epic 21 did not fix it, so the 21.6 headline (56/99) + * must not move because a later epic landed. + */ + val CteParsesIds: Set[String] = Set( + "superset.flightsql.w6.006" + ) + /** 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/CteSpec.scala b/sql/src/test/scala/app/softnetwork/elastic/sql/parser/CteSpec.scala new file mode 100644 index 00000000..52b2c2e0 --- /dev/null +++ b/sql/src/test/scala/app/softnetwork/elastic/sql/parser/CteSpec.scala @@ -0,0 +1,485 @@ +package app.softnetwork.elastic.sql.parser + +import app.softnetwork.elastic.sql.{Alias, NamePart} +import app.softnetwork.elastic.sql.query._ +import org.scalatest.flatspec.AnyFlatSpec +import org.scalatest.matchers.should.Matchers + +import scala.collection.immutable.ListMap + +/** Story 22.5 β€” non-recursive CTEs: the grammar, the substitution into story 22.1's derived tables, + * column scope through 22.1/22.3's check, the render fixed point WITH rendered-text pins, the + * rejections that must be OURS, and the neighbours (the DDL `WITH` families, `with` and + * `recursive` as identifiers) that must not move. + */ +class CteSpec 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("") + + /** Story 21.4's falsifiable rejection idiom. The `not startWith InternalParseFailure` assertion + * is the load-bearing half: once `Parser.apply` carries a `NonFatal` boundary catch, a restored + * `throw` still yields a `Left` whose message CONTAINS the same reason, so `isLeft` + + * `include(...)` alone cannot tell a grammar rejection from an internal fault. + */ + 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) }) + () + } + + private val witness = + "WITH monthly AS (SELECT category, SUM(amount) AS total FROM bi_events GROUP BY category) SELECT * FROM monthly" + + // -- grammar + AST --------------------------------------------------------------------------- + + "The corpus witness superset.flightsql.w6.006" should "parse into a marked derived table" in { + val s = parse(witness) + s.ctes.map(_.name) shouldBe Seq(NamePart("monthly", quoted = false)) + val t = s.from.mainTable + t.name shouldBe "monthly" + t.tableAlias shouldBe None + t.parts shouldBe Nil + val d = t.derived.getOrElse(fail("the CTE reference must be a derived table")) + d.alias shouldBe Alias("monthly") + d.cte shouldBe Some(NamePart("monthly", quoted = false)) + // ONE body value, shared by the WITH entry and the reference β€” the identity a materialise-once + // optimisation would key on, and what makes "executed twice" an EXECUTOR choice, not an AST one. + (d.query eq s.ctes.head.query) shouldBe true + d.outputNames shouldBe Some(Seq("category", "total")) + s.from.hasDerivedTables shouldBe true + s.from.enrichmentRequired shouldBe false + s.relationalClosureRequired shouldBe true + relationalClosureRequired(s) shouldBe true + ctesPresent(s) shouldBe true + // The ALIAS, never an index (story 22.1 AD-1): nothing in core may hand this to Elasticsearch. + s.sources shouldBe Seq("monthly") + s.sql shouldBe witness // byte-identical render + Parser(s.sql) shouldBe Right(s) + s.validate() shouldBe Right(()) + } + + it should "keep a reference's own alias and normalise an alias equal to the CTE name" in { + val aliased = parse("WITH m AS (SELECT a FROM t) SELECT q.a FROM m q") + aliased.from.mainTable.name shouldBe "q" + aliased.from.mainTable.derived.map(_.alias) shouldBe Some(Alias("q")) + aliased.from.mainTable.derived.flatMap(_.cte) shouldBe Some(NamePart("m", quoted = false)) + aliased.from.tableAliases shouldBe ListMap("q" -> "q") + aliased.from.derivedTables.keySet shouldBe Set("q") + val id = aliased.select.fields.head.identifier + id.name shouldBe "a" + id.tableAlias shouldBe Some("q") + // `table` is deliberately NOT asserted. It keeps the FIRST pass's key (`m`, the plain table the + // reference was before substitution) because `Identifier.update` re-derives `table` only when + // `parts.size > 1` β€” MEASURED, and accepted as a residual (lead ruling OQ-5) because every + // consumer keys on `tableAlias`. `SubqueryScope.resolve` reads `tableAlias` FIRST, which is why + // the aliased scope rejection below fires correctly. + + // An alias that merely repeats the CTE name is not information: the canonical form makes the + // three spellings ONE AST, which is what lets the render be a fixed point for all of them. + parse("WITH m AS (SELECT a FROM t) SELECT a FROM m AS m") shouldBe + parse("WITH m AS (SELECT a FROM t) SELECT a FROM m") + parse("""WITH "m" AS (SELECT a FROM t) SELECT a FROM "m" AS m""") shouldBe + parse("""WITH "m" AS (SELECT a FROM t) SELECT a FROM "m"""") + } + + it should "substitute a JOIN-position reference onto StandardJoin.source" in { + val s = parse( + "WITH d AS (SELECT cid, SUM(amount) AS total FROM orders GROUP BY cid) " + + "SELECT o.id, d.total FROM orders o JOIN d ON o.id = d.cid" + ) + val sj = s.from.mainTable.joins.head.asInstanceOf[StandardJoin] + sj.source shouldBe a[DerivedTable] + sj.source.asInstanceOf[DerivedTable].cte shouldBe Some(NamePart("d", quoted = false)) + // Story 22.1's invariant: a derived JOIN source OWNS its alias. + sj.alias shouldBe None + sj.parts shouldBe Nil + s.from.enrichmentRequired shouldBe true + s.from.derivedTables.keySet shouldBe Set("d") + s.validate() shouldBe Right(()) + } + + it should "substitute EARLIER CTEs into a later body, recursively through literal derived bodies" in { + val s = parse("WITH a AS (SELECT x FROM t), b AS (SELECT x FROM a WHERE x > 1) SELECT * FROM b") + val bBody = s.ctes(1).query.asInstanceOf[SingleSearch] + bBody.from.mainTable.derived.flatMap(_.cte) shouldBe Some(NamePart("a", quoted = false)) + // TRUE is what makes story 22.4's `planDerivedLeg` take its NESTED arm for this leg. + bBody.from.relationalClosureRequired shouldBe true + + val nested = parse("WITH a AS (SELECT x FROM t) SELECT * FROM (SELECT x FROM a) d") + // The OUTER source is a LITERAL derived table (no marker) whose BODY holds the CTE reference. + nested.from.mainTable.derived.map(_.cte) shouldBe Some(None) + nested.from.mainTable.derived + .map(_.query) + .collect { case i: SingleSearch => i } + .flatMap(_.from.mainTable.derived.flatMap(_.cte)) shouldBe + Some(NamePart("a", quoted = false)) + } + + it should "reference a CTE twice under two aliases, and leave an unreferenced CTE alone" in { + val twice = parse("WITH m AS (SELECT k, n FROM t) SELECT a.k FROM m a JOIN m b ON a.k = b.k") + twice.from.tables.map(_.name) shouldBe Seq("a") + twice.from.joinSourceKeys shouldBe Set("b") + twice.from.derivedTables.keySet shouldBe Set("a", "b") + val bodies = twice.from.derivedTables.values.map(_.query).toSeq + withClue("inline semantics: two legs, ONE body value ") { + (bodies.head eq bodies(1)) shouldBe true + } + + val unref = parse("WITH u AS (SELECT 1 AS x) SELECT a FROM t") + unref.ctes should have size 1 + unref.from.hasDerivedTables shouldBe false + // A WITH clause is the engine's ALWAYS: the arrow regex classifier keys on the leading token + // and cannot count references, and the anti-drift property asserts the two agree. + unref.relationalClosureRequired shouldBe true + ctesPresent(unref) shouldBe true + } + + it should "shadow an index of the same name, and never substitute a QUALIFIED reference" in { + parse( + "WITH bi_events AS (SELECT 1 AS x) SELECT x FROM bi_events" + ).from.mainTable.derived shouldBe defined + val qualified = parse("""WITH monthly AS (SELECT 1 AS x) SELECT a FROM "sch".monthly""") + qualified.from.mainTable.derived shouldBe None + qualified.from.mainTable.parts.size shouldBe 2 + } + + it should "attach the WITH list to the FIRST branch of a UNION ALL and substitute into every branch" in { + Parser("WITH m AS (SELECT a FROM t) SELECT a FROM m UNION ALL SELECT a FROM m") match { + case Right(ms: MultiSearch) => + ms.requests.head.ctes should have size 1 + ms.requests(1).ctes shouldBe Nil + ms.requests.forall(_.from.hasDerivedTables) shouldBe true + relationalClosureRequired(ms) shouldBe true + ctesPresent(ms) shouldBe true + ms.sql shouldBe "WITH m AS (SELECT a FROM t) SELECT a FROM m UNION ALL SELECT a FROM m" + Parser(ms.sql) shouldBe Right(ms) + ms.validate() shouldBe Right(()) + case other => fail(s"expected MultiSearch, got $other") + } + } + + it should "take a FROM-less body and a UNION ALL body" in { + parse("WITH one AS (SELECT 1 AS x) SELECT x FROM one").ctes.head.query shouldBe a[ + FromlessSelect + ] + parse( + "WITH u AS (SELECT a FROM t UNION ALL SELECT a FROM v) SELECT a FROM u" + ).ctes.head.query shouldBe a[MultiSearch] + } + + // -- column scope ---------------------------------------------------------------------------- + + "An outer reference to a CTE" should "resolve against the CTE's projection and reject an un-projected column" in { + parse("WITH m AS (SELECT amount AS total FROM t) SELECT m.total FROM m WHERE m.total > 1") + rejects( + "WITH m AS (SELECT amount AS total FROM t) SELECT m.amount FROM m", + "Column 'amount' is not projected by derived table 'm'", + "it projects: total" + ) + // The ALIASED reference is checked too, and this is exactly where the stale `Identifier.table` + // residual would have hidden the statement from the check: `SubqueryScope.resolve` reads + // `tableAlias` first, which is what makes the aliased form reachable without a new fallback. + rejects( + "WITH m AS (SELECT amount AS total FROM t) SELECT q.amount FROM m q", + "Column 'amount' is not projected by derived table 'q'" + ) + // Chained CTEs stay checkable: `SELECT * FROM ` forwards that source's + // projection instead of going opaque. + rejects( + "WITH a AS (SELECT x FROM t), b AS (SELECT * FROM a) SELECT b.nope FROM b", + "Column 'nope' is not projected by derived table 'b'", + "it projects: x" + ) + parse("WITH a AS (SELECT x FROM t), b AS (SELECT * FROM a) SELECT b.x FROM b") + // `*` is never a derived-table reference, so it is never scope-checked. + parse("WITH m AS (SELECT amount AS total FROM t) SELECT * FROM m") + } + + it should "stay OPAQUE for a select list that is not exactly the bare star" in { + // The AD-7 guard: `SELECT *, 1 AS extra FROM a` must NOT be typed without `extra`, or a legal + // `b.extra` would be rejected. Opaque means the scope check declines, i.e. BOTH names pass. + parse("WITH a AS (SELECT x FROM t), b AS (SELECT *, 1 AS extra FROM a) SELECT b.extra FROM b") + parse("WITH a AS (SELECT x FROM t), b AS (SELECT *, 1 AS extra FROM a) SELECT b.nope FROM b") + } + + it should "reject a duplicate correlation name through story 22.1's sequence check" in { + rejects( + "WITH m AS (SELECT k FROM t) SELECT k FROM m JOIN m ON m.k = m.k", + "Alias 'm' is used by more than one source" + ) + } + + // -- rejections that MUST be ours ------------------------------------------------------------ + + "A WITH clause" should "refuse RECURSIVE, a column list, a duplicate, a self reference and a forward reference" in { + rejects( + "WITH RECURSIVE a AS (SELECT 1 AS x) SELECT * FROM a", + "WITH RECURSIVE is not supported" + ) + rejects( + "WITH a (x, y) AS (SELECT 1 AS x, 2 AS y) SELECT * FROM a", + "CTE 'a' declares a column list", + "alias the columns" + ) + rejects( + "WITH a AS (SELECT 1 AS x), a AS (SELECT 2 AS x) SELECT * FROM a", + "CTE 'a' is defined more than once" + ) + // πŸ”΄ The message names what HAPPENED, not what it resembles. The commonest analyst idiom that + // lands here β€” `WITH orders AS (SELECT id FROM orders WHERE id > 1) SELECT * FROM orders` β€” + // contains no recursion at all: ANSI/PostgreSQL/DuckDB bind the inner `orders` to the BASE + // TABLE, this engine takes AD-2 rule 3 and refuses it, and telling that author "recursive CTEs + // are not supported" is simply false. The remedy must be in the message. + rejects( + "WITH a AS (SELECT x FROM a) SELECT * FROM a", + "A CTE body may not name the CTE itself", + "Rename the CTE" + ) + rejects( + "WITH orders AS (SELECT id FROM orders WHERE id > 1) SELECT * FROM orders", + "A CTE body may not name the CTE itself", + "WITH orders_f AS (SELECT ... FROM orders ...)" + ) + rejects( + "WITH a AS (SELECT x FROM b), b AS (SELECT x FROM t) SELECT * FROM a", + "CTE 'a' references CTE 'b', which is defined later" + ) + rejects( + """WITH "s"."a" AS (SELECT 1 AS x) SELECT * FROM a""", + "A CTE name must be a single unqualified name" + ) + // `cteBody`'s OWN err: `derivedTableBodyInner` carries none, and `FromParser.derivedTable`'s + // names the wrong construct. + rejects("WITH a AS (t) SELECT * FROM a", "A CTE body must be a SELECT") + } + + it should "descend into an embedded WHERE-subquery body (the CTE must not be read as an INDEX)" in { + // Story 22.2 landed first, so the `Right` branch is LIVE: a parsed `IN (SELECT …)` whose walk + // is EMPTY, or whose inner main table is not MARKED, is the forgotten-descent signal β€” the + // silent-wrong-answer mode (`c` read as an index) this pin exists to catch. + val s = parse("WITH c AS (SELECT id FROM t) SELECT * FROM u WHERE k IN (SELECT id FROM c)") + val embedded = CteSubstitution.embeddedStatements(s) + embedded should not be empty + embedded.collect { case i: SingleSearch => i }.foreach { inner => + inner.from.mainTable.derived.flatMap(_.cte) shouldBe Some(NamePart("c", quoted = false)) + } + s.validate() shouldBe Right(()) + s.sql shouldBe "WITH c AS (SELECT id FROM t) SELECT * FROM u WHERE k IN (SELECT id FROM c)" + Parser(s.sql) shouldBe Right(s) + } + + it should "descend into a subquery nested under OR, under NOT IN and under a relation wrapper" in { + // πŸ”΄ A criteria tree is NOT a list of conjuncts: a node under `OR`, under a relation wrapper or + // behind a `NOT` arrives wrapped, and a walk that dispatches on the node's TYPE alone never + // sees it. Each of these shapes must still MARK the inner reference. + // + // ⚠️ Every shape below is a WHERE subquery, and that is not an omission: WHERE is the ONLY + // criteria position a subquery can occupy today. MEASURED on the control as well as on this + // branch β€” `HAVING k IN (SELECT ...)` is rejected ("A subquery is not supported in HAVING") + // and a subquery in a JOIN `ON` is rejected ("ON clause ... must use either equality operator + // or AND predicate"). `CteSubstitution.embeddedStatements` walks HAVING and ON anyway, as + // DEFENCE for whoever opens those positions; neither arm can fire today, so nothing here + // covers them and a later reader should not think otherwise. + Seq( + "WITH c AS (SELECT id FROM t) SELECT * FROM u WHERE a = 1 OR k IN (SELECT id FROM c)", + "WITH c AS (SELECT id FROM t) SELECT * FROM u WHERE k NOT IN (SELECT id FROM c)", + "WITH c AS (SELECT id FROM t) SELECT * FROM u WHERE EXISTS (SELECT id FROM c)", + "WITH c AS (SELECT id FROM t) SELECT * FROM u WHERE NESTED(k.a = 1 AND k.b IN (SELECT id FROM c))" + ).foreach { sql => + withClue(s"[$sql] ") { + val s = parse(sql) + val inners = CteSubstitution.embeddedStatements(s).collect { case i: SingleSearch => i } + inners should not be empty + inners.foreach( + _.from.mainTable.derived.flatMap(_.cte) shouldBe Some(NamePart("c", quoted = false)) + ) + Parser(s.sql) shouldBe Right(s) + } + } + } + + it should "be a loud parse error everywhere but the top of a SELECT" in { + Seq( + "SELECT a FROM (WITH x AS (SELECT a FROM t) SELECT a FROM x) d", + "CREATE TABLE tgt AS WITH x AS (SELECT a FROM t) SELECT a FROM x", + "CREATE MATERIALIZED VIEW mv AS WITH x AS (SELECT a FROM t) SELECT a FROM x", + "INSERT INTO tgt WITH x AS (SELECT a FROM t) SELECT a FROM x", + "WITH SELECT a FROM t", + "WITH a () AS (SELECT 1 AS x) SELECT * FROM a" + ).foreach(sql => rejects(sql)) // the message is grammar-internal: NEVER pin it + } + + it should "validate an UNREFERENCED body and refuse an un-substituted programmatic reference" in { + // Nothing in the FROM tree points at an unreferenced CTE, so without the `ctes` arm in + // `validate()` its own GROUP BY rule would be skipped entirely. + rejects("WITH a AS (SELECT a, b FROM t GROUP BY a) SELECT 1 FROM t", "Non-aggregated fields") + + val body = Parser("SELECT x FROM t") match { + case Right(s: DqlStatement) => s + case other => fail(s"unexpected $other") + } + val cte = Cte(NamePart("a", quoted = false), body) + SingleSearch(from = From(Seq(Table("a"))), where = None, ctes = Seq(cte)).validate() match { + case Left(msg) => msg should include("CTE 'a' is referenced but was not substituted") + case Right(_) => fail("a bare reference named like a CTE must not validate") + } + // The converse, or the arm above would pass for a statement that simply names nothing. + SingleSearch(from = From(Seq(Table("u"))), where = None, ctes = Seq(cte)) + .validate() shouldBe Right(()) + } + + /** πŸ”΄ M-1 (found by independent review). `CteSubstitution.apply` is PUBLIC, and + * `SingleSearch.unsubstitutedCteReference` names it in the very message it hands an embedder + * ("build the statement through Parser or CteSubstitution"), so a caller CAN reach the arm for a + * body kind that cannot carry a WITH list. + * + * It used to be `case other => other`, which DROPPED the list before it was ever attached. + * MEASURED on that form: `Right(SelectStatement)`, `ctesPresent` false, + * `relationalClosureRequired` false, `validate()` `Right(())`, the render lost the WITH clause + * entirely, and `sources` was `List(a)` β€” the statement would have executed against an INDEX + * named after the CTE. Every safety net this story has keys on `ctes.nonEmpty`, so dropping the + * list disarms all of them at once. Loud beats silent. + */ + "CteSubstitution.apply" should "REFUSE a body kind that cannot carry a WITH list" in { + val body = Parser("SELECT x FROM t") match { + case Right(d: DqlStatement) => d + case other => fail(s"unexpected $other") + } + val cte = Cte(NamePart("a", quoted = false), body) + CteSubstitution(Seq(cte), SelectStatement("SELECT x FROM a")) match { + case Left(msg) => + msg should include("cannot be attached to a SelectStatement") + msg should include("re-parsed") + case Right(s) => + fail(s"the WITH list was silently dropped: ctes=${ctesPresent(s)} render=[${s.sql}]") + } + // A programmatic `MultiSearch(Nil)` has no branch 0 to carry the list either. + CteSubstitution(Seq(cte), MultiSearch(Nil)).isLeft shouldBe true + // …and the control: the two body kinds that CAN carry it still do. + CteSubstitution(Seq(cte), body.asInstanceOf[SingleSearch]) match { + case Right(s: SingleSearch) => s.ctes should have size 1 + case other => fail(s"a SingleSearch must still be substituted, got $other") + } + } + + // -- render: the fixed point AND the text ---------------------------------------------------- + + private val renders = Seq( + witness -> witness, + "with m as (select a from t) select a from m" -> + "WITH m AS (SELECT a FROM t) SELECT a FROM m", + "WITH m AS (SELECT a FROM t) SELECT q.a FROM m q" -> + "WITH m AS (SELECT a FROM t) SELECT q.a FROM m AS q", + "WITH `My CTE` AS (SELECT a FROM t) SELECT a FROM `My CTE`" -> + """WITH "My CTE" AS (SELECT a FROM t) SELECT a FROM "My CTE"""", + """WITH "My CTE" AS (SELECT a FROM t) SELECT a FROM "My CTE"""" -> + """WITH "My CTE" AS (SELECT a FROM t) SELECT a FROM "My CTE"""", + "WITH a AS (SELECT x FROM t), b AS (SELECT x FROM a WHERE x > 1) SELECT * FROM b" -> + "WITH a AS (SELECT x FROM t), b AS (SELECT x FROM a WHERE x > 1) SELECT * FROM b", + "WITH d AS (SELECT cid FROM x) SELECT o.id FROM orders o LEFT JOIN d ON o.id = d.cid" -> + "WITH d AS (SELECT cid FROM x) SELECT o.id FROM orders AS o LEFT JOIN d ON o.id = d.cid", + "WITH one AS (SELECT 1 AS x) SELECT x FROM one" -> + "WITH one AS (SELECT 1 AS x) SELECT x FROM one", + "WITH u AS (SELECT 1 AS x) SELECT a FROM t" -> + "WITH u AS (SELECT 1 AS x) SELECT a FROM t" + ) + + renders.foreach { case (in, text) => + it should s"render [$in] as [$text] and re-parse to an EQUAL AST" in { + val stmt = Parser(in).toOption.getOrElse(fail(s"[$in] rejected: ${reasonOf(in)}")) + // Both halves. The fixed point alone is satisfied by a LOSSY render (it is lossy on both + // sides); the TEXT alone does not prove the render re-parses to the same statement. The + // render is what `MaterializedViewExtension` persists and re-runs. + stmt.sql shouldBe text + Parser(stmt.sql) shouldBe Right(stmt) + } + } + + // -- neighbours: the productions this story must not move ------------------------------------ + + "Neighbouring productions" should "keep `with` and `recursive` as ordinary identifiers" in { + // Story 22.5 reserves NOTHING: rejecting a name that parses today is a breaking change and + // belongs to whoever schedules one. These four are the no-narrowing pin. + Seq( + "SELECT * FROM t with", + "SELECT with FROM t", + "SELECT * FROM t recursive", + "SELECT recursive FROM t" + ).foreach { sql => + val stmt = Parser(sql).toOption.getOrElse(fail(s"[$sql] rejected: ${reasonOf(sql)}")) + withClue(s"[$sql] ") { Parser(stmt.sql) shouldBe Right(stmt) } + } + // ... and the QUOTED spelling is the documented escape hatch for a CTE named `recursive`, + // because `keyword("RECURSIVE")` cannot match a `"`. + val escaped = parse("""WITH "recursive" AS (SELECT 1 AS x) SELECT x FROM "recursive"""") + escaped.ctes.map(_.name) shouldBe Seq(NamePart("recursive", quoted = true)) + // ... as is any position but the first. + parse("WITH a AS (SELECT 1 AS x), recursive AS (SELECT 2 AS y) SELECT y FROM recursive") + } + + it should "leave the pipeline `WITH PROCESSORS` family exactly where it was" in { + // Copied from `PipelineRoundTripIdentitySpec:117`, reduced to one processor. This family gets + // the TEXT fixed point only: a pipeline statement's AST is NOT a fixed point on `origin/main` + // either. MEASURED against a control worktree at unmodified `de8f7594` β€” `Parser(s.sql) == + // Right(s)` is `false` there while `Parser(s.sql).map(_.sql) == Right(s.sql)` is `true`. It is + // pre-existing, unowned and untouched by this story; the TEXT assertion is what this story + // could actually have broken. + val sql = + """CREATE OR REPLACE PIPELINE user_pipeline WITH PROCESSORS """ + + """(RENAME (field = "old_name", target_field = "new_name", ignore_failure = true))""" + val stmt = Parser(sql).toOption.getOrElse(fail(s"[$sql] rejected: ${reasonOf(sql)}")) + Parser(stmt.sql).toOption.map(_.sql) shouldBe Some(stmt.sql) + } + + it should "leave the watcher `WITH INPUT ` family exactly where it was" in { + // The SEVENTH and last `keyword("WITH")` family (`Parser.scala:1085`, + // `opt(keyword("WITH") ~ keyword("INPUT")) ~> httpRequest`) β€” a DIFFERENT production from the + // `WITH INPUT (...)` search form in the block below. Copied from `ParserSpec:2920`. + // + // TEXT fixed point only, for the same measured reason as the pipeline row: a watcher carrying + // an HTTP input is NOT an AST fixed point on `origin/main` either (control worktree at + // unmodified `de8f7594`: AST `false`, TEXT `true` β€” the two renders are byte-identical and the + // ASTs still compare unequal). Pre-existing, unowned, untouched by this story. + val sql = + """CREATE OR REPLACE WATCHER my_watcher AS AT SCHEDULE '0 */5 * * * ?' """ + + """WITH INPUT GET PROTOCOL https HOST "www.example.com" PATH "/api/data" """ + + """HEADERS ("Authorization" = "Bearer token") TIMEOUT (connection = "5s") """ + + """ALWAYS DO log_action AS LOG "x" AT INFO END""" + val stmt = Parser(sql).toOption.getOrElse(fail(s"[$sql] rejected: ${reasonOf(sql)}")) + Parser(stmt.sql).toOption.map(_.sql) shouldBe Some(stmt.sql) + } + + it should "leave every other DDL `WITH` family exactly where it was" in { + // One statement per remaining `keyword("WITH")` family in the grammar, each copied from an + // existing fixture rather than invented (ParserSpec's materialized-view rows, WatcherSpec:198 + // / :288, QuotedDmlDdlNameSpec:545). + Seq( + "CREATE MATERIALIZED VIEW mv WITH (delay = '1s') AS SELECT id FROM orders", + "CREATE OR REPLACE MATERIALIZED VIEW mv REFRESH EVERY 60 SECONDS WITH (delay = '1s') AS SELECT id FROM orders", + "REFRESH MATERIALIZED VIEW mv WITH SCHEDULE NOW", + "CREATE OR REPLACE WATCHER my_watcher AS EVERY 5 MINUTES FROM my_index WITHIN 2 MINUTES " + + "WHEN SCRIPT 'ctx.payload.hits.total > params.threshold' USING LANG 'painless' " + + "WITH PARAMS (threshold = 10) RETURNS TRUE DO log_action AS LOG 'x' AT INFO END", + "CREATE OR REPLACE WATCHER w2 AS EVERY 5 MINUTES WITH INPUT (keys = [\"v1\",\"v2\"]) " + + "NEVER DO a AS LOG 'x' END", + "CREATE OR REPLACE WATCHER my_watch AS EVERY 5 MINUTES WITH INPUTS my_input AS " + + "FROM t WITHIN 2 MINUTES ALWAYS DO my_action AS LOG 'x' END", + // ... and the derived-table / WHERE-subquery neighbours story 22.5 builds on. + "SELECT a FROM (SELECT a FROM t) d", + "SELECT a FROM (SELECT a FROM t WHERE x = 1) d", + "SELECT * FROM u WHERE k IN (SELECT id FROM t)" + ).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/testkit/src/main/scala/app/softnetwork/elastic/client/repl/ReplGatewayIntegrationSpec.scala b/testkit/src/main/scala/app/softnetwork/elastic/client/repl/ReplGatewayIntegrationSpec.scala index 66152b5f..a4112e38 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 @@ -1450,6 +1450,56 @@ trait ReplGatewayIntegrationSpec extends ReplIntegrationTestKit { rows shouldBe Seq(Map("1" -> 1)) } + // ========================================================================= + // 6e. CTEs β€” story 22.5: they PARSE, then fail LOUDLY without the relational engine + // ========================================================================= + + behavior of "REPL - CTEs without the relational engine" + + it should "refuse WITH ... SELECT with HTTP 400 naming the extension and the WITH clause" in { + // `dql_orders` is created by section 5, so the index EXISTS: a 404 can never satisfy this + // assertion in place of the 400 guard. Before this story the statement died on a LEXER error + // (`string matching regex '(?i)COPY\b' expected but 'W' found`) β€” the message change is the + // release note. + val res = executeSync( + "WITH big AS (SELECT id FROM dql_orders WHERE id > 1) SELECT * FROM big" + ) + res shouldBe a[ExecutionFailure] + val error = res.asInstanceOf[ExecutionFailure].error + error.statusCode shouldBe Some(400) + // πŸ”΄ Both substrings must survive `GatewayApi.excerpt` (200-character cap, MIDDLE elided: + // head 120 + "..." + tail 77). `A WITH clause (common table expression)` opens the message, so + // both terms sit well inside the first 120 characters. No unit test can see that β€” only this + // one goes through `GatewayApi.run(sql)`. + error.message should include("WITH clause") + error.message should include("softclient4es-arrow-extensions") + // It must name what the analyst WROTE, not the derived table the substitution turned it into. + error.message should not include "derived table" + } + + it should "refuse a CTE statement whose CTE is never referenced" in { + val res = executeSync("WITH u AS (SELECT 1 AS x) SELECT id FROM dql_orders") + res shouldBe a[ExecutionFailure] + res.asInstanceOf[ExecutionFailure].error.statusCode shouldBe Some(400) + res.asInstanceOf[ExecutionFailure].error.message should include("WITH clause") + } + + it should "refuse WITH RECURSIVE by name, at parse time, before any leg runs" in { + val res = executeSync("WITH RECURSIVE a AS (SELECT 1 AS x) SELECT * FROM a") + res shouldBe a[ExecutionFailure] + res.asInstanceOf[ExecutionFailure].error.message should include( + "WITH RECURSIVE is not supported" + ) + } + + it should "still answer the un-nested statement β€” the guard did not widen" in { + val rows = assertQueryRows(System.nanoTime(), executeSync("SELECT 1")) + rows shouldBe Seq(Map("1" -> 1)) + // ... and a statement that merely MENTIONS `with` as an identifier is untouched. + val plain = executeSync("SELECT id FROM dql_orders ORDER BY id LIMIT 1") + plain shouldBe a[ExecutionSuccess] + } + // ========================================================================= // 7. Error handling // =========================================================================