Repository navigation
Story 21.5 — literal escaping and the CAST/CONVERT target-type surface - #307
Merged
Merged
Conversation
Story 21.5, part A (#274) — plus the double-quoted twin the lead folded in at the dev kickoff (OQ-2), which story 21.1 left unowned. Doubling was unsupported ANYWHERE in the grammar, so any literal containing an apostrophe — a surname, `don't`, a French label — was a parse error on every surface: BI-generated SQL, analyst-typed SQL, the REPL, the drivers, DML VALUES, DDL defaults and SCRIPT AS bodies. There is exactly ONE string-literal production and every literal position funnels into it, so one regex closes all thirteen measured shapes. The three content alternatives (`[^'\\]`, `\\.`, `''`) are disjoint on their first character, so the quantifier stays deterministic. Nothing that parsed before can change meaning: the content alternatives cannot cross a LONE quote, so the only place the widened regex reaches further is across a DOUBLED quote — and two adjacent literals with no separator were never a valid parse here. The `""` half is a pure widening for the same reason (`WHERE a = "x""y"` was measured REJECT on origin/main) and it flips no reading: `quotedNameRegex` has accepted the doubled escape since 21.1, so `SELECT "a""b" FROM t` already parsed as the column `a"b`, and which production claims a quoted lexeme is still decided by ordering — `quotedIdentifier(UnlessArithmetic)` first in name positions, `literal` first in value positions. Pinned as a disambiguation pair. The render is deliberately unchanged (AD-2): `SELECT 'O''Brien'` re-renders `'O\'Brien'` and re-parses to an EQUAL AST. An ANSI-doubling render would also have to stop escaping backslashes, and `StringValue("C:\\").sql` would then emit `'C:\'`, which this grammar rejects — an un-reparseable render. The three quote-aware scanners (Parser.normalize, Parser.scriptBody, GatewayApi.splitStatements) needed ZERO logic edits, and the reason is structural rather than lucky: the first quote of a doubled pair closes the run and the second, immediately adjacent, re-opens it, so the window in which a scanner is outside a quoted run while the grammar is inside it is ZERO characters wide. Regression tests pin that in both modules; their stale comments quoting the old regex are corrected in the same commit. AD-12, measured both ways: `unescapeStringLiteral` is NOT folded onto `Parser.unquoteName`, contradicting a merge note 21.1 left in-source for this story. The escape ALPHABETS differ — an identifier un-escapes a backslash before ANY character (`SELECT "a\nb"` is the column `anb`), a literal only before its own delimiter or another backslash (`'a\nb'` keeps the backslash, which COPY INTO paths depend on). Folding would have silently changed what a quoted identifier means. The note now records the divergence instead of prescribing the fold. Hygiene: CopyInto.sql stops hand-inlining a fourth, byte-identical copy of the escape rule and calls escapeStringLiteral. sql 839, core 914 — both green. Story 21.5
…tually convert Story 21.5, part B (#275) — with the OQ-7 uniform parameter rule the lead gave at the dev kickoff. The OPERAND surface was never the gap (#267, refuted and closed): the failing production is the target-type regex, whose last alternative is why every rejection read `regex '(?i)(binary|varbinary)' expected but 'X' found`. Newly accepted: SIGNED / UNSIGNED [INTEGER|INT] -> BIGINT (Tableau's MySQL dialect) DECIMAL / NUMERIC / DEC -> DOUBLE TEXT / KEYWORD -> Text / Keyword (were DDL-only) CONVERT(x USING <charset>) -> VARCHAR (ES is UTF-8; charset dropped) Every alias maps onto an EXISTING SQLType. No parameterised SQLType is introduced: every consumer — coerce's ~40 arms, painlessType, canConvert's numericRank, the macro, the four es{N} bridges, SQLTypes.apply(String) — matches on case-object EQUALITY and would silently miss a case class. That is also what keeps the render a fixed point, and it is how the dropped precision is made HONEST rather than hidden: `CAST(x AS DECIMAL(10,2))` re-renders `CAST(x AS DOUBLE)`, so every artifact that echoes a statement shows the type the engine applied. NUMERIC deliberately maps to Double and NOT to SQLTypes.Numeric: coerce has exactly one arm producing Numeric (from a Boolean), so a Numeric target would fall through to the identity fallback and emit the operand unconverted — the #205 silent-wrong-answer shape. OQ-7, the uniform rule: every type SQL gives a length, precision or scale accepts `(p[,s])`, and every parameter is parsed and discarded — character, exact and approximate numeric, integer display width, fractional-second temporal, binary. DATE, BOOLEAN, KEYWORD, STRUCT, ARRAY and SIGNED/UNSIGNED get none, because no dialect parameterises them; both halves are pinned so the rule cannot drift back into a list. `typeParams` is attached to each INDIVIDUAL production and never hoisted onto `sql_type` (AD-6) — in DDL a type is followed by `FIELDS (...)` / `SCRIPT AS (...)` and a `::` cast by arbitrary expression text, which is the #219/#220 paren-greed shape. It is safe because `opt` recovers a Failure and none of start/separator/end can raise an Error. The parameter is an UNSIGNED literal and there are at most two, so `DECIMAL(10,2,3)` and `CHAR(-1)` reject: widening the rule multiplies the over-permissive surface, and discarded values make strictness free. Shares a revert boundary with the CHAR/TEXT/KEYWORD conversion fix, deliberately (AD-11.3 permits it with the argument stated): reverting the acceptance widening alone is safe, but reverting the conversion fix alone would re-open a silent no-op on exactly the traffic the widening newly admits. - coerce's `case (_, SQLTypes.Varchar)` becomes `case (_, _: SQLLiteral)`, so `CAST(1 AS CHAR)` emits `String.valueOf(1)` instead of the bare `1` — a silent no-op CHAR has had since the production existed. SQLLiteral's implementor set is exactly {SQLVarchar, SQLChar} + EsqlText/EsqlKeyword, and the identity arm still precedes it, so the widening is tight. - elasticType gains a `Char` arm (AD-9). It had none, so `Char` fell to `case _ => "object"` and `CREATE TABLE t (c CHAR)` silently created an OBJECT-typed field. This story advertises `CHAR(n)` in DDL, so shipping it without the arm would route new traffic into a persisted wrong mapping. - Found implementing that: `Column.node` skips `null_value` for `Varchar | Text` only, and Elasticsearch REFUSES `null_value` on a `text` field — so once CHAR maps to text, `CREATE TABLE t (c CHAR DEFAULT 'x')` would have emitted a mapping ES rejects. `Char` joins the skip. The rule is "a text-mapped column takes no null_value", with no "except". The macro's `SQLTypes.Varchar` arms become `_: SQLVarchar` so the newly reachable TEXT/KEYWORD targets land on the String check instead of `areTypesCompatible`'s permissive `case _ => true` fallback — a hole this story would otherwise open. Pinned with a positive and a negative case in macros-tests. sql 854, core 914, macrosTests 21, bridge 197 — all green. Story 21.5
Story 21.5, part C1 (lead ruling 2026-09-04: "it has to be fixed" — this is a
bug fix, not a behaviour choice).
`SQLTypeUtils.coerce`'s six `VARCHAR -> NUMERIC` and four `VARCHAR -> TEMPORAL`
arms matched the `SQLTypes.Varchar` case OBJECT. No Elasticsearch mapping ever
reports VARCHAR — `SQLTypes.apply(String)` maps every string field to `Text` or
`Keyword` — so the one arm that worked was unreachable from a real index and
every cast over a string column fell to the identity fallback, emitting the RAW
STRING. Measured against a real mapping:
literal CAST('125' AS BIGINT) Long.parseLong("125").longValue() OK
VARCHAR CAST(legacy AS BIGINT) ... Long.parseLong(...) : null OK
KEYWORD CAST(name AS BIGINT) (doc['name'].size() == 0 ? ... ) WRONG
TEXT CAST(descr AS BIGINT) (doc['descr'].size() == 0 ? ... ) WRONG
`_: SQLVarchar` covers Varchar/Text/Keyword and nothing else. `SQLChar` is
deliberately excluded (no ES mapping produces it), and the identity arm still
precedes the widened TARGET arm, so a string-to-same-string cast stays an
identity.
🔴 Why no existing test caught it, and why the new one looks unusual: a
schema-less parse leaves an identifier's `baseType` at `SQLTypes.Any`, so
`coerce` falls through for EVERY column and the module's own suites cannot
distinguish "no arm matched" from "no conversion needed". `CastConversionSpec`
attaches a real schema and re-runs `update(Some(schema))` — the only way
`baseType` becomes the mapped column's type.
Behaviour change, and it leads the release note: a stored query that silently
returned a string now returns a number or a date, and a value that cannot be
parsed now FAILS where it used to pass through. That is the documented CAST
contract; TRY_CAST / SAFE_CAST is the null-on-failure escape hatch.
sql 861 (3 remaining failures are part C2's own RED, fixed in the next commit).
Story 21.5
Story 21.5, part C2 (lead fold-in 2026-09-04: "it has to be fixed also" — do not ship CAST half-fixed). `SQLTypeUtils.coerce` carried WIDENING arms only (Int->BigInt, Int->Double, BigInt->Double), so every narrowing pair fell to the identity fallback: `CAST(1.9 AS INT)` emitted `1.9`, `CAST(300 AS TINYINT)` emitted `300`, and `CAST(<double column> AS BIGINT)` emitted the raw doc value. Rank-guarded, not thirteen enumerated pairs. What an enumeration would spell out a SECOND time is "the Painless primitive of a numeric SQLType" — which `painlessType` already IS, and `numericRank` already encodes the lattice (TinyInt 1 ... Double 6, exactly Java's widening order). One key, one derivation: the defect that recurred four times in story 21.3. A future numeric type now joins both halves at once instead of silently missing the narrowing one. Semantics: truncation toward zero — Java/Painless cast semantics, and the same form the widening arms already emit over the same null-guarded doc read, so the shape is precedented in-repo rather than invented. MySQL rounds; the divergence is documented, not hidden. The integration leg proves Elasticsearch accepts it. 🔴 `canConvert` is NOT touched, and `isNumericNarrowing` says why in-source: it asks a different question about the same ranks — whether SCHEMA EVOLUTION may change a column's type — and relaxing its "expansion only" rule would silently legalise narrowing column-type changes in an ALTER diff. `schema/TableDiff.scala` is its only production consumer. Behaviour change, release-note line 2: stored queries that consumed the unnarrowed value now see narrowed values. sql 864, core 914, bridge 197 — all green. Story 21.5
Story 21.5, part D (lead ruling 2026-09-04: fix by escaping, no regression). `Value.painless` wrapped a string in double quotes with NO escaping — `case s: String => s""""$s""""`. So `SELECT 'a"b'` emitted the Painless `"a"b"`, a script SYNTAX error Elasticsearch rejects at execution, and a value ending in a backslash emitted an unterminated string. Pre-existing and reachable today with ordinary data, on every literal-bearing script surface: script fields, script filters, script sorts, painless-computed columns. Backslash first, then the quote — escaping the quote first would double the backslash it had just introduced. `escapePainlessString` is deliberately NOT shared with `escapeStringLiteral` even though today's rules rhyme: one escapes for PAINLESS (double-quoted, Java escapes), the other for a SQL LITERAL (single-quoted, and it must stay reversible by `unescapeStringLiteral`). Different grammars, so folding them would couple two contracts that only happen to agree. 🔴 The sweep re-pinned NOTHING, and that is the finding, not an absence of work: every existing emitted-script assertion across sql / core / both bridge trees / macrosTests held BYTE-IDENTICAL, because for a literal carrying neither `"` nor `\` the emission is unchanged — and because no existing test ever content-asserted a literal containing either. The defect and the coverage gap had the same shape. `PainlessLiteralEscapingSpec` pins both halves: the escaped emission AND the byte-identity bound. A Painless-SYNTAX claim can only be proven by Elasticsearch, so the testkit carries the end-to-end case. sql 869, core 914, bridge 197 x 2, macrosTests 21 — all green. Story 21.5
… C cannot reach
Story 21.5. A schema-less unit test cannot see part C at all, and only
Elasticsearch can settle a Painless-SYNTAX claim, so these three defects need
their own fixture and CONTENT assertions rather than `isSuccess`. A dedicated
`cast_conversions` table: `dql_users` has no keyword column holding a numeric
string and no DOUBLE column, and adding either would move every 4-row oracle
above it.
Proven end to end on real ES 6.8 (rest + jest) / 7.17 / 8.18 / 9.0:
- C2 narrowing over a literal — `CAST(1.9 AS INT)` returns 1, `CAST(300 AS
TINYINT)` no longer returns 300. This is what proves Elasticsearch accepts
the emitted `((int) …)` over a def-typed expression.
- C1 conversion over a literal — SIGNED, DECIMAL(p,s), and `CAST(1 AS
CHAR(10))`, which used to emit the bare `1`.
- D — a script whose literal contains a `"` and one whose literal ends in a
backslash both COMPILE and return the right value. Before this story
Elasticsearch rejected them at script-compile time.
- A — `UPPER('it''s')` returns `IT'S`.
- B — the new target types are accepted over a column.
🔴 The C2 assertion is an EXACT double, deliberately: `longValue()` alone is
satisfied by the unnarrowed 1.9 as well, i.e. a gate that cannot fail. It passed
that way on the first run and was tightened.
🔴 And the integration leg found what the unit tests structurally could not:
part C's arms are UNREACHABLE for a COLUMN operand. `coerce` is indexed by
`baseType`, which for an identifier is `col.map(_.dataType)`, and `col` is
populated only by `SingleSearch.update(Some(schema))` — which has exactly ONE
production call site (`Table.mergeWithSearch`, inferring a CTAS target's
columns, returning a Table rather than a search to execute).
`resolveTemporalLiterals` loads a schema at the execution seam but rewrites
WHERE literals only. So every identifier reaches `coerce` as `SQLTypes.Any` and
NO cast over a column has ever emitted a conversion, for any source type —
measured after the fix: `CAST(<keyword> AS BIGINT)` still returns "125" and
`CAST(<double> AS BIGINT)` still returns 1.9.
Pinned as a known limitation with a delete-me-when-fixed note; it records a
DEFECT, not a contract. Attaching the schema at the seam is deliberately NOT
folded in: it would also silence `coerce`'s `case SQLTypes.Any if
!ctx.isProcessor` branch, which today makes every untyped identifier a
ZonedDateTime and injects `.toLocalDate()`, so it changes what EVERY scripted
query emits. Escalated to the lead; local record
`docs/issues/local-21.5-cast-conversion-defect.md`.
es8java 69/69 · es9java 69/69 · es7rest 69/69 · es6rest 68 (+1 pre-existing
enrich-policy environment cancel) · es6jest 68 (idem).
Story 21.5
…scaping
Story 21.5. Three published statements were FALSE and each one steered a reader
away from something that works or towards something broken; a fix a user cannot
discover is not delivered.
D1 — dql_statements.md claimed `CAST('125' AS BIGINT)` "does not parse (a
pre-existing grammar gap for literal operands)" and steered readers to
`'125'::BIGINT`. Both halves were wrong: every spelling parses (issue #267,
refuted and closed), and `::` is the ONE always-unsafe form — it hard-codes
`safe = false`, so the note pointed at the riskier spelling. Now names TRY_CAST.
D2 — functions_type_conversion.md said `DECIMAL` / `NUMERIC` are "not cast
targets" and that `TEXT` / `KEYWORD` are column types only. After #275 they are
all cast targets. The target list gains SIGNED/UNSIGNED, DECIMAL/NUMERIC/DEC,
TEXT/KEYWORD, BINARY/VARBINARY and the parameterised forms, and the three
limitations are stated plainly rather than implied: SIGNED and UNSIGNED are both
64-bit SIGNED, DECIMAL is APPROXIMATE, and a precision is accepted and IGNORED —
with the render normalising so the ignored parameter cannot look honoured.
D3 — known_limitations.md listed DECIMAL as unsupported. Removed, and the
approximation limitation put in its place so the constraint is not simply
deleted.
D5 — type_conversion.md is a SECOND conversion page with its own target-type
list. Two independent lists is the drift `feedback_dual_docs_sync` exists to
prevent, so it now defers to functions_type_conversion.md explicitly.
D6 — ddl_statements.md's SQL-type table gains CHAR (`text`, the AD-9 fix) and
DECIMAL/NUMERIC/DEC (`double`), plus the note that a length or precision is
accepted and ignored in DDL exactly as in a cast.
D4 — string literals had NO escaping documentation at all (grepped: zero hits
for escape / apostrophe / backslash across documentation/sql). New section
beside the quoted-identifier one: `''` is the standard and preferred escape,
`\'` and `\\` are accepted for compatibility, any OTHER backslash sequence is
LITERAL (there is no `\n`), a value ending in a backslash must be written
`'C:\\'`, and statements re-render in the backslash form.
Also documented, in D2: a cast whose operand is a COLUMN does not yet emit a
conversion, because the executing query carries no schema. Casts over literals
convert as documented. See docs/issues/local-21.5-cast-conversion-defect.md.
All 22 new or changed SQL examples were run through the real parser: every one
parses with `astEq = true` (documentation/ is not compiled, so nothing else
catches a broken example — project_release_doc_sweep).
⚠️ The softclient4es-web MDX mirror is owed and rides the site's own train; it
must not be pushed before the epic's 0.23.0 coordinate is published.
Story 21.5
… DDL type Story 21.5 — two items from the inline review, fixed in-branch. 1. AC-1 claims array literals among the positions `''` reaches, and no row exercised one. `TypeParser.literals` (`"[" ~> repsep(literal, ",") <~ "]"`) is a DIFFERENT caller of `literal` from the `IN` list already covered. Measured on origin/main: `ALTER TABLE t SET MAPPING x = ['a''b','c']` and `CREATE TABLE t (c VARCHAR DEFAULT ['a''b','c'])` were BOTH rejected there, so this is the widening genuinely reaching that caller. The ALTER form is round-tripped; the DDL DEFAULT form deliberately is NOT. `Values.sql` renders an array as an IN-list, which does not re-parse as an array — issue I6, PRE-EXISTING and independent of this story. Measured, not assumed: on origin/main `DEFAULT ['ab','c']`, with no doubled quote at all, already fails the same fixed point. Recorded in the test rather than pinned as passing. 2. `sql_type` now carries SIGNED / UNSIGNED, and `extension_type` delegates to it, so they are accepted as DDL COLUMN types too (`c SIGNED` creates a `long`). That fell out of the same production and was undocumented; the DDL type table now carries the row. sql 870 green. Story 21.5
…easurement
Story 21.5. Held-PR mode: this is the follow-up commit that keeps the PR born
clean.
🔴 M1 is REFUTED, and the proposed fix was measured to CAUSE the defect it was
meant to close. The finding said Painless "like Java" forbids a raw line
terminator in a string literal and asked for `\n`/`\r` escaping. Implemented and
run against real ES 8.18, the answer came back verbatim:
unexpected character ["a\n]. The only valid escape sequences in strings
starting with ["] are [\\] and [\"].
Painless's lexer content rule is `~[\\"]`: a RAW line terminator is LEGAL inside
the literal and the two-character escape is a COMPILE ERROR — the opposite of
Java. Measured both ways on the same suite: 70/70 green passing the terminator
through, 1 failing with the escaping. So the escaped set stays exactly `\` and
`"`, which is what the finding's own "enumerate rather than patch two cases"
instruction produces once the enumeration comes from Painless's grammar instead
of Java's. What the finding DID earn is kept in full: the scaladoc now derives
the set from the quoted ES error, and the new unit + testkit rows pin the
PASS-THROUGH so the proposal cannot be re-introduced silently (mutation M14:
re-adding it is RED).
Fixed as prescribed:
M2 the new `""` doc example annotated a COLUMN as a value, contradicting the
same file 30 lines above. Moved into a genuine value position, with the
SELECT-list reading spelled out. The lesson generalised: the doc sweep is
upgraded from "parses" to "MEANS what the annotation says", and running it
that way immediately found five more FALSE `-- Result:` lines on the same
page — CAST to BOOLEAN is a no-op (returns 1, not true), a space-separated
timestamp raises, an epoch is read as MILLIseconds, and the DECIMAL block
promised rounding this story explicitly does not do. All annotated
honestly; none is fixed in the engine (out of scope, pre-existing).
M3 the AD-13 limitation was disclosed in prose and contradicted by two
worked examples 20 lines below. Both are column operands: annotated as
returning the stored value. `known_limitations.md` gains the limitation.
M4 narrowing semantics were mis-stated. `(byte) 300` is 44 — Java narrowing
DISCARDS the high-order bits, i.e. WRAPS; "truncation toward zero" is only
true of float->integral. The implementation was right and the gloss wrong.
Corrected in-source, documented, and the loose integration gate
(`should not be 300`) tightened to the deterministic `shouldBe 44`.
L1 the stale `CAST('125' AS BIGINT) does NOT parse` comment D1 required
removing — the docs half had been fixed and the source half left behind.
L2 a dead assertion with no matcher, removed.
L3 AD-14's escaper map presented as complete: the `StringValue.ddl` twin is
now named and its non-sharing justified (it is pinned to the SQL grammar,
not Painless's — which is exactly why it must NOT follow this escaper).
L4 `leastCommonSuperType`'s `contains(SQLTypes.Varchar)` — the same
case-object hole, 100 lines above the arms this story fixed, made
REACHABLE by it. Widened to `_: SQLVarchar` and pinned.
L5 `CONVERT(x USING 'utf8')` rejected; the charset is now `(ident | literal)`.
L6 a pre-existing false `Result:` on a page this story rewrote. Corrected —
and NOT replaced with a `DATE_PARSE` alternative, because probing it
showed DATE_PARSE emits malformed Painless. One falsehood is not fixed by
publishing another.
Assertion sweep beyond M4.3, per the finding's instruction: five more of this
branch's own assertions were absence- or prefix-shaped and are now exact values.
🔴 Two harness lessons, both self-inflicted and both worth the record. The
falsification script reported three GREENs that were really RED: it matched
`"TESTS FAILED"` while sbt prints `*** 1 TEST FAILED ***` for a single failure.
And its `git checkout --` restore destroyed FIVE uncommitted source edits,
because the files carried work that was not yet committed. Restore by rewriting
the captured bytes, never by checkout, and never trust a mutation verdict a hand
run has not reproduced.
sql 872 x2 legs - core 914 x2 - macrosTests 21 - bridge 197 - es6bridge 197;
integration es8java 70/70 - es9java 70/70 - es7rest 70/70 - es6rest 69 -
es6jest 69 (+1 pre-existing enrich-policy environment cancel on the ES6 legs);
scalafmtCheckAll clean.
Story 21.5
…lumn converts Story 21.5, issue #306 — folded in by lead ruling (R-1 reversed: "core issue #306 should be part of this story"). The blast-radius analysis that argued for a separate story is not withdrawn; it is accepted as the cost, and measured below. `SQLTypeUtils.coerce` is indexed by the operand's `baseType`, which for an identifier is `col.map(_.dataType)`, and `col` is populated only by `SingleSearch.update(Some(schema))`. That had exactly ONE production call site — `Table.mergeWithSearch`, which infers a CTAS target's COLUMNS and returns a Table, not a statement to execute. So every executing query reached `coerce` with `baseType = Any` and NO cast over a column ever emitted a conversion, for any source type. Parts C1/C2 fixed the arms; this makes them reachable. SEAM: `SearchApi.resolveTemporalLiterals`, which already loads the schema for this exact statement, under the same single-concrete-index precondition and the same 5-minute cache. Every executing entry point already calls it — `search`, `searchAsync` and `searchWithInnerHits` (single + MultiSearch legs each), `ScrollApi.scroll`, and `IndicesApi`'s update/delete-by-query bodies — so coverage is by construction with no new call site and no extra round trip. In `search` the resolved statement is what flows into every branch (`scrollRows`, `searchWithWindowEnrichment`, `singleSearch`), so the routing cannot decide whether the conversion happens. UNCONDITIONAL within that precondition, deliberately not scoped to statements containing a cast: `coerce` also serves `FunctionN` argument coercion and CASE branch coercion, so a cast-scoped attach would make `UPPER(col)` emit differently depending on whether the same statement happened to carry a cast elsewhere — a #205-family inconsistency keyed on incidental content. The `hasCandidates` early return had to go: almost no statement carries a temporal WHERE literal, so the attach would have been dead for nearly every query. Cost, stated rather than buried: one schema lookup per index per cache TTL for every single-index statement. `TemporalLiteralSearchSpec`'s pin is RETARGETED to that new contract, not deleted — a multi-source or wildcard FROM still skips entirely, and that assertion is untouched. 🔴 A LATENT defect this made reachable, found on real ES and fixed here: a nullable conversion emitted `(x != null ? ((long) x) : null)`, and Painless cannot unify a PRIMITIVE with `null` — `class_cast_exception: Cannot cast from [long] to [java.lang.Object]`. Every primitive-producing arm needs boxing. `nullable` is true only for an identifier operand, so this could not fire while `baseType` was Any; literal operands are `nullable = false` and skip the ternary, which is why every literal case passed before and after. Boxed ONLY when the target is a ranked numeric — a blanket `(def)` moved a bridge pin that was never broken, which is churn, not a correction, so the fix reuses the existing `numericRank` lattice and leaves every reference-returning arm byte-identical. MOVED PINS: exactly ONE, and it is a correction — the `TemporalLiteralSearchSpec` performance contract above. `sql` (872), both bridge trees (197 each) and `macrosTests` (21) move by ZERO, because they parse schema-less and the attach lives in `core` — the same structural blindness that let the original defect survive, which is why the evidence is the integration legs, not the unit suites. The feared `Any`-branch fallout (untyped identifiers are treated as ZonedDateTime and get `.toLocalDate()` injected) is measured on real ES: `YEAR(birthdate)` and `DATE_TRUNC(birthdate, MONTH)` over a DATE column still return 1994 and 1994-01-01 through the newly typed route. `SchemaAttachSpec` is NEW and closes a citation gap: `SearchApi` claimed the double-resolve idempotency was "pinned in SchemaAttachSpec" and no such spec existed. It now pins the attach, the no-temporal-literal case, the wildcard skip, idempotency through `update` AND through the seam applied twice (`search` -> `scrollRows` -> `scroll` genuinely resolves twice), and the boxing. The R-1-era disclosures are REVERTED: the testkit's delete-me-when-fixed pin is retargeted to assert the converted values exactly, the limitation block and the `known_limitations.md` entry are gone, the worked examples return to column operands with converted results, and the release note is back to its PD-6 ordering with a downstream-rebuild line — the generated `script_fields` move, so repos pinning generated JSON must regenerate. FALSIFICATION: attach disabled -> RED (3); C1 widening reverted with the attach in place -> RED (2 — the keyword conversions only, the numeric narrowing SURVIVES, proving the attach and the arms are independently load-bearing); boxing removed -> RED (1). sql 872 x2 legs - core 920 x2 - macrosTests 21 - bridge 197 - es6bridge 197; real ES es8java 70/70, es9java 70/70, es7rest 70/70, es6rest 69, es6jest 69 (+1 pre-existing enrich-policy environment cancel on the ES6 legs); scalafmtCheckAll clean. Story 21.5
Story 21.5 / issue #306. HIGH-1, HIGH-2 and MEDIUM-7 are untouched and still with the lead. MEDIUM-3 — the four `<varchar> -> temporal` arms of `SQLTypeUtils.coerce` `return` a param name from inside `ctx.addParam(...)`, so they never reach the nullable wrapper at the end of the method. #306 made them reachable for a COLUMN operand, which turned that into a live defect: a document merely MISSING the field yielded `LocalDate.parse(null, …)`. One shared `temporalGuard` now wraps the param BODY (guarding the reference would be too late — the parse runs when the param is evaluated). Verified by `awk` that only those four arms return early; `:374` and `:393` are the identity and fallback arms, which correctly need no guard. MEDIUM-4 — the #306 commit's splice DELETED two integration tests and the suite count concealed it (two out, two in, 70/70 both sides). Both restored from `bb16a421`; they are the only real-ES executions of #305's escaping claims. MEDIUM-5 — the Any-branch test could not fail: it asserted `YEAR(...)` / `DATE_TRUNC(...)`, which route through `Identifier.originalType`, and that is `if (name.trim.nonEmpty) SQLTypes.Any` for ANY named column, attached or not. Removed, with the reason recorded at the site; the real Any-branch evidence lands with the HIGH fix. MEDIUM-6 — `resolveTemporalLiterals`' scaladoc still described the removed early return. Performance — `IndicesApi.loadSchema` did get/fetch/put, so N concurrent first-touch queries all missed and all fetched. Now one atomic `schemaCache.compute`, per #238's `shardCountCache` precedent. LOW-8 — the `(def)` box is applied only where the target actually produces a Painless primitive (`producesPainlessPrimitive`); blanket boxing had moved a bridge pin that was never broken. LOW-9 — the perf pin's prose claimed a per-statement FETCH regression would fail there while the next comment conceded the fixture overrides `loadSchema`. New test counts real round trips through the production cache (unseeded client, three statements, one fetch), so the claim is now true. LOW-10 — `Try(...).isFailure shouldBe true` was satisfied by any breakage at all, `collectRows`' own `res.isSuccess` assertion included. Now a control query proves the index, column and value are fine without the cast, and the failure is asserted to be Elasticsearch rejecting the query at execution. MEASURED: the Painless NumberFormatException reaches no Throwable message — the client keeps the root cause in its typed ErrorCause tree, the same asymmetry #224 worked around — so the value itself is unassertable here. LOW-11 — the (B) target-type test deferred to "the known limitation above", which #306 removed, and asserted only `nbResults = Some(1)` — which passes when all six columns come back as the raw keyword. Now asserts the values. Also widens `SchemaAttachSpec`: every case rendered with `painless(None)`, but production emits `script_fields` via `painless(Some(context))`, and that is the only path on which a conversion can be hoisted into a param — i.e. the only path on which MEDIUM-3's guard can be lost. Falsified (F4): removing `temporalGuard` reds that test and only that test. Verified: sql 872 and core 922 on 2.12 + 2.13, bridge 197, es6bridge 197, macrosTests 21, scalafmtCheckAll clean; GatewayApiIntegrationSpec 71/71 on real ES 8.18, 9.0, 7.17 and 70/70 + 1 pre-existing enrich-policy cancellation on ES 6.8 rest and jest. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…IGH-2)
Ruling 1. An ES `date` field is a millisecond timestamp, not a calendar
date, so `SQLTypes.apply(IndexField)` now resolves it to `Timestamp`. The
already-correct `(Timestamp, Date) => .toLocalDate()`,
`(Timestamp, Time) => .toLocalTime()` and `(Timestamp, BigInt)` arms then
fire for columns, and `CAST(x AS TIMESTAMP)` becomes the identity it should
always have been. The LocalDate-shaped `(Date, ...)` arms stay for literals.
Placed on the `IndexField` overload, NOT `apply(String)`: that one also
parses SQL type names, so the arm there would make `CREATE TABLE t (c DATE)`
declare a TIMESTAMP column.
🔴 ES 6.8 is NOT green and this commit does not claim it is - see the report.
`doc[...].value` yields a `JodaCompatibleZonedDateTime` there, which has
`toLocalDate()` but NOT `toLocalTime()`, so the ruling's premise holds on
ES 7/8/9 only. Escalated rather than re-baselined.
THE TRAP, checked first as instructed. `Column.diff` compares
`elasticType(actual) != elasticType(desired)` and every temporal type maps
to `"date"`, so the column diff was safe - MEASURED `columns = List()`. But
the round trip broke one level down: `IndexField.apply` PREFERS the recorded
`_meta.columns.<c>.data_type` over Elasticsearch's own `type`, so a table
declared `birthdate DATE` re-serialised its `_meta` as TIMESTAMP and the raw
JSON mappings comparison reported
`MappingSet(_meta.columns.birthdate.data_type, 'DATE')` - a spurious mapping
change on every existing index with a date column, on every
CREATE TABLE IF NOT EXISTS, table diff and ALTER planning pass.
`_meta.columns` is therefore excluded from the mappings diff: it is a SHADOW
of what step 3 already compares, and the two derivations disagree by
construction - step 3 asks what Elasticsearch can represent, the JSON
comparison asks whether the SQL SPELLING matches. Nothing is lost;
`Column.diff` covers type, default, script, comment, NOT NULL, options and
multi-fields, a strict superset of a `_meta.columns` entry. The rest of
`_meta` (primary_key, partition_by, type) is owned by nothing else and stays.
ParserSpec's round-trip pin then passes UNEDITED - the pin was right, the
diff was wrong.
Tests. New `ColumnMetaDiffSpec` pins both halves: the DATE round trip is
diff-clean, and every property a `_meta` entry carries is still reported.
New integration test covers all four temporal casts over a column declared
DATE (the harder case - `_meta` records DATE, so fixing only the ES fallback
would not reach it) with a 14:30Z time of day so truncation is visible.
Moved pins, each read and classified rather than re-baselined:
- 3 x `Some(SQLTypes.Date)` -> `Timestamp` (TemporalLiteralsSpec): the
ruling's intended correction; each test's SUBJECT (date_nanos -> Any,
literal resolution) is untouched and still asserted.
- SHOW TABLE `birthdate DATE` -> `birthdate TIMESTAMP`: user-visible,
acknowledged by the ruling, owes a release note.
- ParserSpec round trip: NOT re-baselined - fixed at the diff.
Falsification: F4 removing `temporalGuard` reds only the param test; F5
restoring the `_meta.columns` comparison reds only the round-trip test, with
the exact MappingSet diagnostic; F6 reverting this mapping reds the new
temporal-cast test on real ES 8.18.
Verified: sql 874 and core 922 on 2.12 + 2.13, bridge 197, es6bridge 197,
macrosTests 21, scalafmtCheckAll clean, build.sbt still 0.23.0-SNAPSHOT;
GatewayApiIntegrationSpec 72/72 on real ES 8.18, 9.0 and 7.17. ES 6.8 rest
and jest are RED on two tests, one root cause, reported not hidden.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…, option a)
ES 6.8 hands Painless a `JodaCompatibleZonedDateTime`, ES 7+ a
`java.time.ZonedDateTime`. MEASURED on 6.8.23:
dynamic method [org.elasticsearch.script.JodaCompatibleZonedDateTime,
toLocalTime/0] not found
`toLocalDate()` exists there and `toLocalTime()` does not, so Ruling 1's
premise ("an ES date IS a ZonedDateTime") holds on 7/8/9 only. `toInstant()`
exists on BOTH, so re-zoning through it yields a real ZonedDateTime on every
major -- and on 7/8/9 the chain is an IDENTITY, since ES stores and returns
UTC and 'Z' IS UTC.
ONE derivation, not a fifth copy. `painlessUtcZonedDateTime` and its
`…LocalDate` / `…LocalTime` compositions live in the sql package object, and
all FIVE pre-existing literal copies now route through it: three in
`function/package.scala`, one in `package.scala` whose entire rule was a
three-word `// compatible ES6+` comment, plus the two `coerce` arms. Zero
literal copies remain. This is the story's fourth "one key, two derivations"
and the first handled at the source rather than after the fact.
Applied uniformly to BOTH conversions, not only the one that failed --
following the accident that `toLocalDate` happens to work on 6.8 would leave
exactly the "except" the uniform-justification rule rejects.
TIMESTAMP and DATETIME are split, and that is a correctness boundary rather
than an exception: `coerce`'s `(varchar, DateTime)` arm emits
`LocalDateTime.parse(...)`, and `LocalDateTime` has no zero-argument
`toInstant()`, so normalising there is a compile error inside Elasticsearch.
That DATETIME denotes a LocalDateTime from one arm and a ZonedDateTime from
another is the SAME defect class as the two HIGHs -- one type, two runtime
values -- recorded, not fixed here.
Ingest guard. The normalisation was leaking into `PainlessContext(Processor)`,
where the operand is `ctx.<field>`. The rule, at the precision it holds: the
mapped type describes what `doc['f'].value` hands a QUERY script; in ingest
the operand is the raw JSON value, so an arm whose SOURCE type has a
different JSON representation -- the temporal ones -- must not fire. A
`keyword` is a String in both, so `CAST(code AS BIGINT)` is deliberately NOT
guarded. The guard makes the arms not match, falling to the identity, which
is BYTE-IDENTICAL to pre-#306 (verified at d1eef58, where the column read
back as Date and matched no temporal arm).
⚠️ NOT green everywhere: es6 rest and jest fail `alter a STRUCT column with
SET FIELDS`. Reported, not deferred -- scope probe pending.
Verified: sql 874, core 922, bridge 197, es6bridge 197, macrosTests 21;
GatewayApiIntegrationSpec 72/72 on real ES 7.17, 8.18 and 9.0; the four-way
temporal cast over a DATE column now PASSES on real ES 6.8.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Pulled into 21.5 by lead ruling: the `_meta.columns` diff exclusion (kept,
and independently correct) newly REACHES a latent defect, and a PR does not
ship a failure it reaches and defer it.
🔴 ONE expression produced two defects. `GenericProcessor.column` returned
`UUID.randomUUID().toString` for a processor with no `field` -- exactly a
script processor read back from Elasticsearch. MEASURED: three consecutive
reads of one processor's `column` gave three different UUIDs.
Two consumers read it:
- `Table.diff` keys processors "<pipeline>-<type>-<column>", so a read-back
processor could never match its declared counterpart => REMOVED and
re-ADDED on every ALTER, for a processor nobody touched (the churn);
- `ProcessorRemoved.stmt` renders `DROP PROCESSOR <TYPE>(<column>)`, so the
generated DDL read `DROP PROCESSOR SCRIPT(b18b1864-5e89-44fd-...)`, whose
hyphens are not an `ident` -- our own parser rejected it with "Mismatched
closing parentheses in ALTER PIPELINE statement", a message that names the
wrong cause.
`column` now falls back to a deterministic, identifier-safe id derived from
the processor's own content.
⚠️ The render, not the grammar, was at fault. The earlier plan (per #281,
balance the parens in `Parser.alterPipeline`) was the WRONG fix for this
symptom: the grammar was handed a value that is not an identifier. The
`start.? ~ ... ~ end.?` shape still deserves the #281 audit, but separately.
Backward compatible: `column` is computed at diff time and NEVER persisted,
so no stored pipeline carries the old value and nothing migrates. A processor
that HAS a `field` is byte-identical to before -- pinned as the compatibility
case.
This fixes F.2 only. F.1 (the churn) REMAINS: a deterministic id still does
not make an anonymous read-back processor match a declared one. It is now
deterministic rather than random. Nothing gates it -- no test asserts an empty
pipeline diff for an unchanged processor, which is why it survived. Recorded
in 21.8 Part F with that warning.
Only ES 6.8 was ever affected: processor `description` is an ES 7.9+ field, so
on 7/8/9 it survives the round trip and `ScriptDescRegex` re-types the
processor, which never becomes anonymous.
Falsification F8: restoring the UUID reds 3 of `AlterPipelineRenderSpec`'s 4
tests (the `field` case correctly stays green) and reds ES 6.8 with the
original diagnostic.
Verified: sql 878 and core 922 on 2.12 + 2.13, bridge 197, es6bridge 197,
macrosTests 21, scalafmtCheckAll clean, build.sbt still 0.23.0-SNAPSHOT.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Lead ruling. HIGH-1/HIGH-2 were real, but the earlier fix put the answer in
the wrong layer: mapping an Elasticsearch `date` to `Timestamp` inside
`SQLTypes.apply(IndexField)` conflated the type the user DECLARED with the
type Painless SEES, and one field cannot carry both facts.
The two facts, now separated:
- `Column.dataType` is DECLARED -- what the user wrote, what
`_meta.columns.<c>.data_type` persists, what SHOW CREATE TABLE prints,
what `Column.diff` compares;
- `SQLTypeUtils.runtimeType` is what `doc['f'].value` yields. DATE, TIME,
DATETIME, TIMESTAMP and TEMPORAL all map to the single ES type `date`, a
millisecond instant, so all five resolve to `Timestamp`. Everything else
is unchanged. ARRAY types are deliberately not collapsed (the runtime
value is a list); `date_nanos` needs no arm since it maps to `Any`.
Applied at exactly ONE site, `GenericIdentifier.baseType`. Verified first
that this is sound: every consumer of `baseType` is Painless emission or
`coerce` -- `function/{time,math,string,cond,convert}`, `SQLTypeUtils`,
`ArithmeticExpression`, `leastCommonSuperType`, and `Value`'s own coerce
call. `originalType` reads it only when the name is EMPTY, so a real column
never reaches it. Nothing wanting the declared type reads `baseType`; the
DDL, `_meta` and diff paths read `Column.dataType`.
REVERTED, both now unnecessary:
- the `date` -> `Timestamp` remap in `SQLTypes.apply(IndexField)`;
- the `_meta.columns` exclusion in the mappings diff (helper deleted).
`_meta` round-trips exactly again, so there is no spurious diff to hide.
KEPT: the Joda-compatible emission (independently correct, still required by
ES 6.8) and the deterministic `GenericProcessor.column` (a genuine
pre-existing defect; 21.8 F.2 stays delivered even though the exclusion no
longer exposes it).
MEASURED consequences, each predicted then checked:
- the `_meta` round trip is clean with `_meta.columns` FULLY compared;
- ES 6.8 `alter a STRUCT column with SET FIELDS` is green WITHOUT the
exclusion -- the unparseable ALTER is gone because its trigger is gone;
- SHOW TABLE reports `birthdate DATE` again, so the user-visible change the
earlier approach owed a release note for is withdrawn.
`ColumnMetaDiffSpec` retargeted rather than deleted: it now pins that the
DECLARED spelling survives the round trip, and says that if it ever reads
TIMESTAMP again the runtime type has leaked back into `Column.dataType`.
Verified: sql 879 and core 922 on 2.12 + 2.13, bridge 197, es6bridge 197,
macrosTests 21, scalafmtCheckAll clean, build.sbt still 0.23.0-SNAPSHOT;
GatewayApiIntegrationSpec green on all five real-ES legs -- es6 rest 72/0/1,
es6 jest 72/0/1, es7 73/73, es8 73/73, es9 73/73.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The lead asked for the exact overhead #306 introduced. Measured on Apple silicon (aarch64, 16 cores, JDK 11), medians over warmed runs. VERDICT: neutral -- documentation only, NO filter. 🔴 TWO costs, reported separately. A single blended number would have been true and useless, because they have opposite profiles. (a) The LOAD, amortised by the 5-minute schema cache: cache HIT 0.1 us parse on MISS, 5 fields 74 us parse on MISS, 50 fields 170 us parse on MISS, 300 fields 962 us Scales with MAPPING WIDTH, and costs one `GET <index>` per index per TTL. Concurrent first-touch queries do not stampede (`compute`). (b) The ATTACH, which never amortises: trivial SELECT 1-2 us single cast 1-3 us scripted + CASE + WHERE 7-11 us window function 5-8 us Scales with STATEMENT SIZE, not mapping width -- a 300-field mapping costs a trivial statement no more than a 5-field one. So (b) dominates in steady state and (a) only on a cold, wide index. Total steady-state overhead ~1-25 us against millisecond-scale statement latency: under 0.1%. SCROLL ROUTE, measured by COUNTING resolutions rather than reasoning about it -- this was the one result that could have been bad: 1 resolution for a LIMITed statement, 2 for an un-LIMITed (scroll-routed) one. Per STATEMENT, not per page, so #238's 10M-row extraction path pays the attach twice in total and is untouched. NO FILTER, declined on the measurement rather than on taste. It would save ~1-11 us on statements that emit no script, at the cost of a second code path and a new way for the attach to go silently missing -- which is the exact failure mode #306 existed to fix. 🔴 And the lead's suggested predicate "a cast is present" is UNSAFE as stated (AD-17): `coerce` also serves FunctionN argument and CASE branch coercion, so a cast-scoped attach would make `UPPER(col)` emit differently depending on whether a cast appeared ELSEWHERE in the same statement -- a #205-family inconsistency driven by incidental content. "Temporal as before" has the mirror defect: it is what made the attach dead for nearly every query. The only self-consistent predicate is "this statement emits Painless over a column" (`shouldBeScripted`), which IS cheaply computable -- recorded in the scaladoc so the question is not re-derived. The wide-mapping parse is the one cost a longer TTL amortises, which gives 21.8 Part D (per-index TTL) a concrete basis it did not have; noted under Tuning. The benchmark harness is deliberately NOT committed: a timing assertion in CI would be `ParseCostProbeSpec` again, whose flakiness cost two unrelated PRs a red. Docs are MD only. The softclient4es-web MDX mirror stays deferred under R-3 until 0.23.0 publishes, so this is not a dual-docs-sync violation -- do not "fix" it. Verified: core 922, sql 879, scalafmtCheckAll clean, build.sbt still 0.23.0-SNAPSHOT. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`resolveTemporalLiterals` stopped being about temporal literals when #306 made it attach the schema to the AST: rewriting the WHERE literals is now the smaller half of its job, and the lookup is no longer skipped for a statement without a temporal candidate. resolveTemporalLiterals -> resolveWithSchema (both overloads) resolveDmlTemporalLiterals -> resolveDmlWithSchema temporalLiteralSchemaMiss* -> schemaMiss* (the whole family) The negative cache never had anything to do with literals -- it remembers a 404 on `GET <index>` -- so dropping the prefix also makes the pairing legible for the first time: `schemaCache` (positive, IndicesApi) beside `schemaMisses` (negative, SearchApi). `TemporalLiterals`, the sql-module object, deliberately KEEPS its name: it genuinely resolves temporal literals, and `TemporalLiterals(single, schema)` at the call site says exactly what it does. Renaming by symbol rather than by substring is what keeps that true. Deletes `TemporalLiterals.hasCandidates`, which the early-return removal orphaned -- no production caller remained, and it is not a candidate for revival: any future load filter keys on `shouldBeScripted`, since "has a temporal literal" is the reading that left the attach dead for nearly every query. Its two explanatory comments keep the reasoning without naming a method that no longer exists. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
fupelaqu
marked this pull request as ready for review
September 8, 2026 05:54
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #274
Closes #275
Closes #304
Closes #305
Closes #306
Epic 21 story 21.5 — the literal and conversion-target surface of the SQL front door. Seven parts: four from the spec, two folded in by lead ruling at dev kickoff, and #306 folded in by a later ruling that reversed an earlier decision to defer it.
What changes
A — SQL-standard
''escaping (#274). Doubling was unsupported anywhere, so any literal containing an apostrophe —O'Brien,don't,l'avion— was a parse error on every surface: BI-generated SQL, the REPL, the drivers, DMLVALUES, DDL defaults,SCRIPT ASbodies. There is exactly one string-literal production, so one three-alternative regex plus a single left-to-right un-escaping scan fixes all thirteen positions. The three alternatives are disjoint on their first character, so the quantifier stays deterministic.B — the CAST / CONVERT target-type surface (#275). New accepted targets:
SIGNED/UNSIGNED(both →BIGINT; Tableau's MySQL dialect emits these),DECIMAL/NUMERIC/DEC,CHAR(n)/VARCHAR(n),TEXT,KEYWORD, andCONVERT(x USING <charset>). Every alias normalises onto an existingSQLType— no parameterisedSQLTypeis introduced, because every consumer matches on case-object equality and would silently miss one.C — the conversion defect (#304).
coerce's string-source arms matched theSQLTypes.Varcharcase object, which no Elasticsearch mapping ever produces, and narrowing numeric casts were silent no-ops. Both fixed.D — Painless literals emitted unescaped (#305). Any literal containing
"or\produced a script that fails to compile, and allowed Painless injection through a crafted literal.Two lead-ruled fold-ins.
""is now accepted inside a double-quoted string literal in value position, symmetric with''(safe only because 21.1 merged first; the identifier/value split is settled by production order and is pinned both ways). And the type-parameter list became a rule rather than a list of three shapes: every parameterisable type accepts(p[,s])and discards it, soINT(11),BIGINT(20),FLOAT(53),TIMESTAMP(3)all parse.typeParamsstays attached per type production and is never hoisted ontosql_type— hoisting it would re-open the #219/#220 paren-greed hole.Behaviour changes — read before upgrading
textorkeywordcolumn now actually converts.CAST(name AS BIGINT),AS INT,AS DOUBLE,AS DATEpreviously returned the value unconverted against a real index. They now emit the real conversion — stored queries return numbers where they returned strings — and a value that cannot be parsed now fails where it used to pass through silently. UseTRY_CAST/SAFE_CASTfor null-on-failure. This is a bug fix: the cast was silently doing nothing.datecolumn now behaves correctly.CAST(ts AS DATE)truncates to the date (it previously returned the full timestamp once columns became convertible), andCAST(ts AS TIMESTAMP)no longer fails the query. Emitted scripts for temporal casts change shape: they normalise through.toInstant().atZone(ZoneId.of('Z')), which is required for ES 6.8 and equivalent on 7/8/9.CAST(1.9 AS INT)returns 1. Note the measured semantics: float→integral truncates toward zero, but out-of-range integral→integral wraps —CAST(300 AS TINYINT)returns 44, it does not clamp. MySQL clamps; this engine follows Java/Painless cast semantics. Documented, not hidden.CAST(<non-string> AS CHAR)emitted the value unconverted; it now emitsString.valueOf(...), asVARCHARalways did.CHARcolumn now creates atextfield, not anobjectfield. ExistingCHARcolumns will be reported as a type change by table diff — that report is correct; the old mapping was wrong."or\were broken and now execute; such scripts change byte-shape.CAST(x AS DECIMAL(10,2))becomesCAST(x AS DOUBLE)— deliberately, so the type the engine applied is visible.SIGNED/UNSIGNED/DECIMAL/NUMERIC/DECare now accepted as DDL column types too. The new words are not reserved:SELECT signed FROM tstill parses as a column.Downstream repos that pin generated JSON must rebuild on 0.23.0 — the bridge's emitted JSON moves with the
script_fields.Statements are still re-rendered with backslash escaping (
'O\'Brien'); both spellings parse. A value ending in a single backslash must still be written'C:\\'.The schema now reaches the execution path (#306)
Folded in by lead ruling after the first review round.
coerceis indexed by an operand'sbaseType, which for an identifier comes from the attached schema — and until this commit the only production caller ofupdate(Some(schema))was CTAS column inference, which returns aTablerather than a statement to execute. So no executed query carried a schema, and no cast over a column had ever converted, for any source type. Part C's arms were correct and unreachable.The schema is now attached at
SearchApi.resolveTemporalLiterals, which already loads it for the same statement under the same precondition — no new call site, no extra round trip. Two choices worth calling out:coercealso serves function-argument and CASE-branch coercion, so a cast-scoped attach would makeUPPER(col)emit differently depending on whether the same statement happened to carry a cast elsewhere — an inconsistency driven by incidental statement content.hasCandidatesearly return had to go. It was correct while the only job was rewriting WHERE literals; almost no statement carries a temporal WHERE literal, so the attach would have been dead for nearly every query.Making a dormant path reachable surfaced a latent defect that would otherwise have shipped as a hard failure on every cast over a nullable column: a nullable conversion emitted
(x != null ? ((long) x) : null), which Elasticsearch rejects withCannot cast from [long] to [java.lang.Object]. It was invisible before becausenullableis true only for an identifier operand, and no identifier ever reached an arm — so every literal case passed both before and after. Fixed by boxing only where the target is a ranked numeric, leaving every reference-returning arm byte-identical.Declared type vs runtime type
Attaching the schema made casts over columns reachable for the first time, and that immediately exposed two regressions, both reproduced against a real Elasticsearch:
CAST(<date column> AS TIMESTAMP)killed the query outright, andCAST(<date column> AS DATE|TIME)silently stopped converting — the latter being the date-truncation idiom Superset and Tableau emit constantly.The cause was one field carrying two different facts.
SQLTypes.Datemeant ajava.time.LocalDatefor a literal, but aZonedDateTimefor a column, because that is whatdoc[…].valuereturns. Until the schema reached the execution path, no column had ever reached the Date-indexed conversion arms, so the ambiguity was invisible.The fix separates the two facts at the single site that needs the distinction:
SQLTypeUtils.runtimeTypederives whatdoc[…].valueyields, applied atGenericIdentifier.baseType— which is whatcoercereads — whileColumn.dataTypekeeps what the user declared.Date,Time,DateTime,TimestampandTemporalall map toTimestamp, since all five are the single Elasticsearch typedate, a millisecond instant. Arrays are deliberately not collapsed, anddate_nanosneeds no arm because it already maps toAny.Every consumer of
baseTypewas enumerated before the change: all of them are Painless emission orcoerce, and none wants the declared type — the DDL,_metaand diff paths all readColumn.dataType. Putting the derivation inSQLTypes.applyinstead was tried first and reverted: it collapsed the declared spelling into the mapped one, which made_metareport a spurious mapping change on every existing index with a date column, and the workaround for that went on to cause two further problems.ES 6.8 is not Java here. It hands Painless a
JodaCompatibleZonedDateTime, which hastoLocalDate()but nottoLocalTime(). The temporal arms therefore emit a normalised.toInstant().atZone(ZoneId.of('Z'))…form, valid on every major, through one shared derivation that also absorbed five pre-existing literal copies of the same idiom.In an ingest script the operand is
ctx.<field>— the raw JSON value, not a temporal object — so arms keyed on a source type whose JSON representation differs must not fire there. That guard is measured, not assumed: the ingest path had never been exercised end to end in the suite before this story.Also fixed here, because it became reachable:
GenericProcessor.columnreturnedUUID.randomUUID().toString, so a read-back script processor could never match its declared counterpart. That produced both a spurious remove-and-add on every ALTER and, on ES 6.8, an unparseableDROP PROCESSOR SCRIPT(<uuid>)— a UUID's hyphens are not an identifier. The id is now derived from the processor's own content, canonicalised before hashing so order-independence is a property rather than an observation, and it is computed at diff time rather than persisted, so nothing stored migrates.Verification
sql879 ·core922 ·macrosTests21 ·softclient4es-sql-bridge197 ·es6bridge197 — both Scala legs (+ sql/test,+ core/test);scalafmtCheckAllclean.String.replacepasses.build.sbtuntouched — the0.23.0-SNAPSHOTline belongs to story 21.4.One finding refuted by measurement
Independent review asked for
\n/\rescaping in the Painless escaper, by analogy with Java. Implementing it and running against real ES 8.18 produced the opposite verdict, quoted verbatim in the source: "The only valid escape sequences in strings starting with ["] are [\\] and [\"]". Painless's lexer content rule is~[\\"]— a raw line terminator is legal, and the two-character escape is a compile error. The proposed fix would have broken every working multi-line literal. The branch now carries a regression test against re-introducing it.🤖 Generated with Claude Code