diff --git a/es6/jest/src/test/scala/app/softnetwork/elastic/client/JestClientBiDialectExecutionSpec.scala b/es6/jest/src/test/scala/app/softnetwork/elastic/client/JestClientBiDialectExecutionSpec.scala new file mode 100644 index 000000000..8ec398e1b --- /dev/null +++ b/es6/jest/src/test/scala/app/softnetwork/elastic/client/JestClientBiDialectExecutionSpec.scala @@ -0,0 +1,19 @@ +/* + * Copyright 2025 SOFTNETWORK + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package app.softnetwork.elastic.client + +class JestClientBiDialectExecutionSpec extends BiDialectExecutionSpec diff --git a/es6/rest/src/test/scala/app/softnetwork/elastic/client/RestHighLevelClientBiDialectExecutionSpec.scala b/es6/rest/src/test/scala/app/softnetwork/elastic/client/RestHighLevelClientBiDialectExecutionSpec.scala new file mode 100644 index 000000000..d8d761e43 --- /dev/null +++ b/es6/rest/src/test/scala/app/softnetwork/elastic/client/RestHighLevelClientBiDialectExecutionSpec.scala @@ -0,0 +1,19 @@ +/* + * Copyright 2025 SOFTNETWORK + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package app.softnetwork.elastic.client + +class RestHighLevelClientBiDialectExecutionSpec extends BiDialectExecutionSpec diff --git a/es7/rest/src/test/scala/app/softnetwork/elastic/client/RestHighLevelClientBiDialectExecutionSpec.scala b/es7/rest/src/test/scala/app/softnetwork/elastic/client/RestHighLevelClientBiDialectExecutionSpec.scala new file mode 100644 index 000000000..d8d761e43 --- /dev/null +++ b/es7/rest/src/test/scala/app/softnetwork/elastic/client/RestHighLevelClientBiDialectExecutionSpec.scala @@ -0,0 +1,19 @@ +/* + * Copyright 2025 SOFTNETWORK + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package app.softnetwork.elastic.client + +class RestHighLevelClientBiDialectExecutionSpec extends BiDialectExecutionSpec diff --git a/es8/java/src/test/scala/app/softnetwork/elastic/client/JavaClientBiDialectExecutionSpec.scala b/es8/java/src/test/scala/app/softnetwork/elastic/client/JavaClientBiDialectExecutionSpec.scala new file mode 100644 index 000000000..2e8ed381e --- /dev/null +++ b/es8/java/src/test/scala/app/softnetwork/elastic/client/JavaClientBiDialectExecutionSpec.scala @@ -0,0 +1,19 @@ +/* + * Copyright 2025 SOFTNETWORK + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package app.softnetwork.elastic.client + +class JavaClientBiDialectExecutionSpec extends BiDialectExecutionSpec diff --git a/es9/java/src/test/scala/app/softnetwork/elastic/client/JavaClientBiDialectExecutionSpec.scala b/es9/java/src/test/scala/app/softnetwork/elastic/client/JavaClientBiDialectExecutionSpec.scala new file mode 100644 index 000000000..2e8ed381e --- /dev/null +++ b/es9/java/src/test/scala/app/softnetwork/elastic/client/JavaClientBiDialectExecutionSpec.scala @@ -0,0 +1,19 @@ +/* + * Copyright 2025 SOFTNETWORK + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package app.softnetwork.elastic.client + +class JavaClientBiDialectExecutionSpec extends BiDialectExecutionSpec 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 7fbca5b5c..db968a38b 100644 --- a/sql/src/main/scala/app/softnetwork/elastic/sql/SQLKeywords.scala +++ b/sql/src/main/scala/app/softnetwork/elastic/sql/SQLKeywords.scala @@ -157,6 +157,7 @@ import app.softnetwork.elastic.sql.query.{ OrderBy, RightJoin, Select, + Top, Unnest, Where } @@ -190,6 +191,10 @@ object SQLKeywords { /** Clause, join, operator and CASE syntax keywords (word-bearing TokenRegex objects). */ val clauseTokens: List[TokenRegex] = List( Select, + // T-SQL's row bound, accepted as a spelling of LIMIT. Listed here because the `Expr` scan in + // `SQLKeywordsSpec` requires every word-bearing TokenRegex object to be registered; it is NOT + // reserved, and `SELECT top FROM t` still reads `top` as a column. + Top, Distinct, From, Where, @@ -439,6 +444,7 @@ object SQLKeywords { "PARAMS", "PARQUET", "PARTITION", + "PERCENT", "PIPELINE", "PIPELINES", "POLICIES", @@ -466,6 +472,7 @@ object SQLKeywords { "TABLE", "TABLES", "TEMPORARY", + "TIES", "TO", "TRUE", "TRUNCATE", diff --git a/sql/src/main/scala/app/softnetwork/elastic/sql/function/string/package.scala b/sql/src/main/scala/app/softnetwork/elastic/sql/function/string/package.scala index 9d4480361..08cd7f4e9 100644 --- a/sql/src/main/scala/app/softnetwork/elastic/sql/function/string/package.scala +++ b/sql/src/main/scala/app/softnetwork/elastic/sql/function/string/package.scala @@ -72,8 +72,17 @@ package object string { case object LeftOp extends Expr("LEFT") with StringOp case object RightOp extends Expr("RIGHT") with StringOp case object For extends Expr("FOR") with TokenRegex + + /** `CHAR_LENGTH`/`CHARACTER_LENGTH` are the SQL-92 spellings a BI tool emits for its `LEN()` + * calculated field. They are aliases, so the render normalises to `LENGTH` and re-parses. + * + * Order is NOT load-bearing here, and an earlier draft of this comment claimed it was: + * `TokenRegex.regex` appends `\b`, so `LEN` cannot match the prefix of `LENGTH` whatever the + * order, and neither `CHAR_LENGTH` nor `CHARACTER_LENGTH` is a prefix of the other. Longest + * first is kept as house style only — the real rule lives on `TokenRegex.regex`. + */ case object Length extends Expr("LENGTH") with StringOp { - override lazy val words: List[String] = List(sql, "LEN") + override lazy val words: List[String] = List(sql, "CHARACTER_LENGTH", "CHAR_LENGTH", "LEN") } case object Replace extends Expr("REPLACE") with StringOp { override lazy val words: List[String] = List(sql, "STR_REPLACE") diff --git a/sql/src/main/scala/app/softnetwork/elastic/sql/function/time/package.scala b/sql/src/main/scala/app/softnetwork/elastic/sql/function/time/package.scala index 5bdfa589b..8fb038f38 100644 --- a/sql/src/main/scala/app/softnetwork/elastic/sql/function/time/package.scala +++ b/sql/src/main/scala/app/softnetwork/elastic/sql/function/time/package.scala @@ -451,7 +451,7 @@ package object time { case object DateDiff extends Expr("DATE_DIFF") with TokenRegex with PainlessScript { override def painless(context: Option[PainlessContext]): String = ".between" - override lazy val words: List[String] = List(sql, "DATEDIFF") + override lazy val words: List[String] = List(sql, "TIMESTAMPDIFF", "DATEDIFF") } case class DateDiff( @@ -809,8 +809,13 @@ package object time { } } + /** `TIMESTAMPADD` is the ODBC/JDBC spelling (`{fn TIMESTAMPADD(SQL_TSI_DAY, -89, …)}`), which BI + * tools emit directly. It is an ALIAS, not a second mechanism: the `(unit, count, base)` + * argument order it uses is the `transactSql` form this function has always parsed, so the + * render normalises to `DATETIME_ADD` and re-parses. + */ case object DateTimeAdd extends Expr("DATETIME_ADD") with TokenRegex { - override lazy val words: List[String] = List(sql, "DATETIMEADD") + override lazy val words: List[String] = List(sql, "DATETIMEADD", "TIMESTAMPADD") } case class DateTimeAdd( 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 a590bd954..093c9351d 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 @@ -87,10 +87,19 @@ object Parser with OrderByParser with LimitParser { + /** 🔴 `TOP n` and `LIMIT n` are two spellings of ONE row bound, so carrying both would need a + * precedence rule nobody could guess from the SQL. Combining them is refused by name instead; + * `t.orElse(l)` then never has to choose. + */ lazy val single: PackratParser[SingleSearch] = { - select ~ from ~ where.? ~ groupBy.? ~ having.? ~ orderBy.? ~ limit.? ~ onConflict.? ^^ { - case s ~ f ~ w ~ g ~ h ~ o ~ l ~ oc => - SingleSearch(s, f, w, g, h, o, l, onConflict = oc).update() + select ~ from ~ where.? ~ groupBy.? ~ having.? ~ orderBy.? ~ limit.? ~ onConflict.? >> { + case (s, t) ~ f ~ w ~ g ~ h ~ o ~ l ~ oc => + if (t.isDefined && l.isDefined) + err( + "TOP and LIMIT both bound the number of rows -- use one of them, not both. TOP " + + "carries no OFFSET, so paging needs the LIMIT n OFFSET m spelling" + ) + else success(SingleSearch(s, f, w, g, h, o, t.orElse(l), onConflict = oc).update()) } } @@ -178,7 +187,14 @@ object Parser * schemaProbeSql rewrites `SELECT 1` into. */ lazy val fromlessSelect: PackratParser[FromlessSelect] = - select ~ limit.? ^^ { case s ~ l => FromlessSelect(s, l) } + select ~ limit.? >> { case (s, t) ~ l => + if (t.isDefined && l.isDefined) + err( + "TOP and LIMIT both bound the number of rows -- use one of them, not both. TOP " + + "carries no OFFSET, so paging needs the LIMIT n OFFSET m spelling" + ) + else success(FromlessSelect(s, t.orElse(l))) + } lazy val row: PackratParser[List[Value[_]]] = lparen ~> repsep(array_of_struct | struct | value, comma) <~ rparen @@ -957,8 +973,15 @@ object Parser lazy val neverWatcherCondition: PackratParser[NeverWatcherCondition.type] = keyword("NEVER") ^^ { _ => NeverWatcherCondition } + /** 🔴 Ordered LONGEST spelling first, and it is load-bearing. These are string literals, not + * anchored tokens: `gt` (`>`) matches the first character of `>=` and succeeds, leaving `=` for + * the value production, which then fails with *"A value or a date/datetime function must be + * provided for comparison"*. `WHEN x >= 0` and `WHEN x <= 0` were rejected for exactly that + * reason, while `>`, `<`, `=` and `<>` parsed. `WhereParser.comparisonOp` has always had the + * right order; this production had not. + */ private lazy val comparison_operator: PackratParser[ComparisonOperator] = - eq | ne | diff | gt | ge | lt | le + eq | ne | diff | ge | gt | le | lt private lazy val dateMathScript : PackratParser[DateTimeFunction with FunctionWithIdentifier with DateMathScript] = diff --git a/sql/src/main/scala/app/softnetwork/elastic/sql/parser/SelectParser.scala b/sql/src/main/scala/app/softnetwork/elastic/sql/parser/SelectParser.scala index 1d7ea11c3..d51aa8ae0 100644 --- a/sql/src/main/scala/app/softnetwork/elastic/sql/parser/SelectParser.scala +++ b/sql/src/main/scala/app/softnetwork/elastic/sql/parser/SelectParser.scala @@ -16,7 +16,7 @@ package app.softnetwork.elastic.sql.parser -import app.softnetwork.elastic.sql.query.{Except, Field, Select} +import app.softnetwork.elastic.sql.query.{Except, Field, Limit, Select, Top} trait SelectParser { self: Parser with WhereParser => @@ -40,12 +40,46 @@ trait SelectParser { Except(e) } - lazy val select: PackratParser[Select] = - Select.regex ~ rep1sep( + /** `TOP n` — T-SQL's row bound, which Tableau emits in its SQL-92 dialect. It is returned + * ALONGSIDE the `Select` rather than stored on it, so the statement keeps exactly one owner of + * its row bound (`Parser.single` folds it into `limit`) and the render normalises to `LIMIT n`. + * + * 🔴 `TOP` is NOT reserved, and must not become so: `SELECT top FROM t` selects a column named + * `top` today. `top.?` is safe because `Top.regex ~> long` FAILS (it does not error) when no + * number follows, and `opt` backtracks to the original position, where `field` reads `top` as + * the identifier it is. Both readings are pinned in `ParserSpec`. + */ + lazy val top: PackratParser[Limit] = + Top.regex ~> (start ~> long <~ end | long) >> { l => + if (l.value < 0 || l.value > Int.MaxValue) { + // 🔴 `failure`, NEVER `err`, and the distinction is the whole correctness of `top.?`. + // `err` yields an `Error`, and `Parsers.|` does not try another alternative after an + // Error — so `opt` could not backtrack and `SELECT top -1 AS x FROM t`, which reads + // `top - 1` and parses on main, became a hard rejection. At this position `TOP` is + // genuinely ambiguous between the clause and a column called `top`; a `Failure` lets + // `field` settle it, which is the only reading that cannot regress. + failure(s"TOP takes a row count between 0 and ${Int.MaxValue}") + } else { + // `TOP n PERCENT` and `TOP n WITH TIES` are real T-SQL that this engine does not + // implement. They are refused BY NAME because the alternative is far worse: with `top` + // having consumed `TOP 5`, `field` reads `PERCENT a` as the column `PERCENT` aliased to + // `a`, so `SELECT TOP 5 PERCENT a FROM t` returned rows for a column the user never + // named — a loud rejection turned into a silent wrong answer (#205/#253 family). + // An `err` is safe HERE, unlike above: no spelling of `TOP PERCENT` parsed before. + (keyword("PERCENT") | (keyword("WITH") ~ keyword("TIES") ^^ (_ => "WITH TIES"))) >> { w => + err( + s"SELECT TOP n $w is not supported -- TOP takes a row COUNT. Use TOP n, or LIMIT n" + ) + } | success(Limit(l.value.toInt, None)) + } + } + + lazy val select: PackratParser[(Select, Option[Limit])] = + Select.regex ~ top.? ~ rep1sep( field, separator - ) ~ except.? ^^ { case _ ~ fields ~ e => - Select(fields, e) + ) ~ except.? ^^ { case _ ~ t ~ fields ~ e => + (Select(fields, e), t) } } diff --git a/sql/src/main/scala/app/softnetwork/elastic/sql/query/Select.scala b/sql/src/main/scala/app/softnetwork/elastic/sql/query/Select.scala index 051b6ef80..9bdcb09dd 100644 --- a/sql/src/main/scala/app/softnetwork/elastic/sql/query/Select.scala +++ b/sql/src/main/scala/app/softnetwork/elastic/sql/query/Select.scala @@ -151,6 +151,14 @@ case class Field( case object Except extends Expr("except") with TokenRegex +/** `SELECT TOP n` (T-SQL; Tableau emits it in its SQL-92 dialect, e.g. `SELECT TOP 1 *`). + * + * It is a SPELLING of the row bound, not a second bound: `SelectParser.select` hands it to + * `Parser.single`, which stores it in the statement's `limit` and renders `LIMIT n`. So there is + * exactly ONE owner of the row bound in the AST, and the render re-parses. + */ +case object Top extends Expr("TOP") with TokenRegex + case class Except(fields: Seq[Field]) extends Updateable { override def sql: String = s" $Except(${fields.mkString(",")})" def update(request: SingleSearch): Except = diff --git a/sql/src/main/scala/app/softnetwork/elastic/sql/time/package.scala b/sql/src/main/scala/app/softnetwork/elastic/sql/time/package.scala index 06951c6c4..556ab590b 100644 --- a/sql/src/main/scala/app/softnetwork/elastic/sql/time/package.scala +++ b/sql/src/main/scala/app/softnetwork/elastic/sql/time/package.scala @@ -99,7 +99,19 @@ package object time { } sealed trait TimeUnit extends PainlessScript with DateMathScript with DateMathRounding { - lazy val regex: Regex = s"\\b(?i)$sql(s)?\\b".r + + /** Accepted spellings of this unit, canonical first. The ODBC/JDBC interval names + * (`SQL_TSI_DAY`, …) are what a BI tool passes to `TIMESTAMPADD`/`TIMESTAMPDIFF`, so they are + * aliases of the units we already have rather than units of their own — the AST carries one + * spelling and the render normalises to `sql`. + * + * 🔴 `SQL_TSI_DAY` could never have matched the previous `\b(?i)DAY(s)?\b`: `_` is a word + * character, so there is no word boundary before `DAY` inside it. Adding the alias therefore + * cannot change what any existing statement parses to — it can only accept more. + */ + def words: List[String] = List(sql) + + lazy val regex: Regex = s"\\b(?i)(${words.mkString("|")})(s)?\\b".r def timeUnit: String = sql.toUpperCase() + "S" @@ -129,31 +141,39 @@ package object time { } case object YEARS extends Expr("YEAR") with CalendarUnit { + override def words: List[String] = List(sql, "SQL_TSI_YEAR") override def script: Option[String] = Some("y") } case object MONTHS extends Expr("MONTH") with CalendarUnit { + override def words: List[String] = List(sql, "SQL_TSI_MONTH") override def script: Option[String] = Some("M") } case object QUARTERS extends Expr("QUARTER") with CalendarUnit { + override def words: List[String] = List(sql, "SQL_TSI_QUARTER") override def script: Option[String] = throw new IllegalArgumentException( "Quarter must be converted to months (value * 3) before creating date-math" ) } case object WEEKS extends Expr("WEEK") with CalendarUnit { + override def words: List[String] = List(sql, "SQL_TSI_WEEK") override def script: Option[String] = Some("w") } case object DAYS extends Expr("DAY") with CalendarUnit with FixedUnit { + override def words: List[String] = List(sql, "SQL_TSI_DAY") override def script: Option[String] = Some("d") } case object HOURS extends Expr("HOUR") with FixedUnit { + override def words: List[String] = List(sql, "SQL_TSI_HOUR") override def script: Option[String] = Some("H") } case object MINUTES extends Expr("MINUTE") with FixedUnit { + override def words: List[String] = List(sql, "SQL_TSI_MINUTE") override def script: Option[String] = Some("m") } case object SECONDS extends Expr("SECOND") with FixedUnit { + override def words: List[String] = List(sql, "SQL_TSI_SECOND") override def script: Option[String] = Some("s") } diff --git a/sql/src/test/resources/corpus/epic-21-attribution.csv b/sql/src/test/resources/corpus/epic-21-attribution.csv index 322b76f3a..650601b21 100644 --- a/sql/src/test/resources/corpus/epic-21-attribution.csv +++ b/sql/src/test/resources/corpus/epic-21-attribution.csv @@ -75,7 +75,7 @@ "tableau.sql92.wx.007","rejected","rejected_pending_policy","rejected_pending_policy","Tableau temp-table capability probe; deliberately refused AT PARSE TIME by the recognise-to-reject CREATE [LOCAL | GLOBAL] TEMPORARY TABLE production, which names the construct and the reason instead of an unrelated combinator. MUST STAY REJECTED -- a flip means a fix went too far" "tableau.sql92.wx.008","parses","capability_open","capability_open","Tableau temp-table capability probe (DROP TABLE, quoted name); parses since story 21.7's uniform Parser.ident quoting -- excluded from scoring, the capability answer is an open product decision tracked outside Epic 21" "tableau.sql92.wx.009","parses","fixed","epic22","Tableau's row-count probe with its schema qualifier left on. Story 22.1 made it PARSE; story 22.7 pass 2 MEASURED that it also EXECUTES, which is what earns the score. Acceptance rows E5a (this statement, backticked) and E5b (the ANSI-quoted twin) in softclient4es-arrow's JoinExtensionIntegrationSpec return exactly one row with TblMax = 1 on real Elasticsearch 6.8, 7.17, 8.18 and 9.0; row E5c shows the same text is ordinary schema-qualified SQL to DuckDB; and the jdbc driver pins the same outcome over its own fixture with the qualifier read from the connection (JdbcIntegrationSpec, ""corpus E5-jdbc""). SETTLES the 22.4 review's prediction that a fully-quoted qualifier would fold into the index name and fail with index_not_found: it does not. The grammar keeps the qualifier in Table.parts and the bare last part as Table.name, so the read reaches the index -- measured, not argued. Pass 1 scored this row `residual` because nobody had run it; that is now superseded." -"tableau.sql92.wx.010","rejected","residual","local:select-top-n","bisected: the T-SQL TOP n clause is not in the grammar -- the same statement with LIMIT 1 parses" +"tableau.sql92.wx.010","parses","fixed","issue:361","the T-SQL TOP n clause is now in the grammar: SELECT TOP n is folded into the statement's LIMIT by Parser.single, so it renders as LIMIT n and re-parses. SCORED under the lead's correctness bar: BiDialectExecutionSpec asserts on real Elasticsearch, five clients, that TOP n returns exactly n rows and the SAME rows as the LIMIT n spelling, over an index of 24 documents. The integration mutation is recorded: dropping the fold while leaving TOP parsing reddens three rows." "tableau.sql92.wx.011","parses","fixed","epic21","rejected before Epic 21, parses now. Blocked ONLY by identifier quoting (21.1/21.2); it belongs to no silent-wrong-answer family, so PD-3 adds no obligation beyond the parse verdict." "tableau.sql92.wx.012","rejected","rejected_by_design","rejected_by_design","refused ON PURPOSE, permanently: the derived table carries NO CORRELATION NAME and the alias is mandatory (SQL-92 clause 7.6, story 22.1 PD-1). CORRECTION kept from the 2026-09-13 re-measurement: ROWNUM is NOT the blocker -- it resolves as an ordinary identifier and round-trips. That is the HAZARD rather than the refusal: were the mandatory-alias rule ever relaxed, this statement would parse and range-filter a field that does not exist, returning ZERO rows with HTTP 200 (the #205/#209/#224/#253 silent-wrong-answer family). Pinned in CODE as CorpusReplay.RejectedByDesignIds." "tableau.sql92.wx.013","parses","fixed","epic21","rejected before Epic 21, parses now. PD-3 family: aggregate-free GROUP BY with no LIMIT (#253) -- would otherwise have flipped from a loud rejection to a SILENT wrong answer; an ORDINAL GROUP BY / ORDER BY (#298) -- ORDER BY parsed before the epic and was silently DISCARDED. Correctness discharged by MERGED tests on real ES 6.8/7.17/8.18/9.0: GroupByCompletenessSpec ""GROUP BY with no aggregate"" (21.3) + the four ""corpus shape:"" tests (21.6); GroupByCompletenessSpec ""resolve an ordinal GROUP BY / ORDER BY to the n-th SELECT item (OQ-1)"" (21.3) + ""corpus shape: ... ORDER BY 1 ASC"" (21.6)." @@ -90,11 +90,11 @@ "tableau.sql92.w4.022","parses","fixed","epic21","rejected before Epic 21, parses now. Blocked ONLY by identifier quoting (21.1/21.2); it belongs to no silent-wrong-answer family, so PD-3 adds no obligation beyond the parse verdict." "tableau.sql92.w4.023","parses","fixed","epic21","rejected before Epic 21, parses now. Blocked ONLY by identifier quoting (21.1/21.2); it belongs to no silent-wrong-answer family, so PD-3 adds no obligation beyond the parse verdict." "tableau.sql92.w4.024","parses","pre_epic21","pre_epic21","parsed before Epic 21 (re-measured at ac54a079, Task 0) -- Epic 21 may not claim it" -"tableau.sql92.w4.025","rejected","residual","local:odbc-fn-escape-timestampadd","bisected: two independent blockers, neither quoting -- the ODBC escape {fn ...} is not in the grammar, and TIMESTAMPADD(unit,n,ts) is rejected even unbraced with a bare DAY unit" +"tableau.sql92.w4.025","rejected","residual","local:odbc-fn-escape-timestampadd","bisected: two independent blockers, neither quoting. The TIMESTAMPADD half is FIXED (it is now a spelling of DATETIME_ADD, and the ODBC SQL_TSI_ unit names are accepted); the row stays REJECTED on the remaining one, the ODBC escape {fn ...}, which is a pre-parse normalisation rather than a grammar production and has no owner yet." "tableau.sql92.w5.026","parses","fixed","epic21","rejected before Epic 21, parses now. PD-3 family: aggregate-free GROUP BY with no LIMIT (#253) -- would otherwise have flipped from a loud rejection to a SILENT wrong answer. Correctness discharged by MERGED tests on real ES 6.8/7.17/8.18/9.0: GroupByCompletenessSpec ""GROUP BY with no aggregate"" (21.3) + the four ""corpus shape:"" tests (21.6)." "tableau.sql92.w5.027","parses","fixed","epic21","rejected before Epic 21, parses now. Blocked ONLY by identifier quoting (21.1/21.2); it belongs to no silent-wrong-answer family, so PD-3 adds no obligation beyond the parse verdict." "tableau.sql92.w5.028","parses","fixed","epic21","rejected before Epic 21, parses now. PD-3 family: aggregate-free GROUP BY with no LIMIT (#253) -- would otherwise have flipped from a loud rejection to a SILENT wrong answer; an ORDINAL GROUP BY / ORDER BY (#298) -- ORDER BY parsed before the epic and was silently DISCARDED. Correctness discharged by MERGED tests on real ES 6.8/7.17/8.18/9.0: GroupByCompletenessSpec ""GROUP BY with no aggregate"" (21.3) + the four ""corpus shape:"" tests (21.6); GroupByCompletenessSpec ""resolve an ordinal GROUP BY / ORDER BY to the n-th SELECT item (OQ-1)"" (21.3) + ""corpus shape: ... ORDER BY 1 ASC"" (21.6)." "tableau.sql92.w8.029","parses","fixed","epic21","rejected before Epic 21, parses now. Blocked ONLY by identifier quoting (21.1/21.2); it belongs to no silent-wrong-answer family, so PD-3 adds no obligation beyond the parse verdict." "tableau.sql92.w8.030","parses","fixed","epic21","rejected before Epic 21, parses now. PD-3 family: aggregate-free GROUP BY with no LIMIT (#253) -- would otherwise have flipped from a loud rejection to a SILENT wrong answer. Correctness discharged by MERGED tests on real ES 6.8/7.17/8.18/9.0: GroupByCompletenessSpec ""GROUP BY with no aggregate"" (21.3) + the four ""corpus shape:"" tests (21.6)." "tableau.sql92.w8.031","parses","fixed","epic21","rejected before Epic 21, parses now. PD-3 family: aggregate-free GROUP BY with no LIMIT (#253) -- would otherwise have flipped from a loud rejection to a SILENT wrong answer. Correctness discharged by MERGED tests on real ES 6.8/7.17/8.18/9.0: GroupByCompletenessSpec ""GROUP BY with no aggregate"" (21.3) + the four ""corpus shape:"" tests (21.6)." -"tableau.sql92.w8.032","rejected","residual","local:char-length-spelling","bisected: CHAR_LENGTH/CHARACTER_LENGTH are not accepted function spellings (LENGTH and LEN are); every other clause of this statement parses" +"tableau.sql92.w8.032","parses","fixed","issue:361","CHAR_LENGTH/CHARACTER_LENGTH are now accepted spellings of LENGTH (MySQL 8.4 and PostgreSQL 16 both document them). SCORED under the lead's correctness bar: BiDialectExecutionSpec asserts on real Elasticsearch, five clients, that it counts CHARACTERS and not bytes -- measured against a multi-byte string, so a byte-counting implementation reddens -- and that it agrees with LENGTH on every name, including inside the SUM(...) this statement uses." diff --git a/sql/src/test/resources/corpus/series.csv b/sql/src/test/resources/corpus/series.csv index 34ea3b65d..d0dad55f7 100644 --- a/sql/src/test/resources/corpus/series.csv +++ b/sql/src/test/resources/corpus/series.csv @@ -2,3 +2,4 @@ "pre-epic21","ac54a079","2026-09-13","12","12","12","87" "pre-epic22","40c8c63e","2026-09-17","56","56","81","18" "post-epic22","7187c7d9","2026-09-18","70","70","91","8" +"tableau-dialect","PENDING","2026-09-18","72","72","93","6" 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 a839bd61a..33caf1ec7 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 @@ -194,12 +194,13 @@ class CorpusReplaySpec extends AnyFlatSpec with Matchers { // `Epic22UnmeasuredIds`), which is the state PD-3 exists to make representable. if ( a.scored == "fixed" && !Set("epic21", "epic22").contains(a.owner) && - !Issue328FixedIds.contains(a.captureId) + !Issue328FixedIds.contains(a.captureId) && !Issue361FixedIds.contains(a.captureId) ) { sys.error( s"scored=fixed requires owner=epic21 or owner=epic22, not '${a.owner}' -- an " + "issue-owned or capability-open row that merely PARSES is not a fix (PD-3). The ONE " + - "exception is enumerated in CorpusReplay.Issue328FixedIds and is not a predicate you " + + "exceptions are enumerated in CorpusReplay.Issue328FixedIds / Issue361FixedIds and " + + "are not a predicate you " + "may widen: add an id there only with a merged suite asserting that statement's " + "CORRECTNESS, named in its note" ) @@ -384,10 +385,10 @@ class CorpusReplaySpec extends AnyFlatSpec with Matchers { "the number of SCORED FIXES moved -- if that is intended, move this pin too, and say " + "so in the PR: it is the published headline. " ) { - attribution.values.count(_.scored == "fixed") shouldBe 58 + attribution.values.count(_.scored == "fixed") shouldBe 60 } withClue("the published headline moved: ") { - tallyOf(outcomes, attribution).scoredOf99 shouldBe 70 + tallyOf(outcomes, attribution).scoredOf99 shouldBe 72 } // The ONE enumerated exception to "fixed belongs to a shipped epic" (lead ruling 2026-09-17), // checked in BOTH directions: an id the CODE excepts that the table no longer scores is a @@ -407,6 +408,20 @@ class CorpusReplaySpec extends AnyFlatSpec with Matchers { sys.error(s"owner moved to '${a.owner}' -- the exception is keyed on issue:328's rows") } } + + Issue361FixedIds should have size 2 + checkAll(Issue361FixedIds.toList.sorted, "the issue:361 scoring exception")(identity) { id => + val a = attributionOf(attribution, id) + if (a.scored != "fixed") { + sys.error( + s"the code excepts '$id' from G7 but the table scores it '${a.scored}' -- a DEAD " + + "exception. Delete the id from Issue361FixedIds, or score the row." + ) + } + if (a.owner != "issue:361") { + sys.error(s"expected owner 'issue:361' for '$id', found '${a.owner}'") + } + } // The owner check is done per OWNER, not per set: `epic22` owns two sets (fixed + unmeasured), // so comparing the table's `epic22` rows against either set alone reports the other set's rows // as unpinned. @@ -839,6 +854,28 @@ object CorpusReplay { "tableau.sql92.wx.015" ) + /** The SECOND enumerated exception, same bar and same shape as `Issue328FixedIds` (issue #361). + * + * Two Tableau statements rejected for a DIALECT SPELLING rather than a missing capability: a + * `SUM(CHAR_LENGTH(name))` and a `SELECT TOP 1 *`. Their correctness — not their parse — is + * asserted by a merged, five-client suite against real Elasticsearch, `BiDialectExecutionSpec`: + * `CHAR_LENGTH` is measured against a MULTI-BYTE string (`海豚` is 2 characters and 6 UTF-8 bytes, + * so a byte-counting implementation reddens), and `TOP n` is asserted on row COUNT and row SET + * against the `LIMIT n` spelling over an index holding more documents than `n`. + * + * 🔴 The integration mutation was run and recorded: removing the fold of `TOP` into the + * statement's `LIMIT` — while leaving `TOP` parsing — reddens exactly the three `TOP` execution + * rows and leaves the `CHAR_LENGTH` rows green. A suite that stayed green under that mutation + * would not have met the bar, whatever its name. + * + * Same rule as above: a COMPILED SINGLETON, never `owner.startsWith("issue:")`. If it is ever + * wrong, re-score the rows `residual` and DELETE the ids — do not widen the set. + */ + val Issue361FixedIds: Set[String] = Set( + "tableau.sql92.w8.032", + "tableau.sql92.wx.010" + ) + /** Rejected by a blocker Epic 22 does not own: the MySQL null-safe equality operator `<=>` in the * JOIN `ON`. The derived table itself parses (story 22.1's control asserts the `=` spelling of * the same statement parses, so `<=>` is the only variable). diff --git a/sql/src/test/scala/app/softnetwork/elastic/sql/census/DialectCensus.scala b/sql/src/test/scala/app/softnetwork/elastic/sql/census/DialectCensus.scala index a24198b1f..9732e5011 100644 --- a/sql/src/test/scala/app/softnetwork/elastic/sql/census/DialectCensus.scala +++ b/sql/src/test/scala/app/softnetwork/elastic/sql/census/DialectCensus.scala @@ -1213,7 +1213,7 @@ object DialectCensus { "LENGTH", "LEN", FS, - """override lazy val words: List[String] = List(sql, "LEN")""", + """override lazy val words: List[String] = List(sql, "CHARACTER_LENGTH", "CHAR_LENGTH", "LEN")""", "SELECT LEN(name) AS l FROM emp", "1", EsSpecific, @@ -1222,6 +1222,39 @@ object DialectCensus { PainlessField, "alias spelling of LENGTH" ), + e( + "fn.string.length.char-length-alias", + Fn, + "LENGTH", + "CHAR_LENGTH", + FS, + """override lazy val words: List[String] = List(sql, "CHARACTER_LENGTH", "CHAR_LENGTH", "LEN")""", + "SELECT CHAR_LENGTH(name) AS l FROM emp", + "1", + AnsiAdjacent, + s"MySQL 8.4: CHAR_LENGTH() 'Return number of characters in argument' - $MyStr ; " + + s"PostgreSQL 16: char_length ( text ) -> integer - $PgStr", + PainlessField, + "alias spelling of LENGTH; the SQL-92 name, and what Tableau emits for its LEN() " + + "calculated field. Counts CHARACTERS, which is what LENGTH already does here - MySQL's " + + "own LENGTH counts BYTES, so CHAR_LENGTH is the spelling that agrees with us" + ), + e( + "fn.string.length.character-length-alias", + Fn, + "LENGTH", + "CHARACTER_LENGTH", + FS, + """override lazy val words: List[String] = List(sql, "CHARACTER_LENGTH", "CHAR_LENGTH", "LEN")""", + "SELECT CHARACTER_LENGTH(name) AS l FROM emp", + "1", + AnsiAdjacent, + s"MySQL 8.4: CHARACTER_LENGTH() 'Synonym for CHAR_LENGTH()' - $MyStr ; " + + s"PostgreSQL 16: character_length ( text ) -> integer - $PgStr", + PainlessField, + "alias spelling of LENGTH; listed BEFORE CHAR_LENGTH in `words` only as house style " + + "(longest first) - neither is a prefix of the other, so the order is not load-bearing here" + ), e( "fn.string.replace", Fn, @@ -1615,13 +1648,30 @@ object DialectCensus { "DuckDB documents date_diff(part, startdate, enddate) with this exact order (datepart " + "page fetched)" ), + e( + "fn.time.date-diff.timestampdiff-alias", + Fn, + "DATE_DIFF", + "TIMESTAMPDIFF", + FT, + """override lazy val words: List[String] = List(sql, "TIMESTAMPDIFF", "DATEDIFF")""", + "SELECT TIMESTAMPDIFF(MONTH, start_date, end_date) AS d FROM projects", + "3", + EsSpecific, + "ES painless ChronoUnit.between. TIMESTAMPDIFF is the ODBC/JDBC spelling; MySQL 8.4 " + + "defines TIMESTAMPDIFF(unit, dt1, dt2) as dt2 - dt1, and date_diff_transact_sql binds " + + "(unit, d1, d2) to DateDiff(start = d1, end = d2) = between(d1, d2) - the same answer " + + "with the same sign, which is why the MySQL name is safe to accept here (T1)", + PainlessField, + "alias spelling of DATE_DIFF, unit-first form only" + ), e( "fn.time.date-diff.datediff-alias", Fn, "DATE_DIFF", "DATEDIFF", FT, - """override lazy val words: List[String] = List(sql, "DATEDIFF")""", + """override lazy val words: List[String] = List(sql, "TIMESTAMPDIFF", "DATEDIFF")""", "SELECT DATEDIFF(start_date, end_date) AS d FROM projects", "2..3", EsSpecific, @@ -1843,7 +1893,7 @@ object DialectCensus { "DATETIME_ADD", "DATETIMEADD", FT, - """override lazy val words: List[String] = List(sql, "DATETIMEADD")""", + """override lazy val words: List[String] = List(sql, "DATETIMEADD", "TIMESTAMPADD")""", "SELECT DATETIMEADD(updated_at, INTERVAL 2 HOUR) AS d FROM events", "2", EsSpecific, @@ -1851,6 +1901,23 @@ object DialectCensus { "(T1)", PainlessField ), + e( + "fn.time.datetime-add.timestampadd-alias", + Fn, + "DATETIME_ADD", + "TIMESTAMPADD", + FT, + """override lazy val words: List[String] = List(sql, "DATETIMEADD", "TIMESTAMPADD")""", + "SELECT TIMESTAMPADD(DAY, -89, updated_at) AS d FROM events", + "3", + EsSpecific, + "ES date-math / painless plus. TIMESTAMPADD is the ODBC/JDBC scalar-function spelling a " + + "BI tool emits; MySQL 8.4 documents TIMESTAMPADD(unit, interval, datetime_expr) with the " + + "SAME unit-first order this parser already accepted for DATE_ADD/DATETIME_ADD, so the " + + "alias adds a name and no semantics (T1)", + PainlessField, + "alias spelling of DATETIME_ADD, unit-first form only" + ), e( "fn.time.datetime-sub", Fn, @@ -3052,6 +3119,23 @@ object DialectCensus { "maps to size; the standard's FETCH FIRST spelling does not parse here. Above " + "max_result_window the engine routes through bounded scroll paging (issue #224)" ), + e( + "clause.limit.top-n", + Clause, + "TOP", + "TOP", + QS, + """case object Top extends Expr("TOP") with TokenRegex""", + "SELECT TOP 10 id FROM emp", + "1", + EsSpecific, + "maps to the same ES `size` as LIMIT, because the parser folds it into the statement's " + + "LIMIT rather than carrying a second row bound. T-SQL's spelling, which Tableau emits in " + + "its SQL-92 dialect; no PD-3 trio engine accepts it (T1)", + RequestShape, + "renders as LIMIT n, never as TOP n. Combining TOP with LIMIT is refused by name. TOP is " + + "NOT reserved: `SELECT top FROM t` still reads a column called top" + ), e( "clause.limit.offset", Clause, diff --git a/sql/src/test/scala/app/softnetwork/elastic/sql/census/DialectCensusSpec.scala b/sql/src/test/scala/app/softnetwork/elastic/sql/census/DialectCensusSpec.scala index 4742047ce..8fbf1e016 100644 --- a/sql/src/test/scala/app/softnetwork/elastic/sql/census/DialectCensusSpec.scala +++ b/sql/src/test/scala/app/softnetwork/elastic/sql/census/DialectCensusSpec.scala @@ -327,8 +327,10 @@ class DialectCensusSpec extends AnyFlatSpec with Matchers { val registryWords: Set[String] = SQLKeywords.functionTokens.flatMap(SQLKeywords.wordsOf).toSet - withClue(s"accepted function spellings moved from 149 to ${registryWords.size} (F-2/F-5)\n") { - registryWords.size shouldBe 149 + withClue(s"accepted function spellings moved from 153 to ${registryWords.size} (F-2/F-5)\n") { + // 149 -> 153: CHAR_LENGTH + CHARACTER_LENGTH on Length, TIMESTAMPADD on DateTimeAdd, + // TIMESTAMPDIFF on DateDiff - the ODBC/SQL-92 spellings a BI tool emits. + registryWords.size shouldBe 153 } // Three of the nine genuinely un-greppable spellings, named, so a failure says WHAT diff --git a/sql/src/test/scala/app/softnetwork/elastic/sql/parser/TableauDialectSpec.scala b/sql/src/test/scala/app/softnetwork/elastic/sql/parser/TableauDialectSpec.scala new file mode 100644 index 000000000..5263e64ac --- /dev/null +++ b/sql/src/test/scala/app/softnetwork/elastic/sql/parser/TableauDialectSpec.scala @@ -0,0 +1,216 @@ +/* + * 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.parser + +import org.scalatest.flatspec.AnyFlatSpec +import org.scalatest.matchers.should.Matchers + +/** The dialect spellings a BI tool emits that the grammar did not accept. + * + * Sourced from the Epic 19 capture (`sql/src/test/resources/corpus/epic-21-attribution.csv`), + * where each of these was a `residual` row with a `local:` owner and a bisected cause. + * + * Every accepted spelling is asserted to be an ALIAS, not a second mechanism: the render + * normalises to the canonical spelling and that render RE-PARSES. Asserting `isRight` alone would + * be satisfied by a grammar that accepted the word and then produced something it could not read + * back — the failure mode story 21.7 found in `CreateMaterializedView.sql`. + * + * The rejection assertions additionally rule out `Parser.apply`'s `NonFatal` boundary catch: a + * restored `throw` yields a `Left` CONTAINING the same reason, so `isLeft` plus a substring match + * is satisfied by the defect as well as by the fix (story 21.4's lesson). + */ +class TableauDialectSpec extends AnyFlatSpec with Matchers { + + private def canonicalises(sql: String, to: String): Unit = + withClue(s"[$sql] ") { + val parsed = Parser(sql) + parsed.isRight shouldBe true + val rendered = parsed.map(_.sql).getOrElse("") + rendered should include(to) + withClue(s"render [$rendered] must re-parse - ") { + Parser(rendered).map(_.sql) shouldBe Right(rendered) + } + } + + private def refuses(sql: String, because: String): Unit = + withClue(s"[$sql] ") { + noException should be thrownBy Parser(sql) + val reason = Parser(sql).left.map(_.toString).left.getOrElse("") + reason should include(because) + reason should not include "Internal parser error" + } + + // ---------------------------------------------------------------- CHAR_LENGTH + + "CHAR_LENGTH" should "be accepted as a spelling of LENGTH, and render as LENGTH" in { + canonicalises("SELECT CHAR_LENGTH(name) AS n FROM t", "LENGTH(name)") + canonicalises("SELECT CHARACTER_LENGTH(name) AS n FROM t", "LENGTH(name)") + } + + it should "still let LENGTH and LEN through, LENGTH first so LEN cannot eat its prefix" in { + canonicalises("SELECT LENGTH(name) AS n FROM t", "LENGTH(name)") + canonicalises("SELECT LEN(name) AS n FROM t", "LENGTH(name)") + } + + it should "work where the capture actually used it - inside an aggregate" in { + // tableau.sql92.w8.032: SUM(CHAR_LENGTH("bi_events"."name")) + canonicalises( + """SELECT SUM(CHAR_LENGTH("name")) AS "sum_ok" FROM "bi_events"""", + "LENGTH(" + ) + } + + // ------------------------------------------------------------- ODBC functions + + "TIMESTAMPADD" should "be a spelling of DATETIME_ADD, in the ODBC (unit, count, base) order" in { + canonicalises("SELECT TIMESTAMPADD(DAY, -89, event_ts) AS d FROM t", "DATETIME_ADD(DAY, -89,") + canonicalises("SELECT TIMESTAMPADD(MONTH, 2, event_ts) AS d FROM t", "DATETIME_ADD(MONTH, 2,") + + } + + it should "accept the ODBC SQL_TSI_ interval names for every unit we already have" in { + Seq("YEAR", "MONTH", "QUARTER", "WEEK", "DAY", "HOUR", "MINUTE", "SECOND").foreach { unit => + canonicalises( + s"SELECT TIMESTAMPADD(SQL_TSI_$unit, 1, event_ts) AS d FROM t", + s"DATETIME_ADD($unit, 1," + ) + } + } + + it should "accept CURRENT_DATE as the base, which is the shape Tableau emits" in { + // tableau.sql92.w4.025, minus the {fn ...} escape it is wrapped in. + canonicalises( + "SELECT a FROM t WHERE ts >= TIMESTAMPADD(SQL_TSI_DAY, -89, CURRENT_DATE)", + "DATETIME_ADD(DAY, -89, CURRENT_DATE)" + ) + } + + /** MySQL 8.4 defines `TIMESTAMPDIFF(unit, dt1, dt2)` as `dt2 - dt1`, and `date_diff_transact_sql` + * binds `(unit, d1, d2)` to `DateDiff(start = d1, end = d2)`, i.e. `between(d1, d2)` — the same + * answer with the same sign. The MySQL/ODBC order is the one that matters, so it is the one + * asserted. + */ + "TIMESTAMPDIFF" should "be a spelling of DATE_DIFF, in the MySQL/ODBC unit-first order" in { + // 🔴 The ARGUMENT LIST, not just the function name: `include("DATE_DIFF(")` is satisfied by + // the sign-inverted render `DATE_DIFF(MONTH, end_date, start_date)` too, so it would prove + // nothing about the very binding this test exists to pin. + canonicalises( + "SELECT TIMESTAMPDIFF(MONTH, start_date, end_date) AS d FROM t", + "DATE_DIFF(MONTH, start_date, end_date)" + ) + canonicalises("SELECT TIMESTAMPDIFF(DAY, a, b) AS d FROM t", "DATE_DIFF(DAY, a, b)") + } + + it should "leave the bare unit names alone" in { + // The alias must not have widened what INTERVAL accepts, nor narrowed it. + canonicalises("SELECT event_ts + INTERVAL 1 DAY AS d FROM t", "INTERVAL 1 DAY") + canonicalises("SELECT event_ts + INTERVAL 2 MONTHS AS d FROM t", "INTERVAL 2 MONTH") + } + + // --------------------------------------------------------------------- TOP n + + "SELECT TOP n" should "bind the rows, and render as the LIMIT it is" in { + // tableau.sql92.wx.010 + canonicalises("SELECT TOP 1 * FROM t", "LIMIT 1") + canonicalises("""SELECT TOP 1 * FROM "elastic"."bi_events"""", "LIMIT 1") + canonicalises("SELECT TOP 10 a, b FROM t WHERE a = 1 ORDER BY b DESC", "LIMIT 10") + } + + it should "produce exactly the same statement as the LIMIT spelling" in { + Parser("SELECT TOP 5 a FROM t").map(_.sql) shouldBe Parser("SELECT a FROM t LIMIT 5").map(_.sql) + } + + it should "leave a column named `top` alone - TOP is not reserved" in { + canonicalises("SELECT top FROM t", "SELECT top FROM t") + canonicalises("SELECT top AS x FROM t", "SELECT top AS x FROM t") + canonicalises("SELECT top, a FROM t", "SELECT top, a FROM t") + canonicalises("SELECT a FROM t WHERE top = 1", "top = 1") + } + + it should "refuse TOP together with LIMIT rather than silently preferring one" in { + refuses("SELECT TOP 5 a FROM t LIMIT 10", "TOP and LIMIT") + } + + /** 🔴 This is the regression an independent review caught, and the reason `top` uses `failure` + * and not `err`. `SELECT top -1 AS x FROM t` reads `top - 1` and parses on `main`. A named + * "non-negative row count" refusal made it a HARD error, because `Parsers.|` never tries another + * alternative after an `Error`, so `opt` could not backtrack to the column reading. + * + * The control is the same statement with a different identifier: whatever `foo` does, `top` must + * do. Asserting only that `top` parses would pass against a grammar that read it as something + * else entirely. + */ + it should "read `top` as a column when what follows is not a row count" in { + Seq("SELECT top -1 AS x FROM t", "SELECT top-1 AS x FROM t", "SELECT top -1 FROM t").foreach { + sql => + withClue(s"[$sql] ") { + val control = Parser(sql.replace("top", "foo")).map(_.sql.replace("foo", "top")) + Parser(sql).map(_.sql) shouldBe control + } + } + } + + it should "not silently wrap a row count that does not fit an Int" in { + // `Limit` holds an Int. Checking the sign on the Long and then calling `.toInt` would turn + // TOP 2147483648 into LIMIT -2147483648 -- a guard defeated one line after it is written. + Seq("SELECT TOP 2147483648 a FROM t", "SELECT TOP 99999999999999 a FROM t").foreach { sql => + withClue(s"[$sql] -> ${Parser(sql).map(_.sql)} ") { + Parser(sql).map(_.sql).getOrElse("") should not include "LIMIT -" + Parser(sql).map(_.sql).getOrElse("") should not include "LIMIT 276447231" + } + } + } + + /** `TOP n PERCENT` is real T-SQL that this engine does not implement. Before it was refused by + * name, `top` consumed `TOP 5` and `field` then read `PERCENT a` as the column `PERCENT` aliased + * to `a`: `SELECT TOP 5 PERCENT a FROM t` answered with rows for a column nobody named. A loud + * rejection had become a silent wrong answer. + */ + it should "refuse TOP n PERCENT by name rather than inventing a column called PERCENT" in { + refuses("SELECT TOP 5 PERCENT a FROM t", "PERCENT") + refuses("SELECT TOP 5 PERCENT a, b FROM t", "PERCENT") + refuses("SELECT TOP 5 WITH TIES a FROM t", "TIES") + Parser("SELECT TOP 5 PERCENT a FROM t").map(_.sql).getOrElse("") should not include "PERCENT AS" + } + + it should "accept the parenthesised T-SQL spelling" in { + canonicalises("SELECT TOP (5) a FROM t", "LIMIT 5") + } + + it should "apply to the FROM-less form too" in { + canonicalises("SELECT TOP 1 1", "LIMIT 1") + refuses("SELECT TOP 1 1 LIMIT 2", "TOP and LIMIT") + } + + // ------------------------------------------- watcher comparison operator order + + "A watcher condition" should "accept >= and <=, not just > and <" in { + def watcher(op: String): String = + s"""CREATE WATCHER w AS + | EVERY 1 MINUTE + | FROM metrics WHERE cpu_usage > 90 WITHIN 5 MINUTES + | WHEN ctx.payload.hits.total $op 0 DO + | alert LOG 'hi' AT ERROR + |END""".stripMargin + + // `>` and `<` were the controls that passed while `>=` and `<=` did not: the literal `>` + // matched the first character of `>=` and left `=` for the value production. + Seq(">", ">=", "<", "<=", "=", "<>", "!=").foreach { op => + withClue(s"[WHEN ... $op 0] ") { Parser(watcher(op)).isRight shouldBe true } + } + } +} diff --git a/testkit/src/main/scala/app/softnetwork/elastic/client/BiDialectExecutionSpec.scala b/testkit/src/main/scala/app/softnetwork/elastic/client/BiDialectExecutionSpec.scala new file mode 100644 index 000000000..180359824 --- /dev/null +++ b/testkit/src/main/scala/app/softnetwork/elastic/client/BiDialectExecutionSpec.scala @@ -0,0 +1,209 @@ +/* + * Copyright 2025 SOFTNETWORK + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package app.softnetwork.elastic.client + +import akka.NotUsed +import akka.actor.ActorSystem +import akka.stream.scaladsl.Source +import app.softnetwork.elastic.client.bulk._ +import app.softnetwork.elastic.client.result.{ElasticFailure, ElasticSuccess} +import app.softnetwork.elastic.client.spi.ElasticClientFactory +import app.softnetwork.elastic.scalatest.ElasticDockerTestKit +import app.softnetwork.elastic.sql.query.SelectStatement +import app.softnetwork.persistence.generateUUID +import org.scalatest.flatspec.AnyFlatSpecLike +import org.scalatest.matchers.should.Matchers +import org.slf4j.{Logger, LoggerFactory} + +import scala.collection.immutable.ListMap +import scala.language.implicitConversions + +/** The BI dialect spellings, EXECUTED — issues #361 / #362. + * + * 🔴 This spec exists because `CorpusReplaySpec` scores a row `fixed` only against *"a merged + * suite asserting that statement's CORRECTNESS"*, and a parse test is not that. The two corpus + * rows this covers (`tableau.sql92.w8.032`, `tableau.sql92.wx.010`) parsed as soon as the + * spellings were accepted; whether they ANSWER is a different question, and it is the one the + * scoreboard actually publishes. Parse is not an answer (PD-3). + * + * Each assertion is built so that accepting the spelling is NOT enough to pass it: + * + * - `CHAR_LENGTH` is asserted against a MULTI-BYTE string. `'海豚'` is 2 characters and 6 UTF-8 + * bytes, so a byte-counting implementation — which is what MySQL's own `LENGTH` does, and the + * reason `CHAR_LENGTH` is the spelling that agrees with us — answers 6 and reddens. Asserting + * only against ASCII would be satisfied by either. + * - `TOP n` is asserted on ROW COUNT and on the ROW SET, against the `LIMIT n` spelling of the + * same statement, over an index with more documents than `n` AND more than Elasticsearch's + * default of 10. A `TOP` that was parsed and then dropped returns every row and reddens. + */ +trait BiDialectExecutionSpec extends AnyFlatSpecLike with ElasticDockerTestKit with Matchers { + + lazy val log: Logger = LoggerFactory.getLogger(getClass.getName) + + implicit val system: ActorSystem = ActorSystem(generateUUID()) + + lazy val client: ElasticClientApi = ElasticClientFactory.create(elasticConfig) + + private val index = "bi_dialect" + + /** 24 documents on 3 shards — above the default 10 hits, so a dropped bound is visible. */ + private val docCount = 24 + + /** name -> its CHARACTER count. `海豚` is the load-bearing row: 2 characters, 6 UTF-8 bytes. + * + * 🔴 None of these may be `padding`, the name the 24 bulk documents carry: a collision would + * make 25 documents match `name = 'padding'` and quietly change every count oracle below. + */ + private val names: Map[String, Int] = + Map("grace" -> 5, "海豚" -> 2, "café" -> 4, "ada" -> 3, "" -> 0) + + override def beforeAll(): Unit = { + super.beforeAll() + val settings = """{"number_of_shards": 3, "number_of_replicas": 0}""" + val mapping = + """{ + | "properties": { + | "id": { "type": "keyword" }, + | "name": { "type": "keyword" }, + | "amount": { "type": "integer" } + | } + |}""".stripMargin + client.createIndex(index, settings = settings).get shouldBe true + client.setMapping(index, mapping).get shouldBe true + + val docs = (1 to docCount).map { i => + s"""{"id":"doc_$i","name":"padding","amount":$i}""" + }.toList ++ names.keys.map { n => + s"""{"id":"name_${n.hashCode}","name":"$n","amount":0}""" + }.toList + + implicit val bulkOptions: BulkOptions = BulkOptions(defaultIndex = index, logEvery = 1000) + implicit def listToSource[T](list: List[T]): Source[T, NotUsed] = + Source.fromIterator(() => list.iterator) + + client.bulk[String](docs, identity, idKey = Some(Set("id"))) match { + case ElasticSuccess(_) => // ok + case ElasticFailure(error) => + error.cause.foreach(_.printStackTrace()) + fail(s"Bulk indexing failed: ${error.message}") + } + client.refresh(index) + () + } + + override def afterAll(): Unit = { + client.deleteIndex(index) + super.afterAll() + } + + private def rowsOf(sql: String): Seq[ListMap[String, Any]] = { + implicit val ctx: ConversionContext = NativeContext + client.search(SelectStatement(sql)) match { + case ElasticSuccess(response) => response.results + case ElasticFailure(error) => fail(s"[$sql] failed: ${error.message}") + } + } + + /** A `script_fields` value comes back wrapped in Elasticsearch's per-field ARRAY on the row path + * (`n -> List(3)`), while the same expression under an aggregation comes back as a scalar. That + * asymmetry is pre-existing and documented against issue #209; it is not what this spec tests, + * so it is unwrapped here rather than asserted around. + */ + private def longAt(row: ListMap[String, Any], key: String): Long = row(key) match { + case n: Number => n.longValue() + case Seq(n: Number) => n.longValue() + case (n: Number) :: Nil => n.longValue() + case other => fail(s"expected a number at '$key', got ${other.getClass}: $other") + } + + // ───────────────────────────────── CHAR_LENGTH (issue #361) ───────────────────────────────── + + "CHAR_LENGTH" should "count CHARACTERS, not bytes, on a real index" in { + names.foreach { case (name, expected) => + val rows = rowsOf( + s"SELECT CHAR_LENGTH(name) AS n FROM $index WHERE id = 'name_${name.hashCode}'" + ) + withClue(s"CHAR_LENGTH('$name') (${name.getBytes("UTF-8").length} UTF-8 bytes) ") { + rows should have size 1L + longAt(rows.head, "n") shouldBe expected.toLong + } + } + } + + it should "answer exactly what LENGTH answers, for every name" in { + names.keys.foreach { name => + val where = s"WHERE id = 'name_${name.hashCode}'" + val viaAlias = rowsOf(s"SELECT CHAR_LENGTH(name) AS n FROM $index $where") + val viaCanonical = rowsOf(s"SELECT LENGTH(name) AS n FROM $index $where") + withClue(s"['$name'] ") { viaAlias shouldBe viaCanonical } + } + } + + it should "work inside an aggregate, which is the shape the capture used" in { + // tableau.sql92.w8.032 wraps it in SUM(...). 24 documents named `padding`, 7 characters each. + val rows = rowsOf(s"SELECT SUM(CHAR_LENGTH(name)) AS total FROM $index WHERE name = 'padding'") + rows should have size 1L + longAt(rows.head, "total") shouldBe (docCount * 7).toLong + } + + // ─────────────────────────────────── SELECT TOP n (issue #361) ─────────────────────────────── + + "SELECT TOP n" should "return exactly n rows, not every row" in { + Seq(1, 5, 7).foreach { n => + withClue(s"TOP $n ") { + rowsOf(s"SELECT TOP $n id FROM $index WHERE name = 'padding'") should have size n.toLong + } + } + } + + it should "return the same rows as the LIMIT spelling of the same statement" in { + Seq(1, 5, 7).foreach { n => + val viaTop = + rowsOf(s"SELECT TOP $n id FROM $index WHERE name = 'padding' ORDER BY amount ASC") + val viaLimit = + rowsOf(s"SELECT id FROM $index WHERE name = 'padding' ORDER BY amount ASC LIMIT $n") + withClue(s"TOP $n vs LIMIT $n ") { viaTop shouldBe viaLimit } + } + } + + /** The control that makes the two assertions above mean something: without a bound the same + * statement returns all 24 documents. A `TOP` that parsed and was then dropped would return this + * number instead of `n`, and 24 > Elasticsearch's default page of 10, so the difference cannot + * be an artefact of the default. + */ + it should "differ from the unbounded statement, which returns every document" in { + rowsOf(s"SELECT id FROM $index WHERE name = 'padding'") should have size docCount.toLong + } + + it should "bind the rows for the exact captured statement shape" in { + // tableau.sql92.wx.010: SELECT TOP 1 * FROM "elastic"."bi_events" + rowsOf(s"""SELECT TOP 1 * FROM "$index"""") should have size 1L + } + + // ─────────────────────────── TIMESTAMPADD / CHAR_LENGTH round trip ─────────────────────────── + + "TIMESTAMPADD" should "execute as DATETIME_ADD does, on the same statement" in { + val viaAlias = rowsOf( + s"SELECT TIMESTAMPADD(DAY, 1, CURRENT_DATE) AS d FROM $index WHERE name = 'padding' LIMIT 1" + ) + val viaCanonical = rowsOf( + s"SELECT DATETIME_ADD(DAY, 1, CURRENT_DATE) AS d FROM $index WHERE name = 'padding' LIMIT 1" + ) + viaAlias should have size 1L + viaAlias shouldBe viaCanonical + } +}