diff --git a/bridge/src/test/scala/app/softnetwork/elastic/sql/SQLQuerySpec.scala b/bridge/src/test/scala/app/softnetwork/elastic/sql/SQLQuerySpec.scala index 18296027..ded8dd92 100644 --- a/bridge/src/test/scala/app/softnetwork/elastic/sql/SQLQuerySpec.scala +++ b/bridge/src/test/scala/app/softnetwork/elastic/sql/SQLQuerySpec.scala @@ -1534,7 +1534,7 @@ class SQLQuerySpec extends AnyFlatSpec with Matchers { | "field": "createdAt", | "script": { | "lang": "painless", - | "source": "def param1 = (doc['createdAt'].size() == 0 ? null : doc['createdAt'].value); (param1 == null) ? null : ZonedDateTime.parse(param1, new DateTimeFormatterBuilder().appendPattern(\"yyyy-MM-dd HH:mm:ss\").appendFraction(ChronoField.NANO_OF_SECOND, 0, 9, true).toFormatter().withZone(ZoneId.of('Z'))).truncatedTo(ChronoUnit.MINUTES).get(ChronoField.YEAR)" + | "source": "def param1 = (doc['createdAt'].size() == 0 ? null : doc['createdAt'].value); def param2 = (param1 == null) ? null : ZonedDateTime.parse(param1, new DateTimeFormatterBuilder().appendPattern(\"yyyy-MM-dd HH:mm:ss\").appendFraction(ChronoField.NANO_OF_SECOND, 0, 9, true).toFormatter().withZone(ZoneId.of('Z'))); def param3 = (param2 == null) ? null : (def)(param2.truncatedTo(ChronoUnit.MINUTES)); (param3 == null) ? null : (def)(param3.get(ChronoField.YEAR))" | } | } | } diff --git a/documentation/sql/ddl_statements.md b/documentation/sql/ddl_statements.md index 28836ed2..05e060dc 100644 --- a/documentation/sql/ddl_statements.md +++ b/documentation/sql/ddl_statements.md @@ -426,10 +426,21 @@ it currently exists. > statement still succeeds and the raw value is stored. Check the column names if a computed column > comes back with the type of its source instead of its own. -⚠️ **Date and time FUNCTIONS are not usable in a computed column.** In an ingest script the operand -is the raw JSON value of the incoming document — for a `date` column, the string as written — so -`YEAR(created)`, `DATE_TRUNC(...)` and `DATE_DIFF(...)` fail at ingest and the column is left -absent from the document. Conversions (`CAST`) and string functions (`UPPER`, ...) are unaffected. +**Date and time functions in a computed column.** In an ingest script the operand is the raw JSON +value of the incoming document, not the temporal object a query sees, so `YEAR(created)`, +`DATE_TRUNC(...)`, `DATE_ADD(...)`, `DATE_FORMAT(...)` and `DATE_DIFF(...)` parse it first. Both +shapes Elasticsearch accepts into a `date` field work — an ISO string (`"2025-01-10"`, +`"2025-01-10 14:30:00"`, `"2025-01-10T14:30:00Z"`) and epoch milliseconds (`1736467200000`) — and +the column's DECLARED type chooses how a string is read, so a `DATE` column expects a date-only +spelling. `CURRENT_DATE`, `CURRENT_TIMESTAMP`, `NOW` and `TODAY` are the ingest time of the +document. + +> A document that does not carry the source field leaves the computed column absent, as before. + +> Before `0.23.0` these functions failed at ingest and the column was silently missing from the +> stored document. If a table was created on an earlier version, re-run its `CREATE TABLE` (or +> `ALTER TABLE ... ALTER COLUMN ... SET SCRIPT AS`) so the pipeline is re-derived, and reindex any +> documents whose computed column is absent. --- diff --git a/es6/bridge/src/test/scala/app/softnetwork/elastic/sql/SQLQuerySpec.scala b/es6/bridge/src/test/scala/app/softnetwork/elastic/sql/SQLQuerySpec.scala index 7d9944b9..4c2ee99e 100644 --- a/es6/bridge/src/test/scala/app/softnetwork/elastic/sql/SQLQuerySpec.scala +++ b/es6/bridge/src/test/scala/app/softnetwork/elastic/sql/SQLQuerySpec.scala @@ -1534,7 +1534,7 @@ class SQLQuerySpec extends AnyFlatSpec with Matchers { | "field": "createdAt", | "script": { | "lang": "painless", - | "source": "def param1 = (doc['createdAt'].size() == 0 ? null : doc['createdAt'].value); (param1 == null) ? null : ZonedDateTime.parse(param1, new DateTimeFormatterBuilder().appendPattern(\"yyyy-MM-dd HH:mm:ss\").appendFraction(ChronoField.NANO_OF_SECOND, 0, 9, true).toFormatter().withZone(ZoneId.of('Z'))).truncatedTo(ChronoUnit.MINUTES).get(ChronoField.YEAR)" + | "source": "def param1 = (doc['createdAt'].size() == 0 ? null : doc['createdAt'].value); def param2 = (param1 == null) ? null : ZonedDateTime.parse(param1, new DateTimeFormatterBuilder().appendPattern(\"yyyy-MM-dd HH:mm:ss\").appendFraction(ChronoField.NANO_OF_SECOND, 0, 9, true).toFormatter().withZone(ZoneId.of('Z'))); def param3 = (param2 == null) ? null : (def)(param2.truncatedTo(ChronoUnit.MINUTES)); (param3 == null) ? null : (def)(param3.get(ChronoField.YEAR))" | } | } | } diff --git a/sql/src/main/scala/app/softnetwork/elastic/sql/function/package.scala b/sql/src/main/scala/app/softnetwork/elastic/sql/function/package.scala index 1dc509f3..cd73807d 100644 --- a/sql/src/main/scala/app/softnetwork/elastic/sql/function/package.scala +++ b/sql/src/main/scala/app/softnetwork/elastic/sql/function/package.scala @@ -322,7 +322,11 @@ package object function { Option(paramName) case SQLTypes.Any if ctx.isProcessor => in match { - case SQLTypes.DateTime | SQLTypes.Timestamp => + // Every temporal target, not just two. `DATE_FORMAT` (input DATE) and + // the `Temporal`-input functions matched nothing here, so they got no + // coercion at all and formatted the raw JSON scalar. + case SQLTypes.Date | SQLTypes.Time | SQLTypes.DateTime | + SQLTypes.Timestamp | SQLTypes.Temporal => val param = SQLTypeUtils .coerce( a, @@ -407,15 +411,85 @@ package object function { param.addPainlessMethod(p) base case _ => - s"$base$p" + // 🔴 `base` is a COMPOUND expression, not a parameter name, so the method cannot be + // pushed inside a parameter's own null guard the way the branch above does. Simply + // appending it put the method OUTSIDE the guard the previous function emitted: + // + // (param1 == null) ? null : LocalDate.parse(param1, F).get(ChronoField.YEAR) + // + // MEASURED on real Elasticsearch 8.18.3, that is a script COMPILE error -- + // `class_cast_exception: Cannot cast from [int] to [java.lang.Object]` -- because + // `.get(...)` returns a PRIMITIVE and Painless cannot unify a primitive with the + // `null` branch. `YEAR(DATE_PARSE(col, fmt))` and `MONTH(...)` failed on every + // client. It is the same unification rule `SQLTypeUtils.coerce` already answers + // with `(def)`, and the same answer is given here. + // + // 🔴 The cast is UNCONDITIONAL, and that is deliberate rather than lazy. The + // obvious guard is `producesPainlessPrimitive(out)` -- but `out` describes the SQL + // type, not the Painless expression: `Extract.outputType` is `Numeric`, whose + // `painlessType` is `java.math.BigDecimal`, a reference, while `.get(ChronoField)` + // really returns `int`. Keying on it would have left `YEAR` broken while looking + // correct. `(def)` on a value that is already a reference is a no-op, so the rule + // holds with no exception -- and this branch emitted nothing that ran before it + // existed, so no working emission moves because of it. + // + // ⚠️ Boxing alone does NOT fix it and neither does the null-safe `?.`: measured, + // Painless types `cond ? null : X` as `Object`, so `(…)?.get(…)` and + // `((…)).get(…)` both fail with `member method [java.lang.Object, get/1] not + // found`. The method has to land INSIDE the guard, which means binding the guarded + // expression to a name first -- the shape `DateTrunc`'s QUARTERS branch already + // uses, reused rather than re-invented. + // + // 🔴 The guarded result is returned INLINE and deliberately not bound as a + // parameter of its own. Binding it makes it findable, and the next function in the + // chain would then take the `addPainlessMethod` branch above and append to a + // parameter whose value is a guarded ternary — landing outside the guard again, + // one level up. MEASURED on the three-function chain + // `YEAR(DATE_TRUNC(DATETIME_PARSE(col, fmt), MINUTE))`, which produced + // `(param2 == null) ? null : (def)(param2.truncatedTo(…)).get(ChronoField.YEAR)`. + // Left inline, the next function finds nothing, binds THIS expression, and guards + // again — so a chain of any length composes by the same rule. + // + // ⚠️ Scoped to a NULLABLE operand, because that is exactly when the previous + // function emitted a guard for the method to land outside of. A non-nullable + // operand produces no ternary, appending was always correct there, and re-binding + // it would move working bytes for nothing (`LAST_DAY(...)` in a HAVING is the + // case that caught it). + if (!nullable) s"$base$p" // no guard to land outside of: append, as before + else + ctx.addParam(LiteralParam(base)) match { + case Some(bound) => + s"($bound == null) ? null : (def)($bound$p)" + case None => + s"$base$p" + } } } else p case None => - if (checkIfNullable && base.nonEmpty) - s"(def e$idx = $base; e$idx != null ? e$idx${painless(context)} : null)" + val p = painless(context) + // 🔴 The operand is a PREFIX only when the call is CHAINED onto it, i.e. when it + // starts with `.`. A STANDALONE call -- `LocalDate.parse(, ...)`, + // `ZonedDateTime.parse(, ...)`, `DateTimeFormatter.ofPattern(f).format()` + // -- already carries the operand in its own arguments, so prefixing `base` emitted it + // TWICE and fused two tokens with no operator between them: + // + // DATE_PARSE('2025-01-10','yyyy-MM-dd') -> "2025-01-10"LocalDate.parse("2025-01-10", ...) + // DATE_PARSE(name,'yyyy-MM-dd') -> ... e0 != null ? e0def arg0 = (doc['name']... + // + // Painless that cannot compile, for a literal operand and a column operand alike. + // + // The decision is taken HERE, at the assembly, and not in the four `toPainlessCall`s that + // happen to be standalone today: each of those is correct in isolation, and encoding the + // assembly's assumption into them would leave the next standalone-call function broken on + // arrival. It is also the same decision the context-bearing branch above already makes + // with its `else p` -- one rule, one place, so the two renderings cannot drift. + // Guarded family-wide by `StandaloneCallAssemblySpec`. + if (base.nonEmpty && !p.startsWith(".")) p + else if (checkIfNullable && base.nonEmpty) + s"(def e$idx = $base; e$idx != null ? e$idx$p : null)" else - s"$base${painless(context)}" + s"$base$p" } } 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 e76f7187..5bdfa589 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 @@ -142,10 +142,18 @@ package object time { def param: String + /** The processor form. In an ingest script every temporal value is a `ZonedDateTime` — the + * operand parse yields one and Elasticsearch will only accept one back into `ctx.` — so + * the DATE and TIME narrowings are dropped rather than producing a `LocalDate` that + * `ChronoUnit.between` then refuses to pair with the other operand. One type, one rule. + */ + def processorParam: String = param + override def painless(context: Option[PainlessContext]): String = { context match { case Some(ctx) => - ctx.addParam(LiteralParam(param.replaceAll("\\{\\{__now__}}", ctx.timestamp))) match { + val effective = if (ctx.isProcessor) processorParam else param + ctx.addParam(LiteralParam(effective.replaceAll("\\{\\{__now__}}", ctx.timestamp))) match { case Some(p) => return SQLTypeUtils.coerce(p, this.baseType, this.out, nullable = false, context) case _ => @@ -163,7 +171,25 @@ package object time { } object CurrentFunction { - val processorTimestamp: String = "ctx['_ingest']['timestamp']" + + /** 🔴 The ingest clock, and it was `ctx['_ingest']['timestamp']` — which is NULL. + * + * MEASURED on REAL indices (not `_simulate`) across every supported major — ES 6.8.23, + * 7.17.29, 8.18.3 and 9.0.3 — that access throws `null_pointer_exception` and, because a + * computed column's processor carries `ignore_failure: true`, the column was silently ABSENT. + * So `CURRENT_DATE` / `CURRENT_TIMESTAMP` / `NOW` / `TODAY` inside `CREATE TABLE … SCRIPT AS` + * has never worked on any version — including the PUBLISHED `DATE_DIFF(birthdate, + * CURRENT_DATE, YEAR)` example in `documentation/sql/ddl_statements.md` and the REPL testkit's + * `users` table. + * + * ⚠️ It is NOT a version split, which is what it first looked like: `metadata().now` works on + * ES 8+ and does not exist on 6/7, and a `metadata()` mention would fail COMPILATION on 6/7 + * even inside a branch never taken. `System.currentTimeMillis()` is whitelisted on all four + * majors (measured) and is already the unit the surrounding emission expects — the wrapper has + * always been `ZonedDateTime.ofInstant(Instant.ofEpochMilli(), ZoneId.of('Z'))`, written + * for a millis source. The accessor was the only thing wrong. + */ + val processorTimestamp: String = "System.currentTimeMillis()" val queryTimestamp: String = "params.__now__" } @@ -173,10 +199,12 @@ package object time { sealed trait CurrentDateFunction extends DateFunction with CurrentFunction { override def param: String = s"$now.toLocalDate()" + override def processorParam: String = now } sealed trait CurrentTimeFunction extends TimeFunction with CurrentFunction { override def param: String = s"$now.toLocalTime()" + override def processorParam: String = now } case object CurrentDate extends Expr("CURRENT_DATE") with TokenRegex { @@ -251,17 +279,32 @@ package object time { callArgs: List[String], context: Option[PainlessContext] ): String = { + // 🔴 `truncatedTo` does not exist on `LocalDate`, which is what a DATE-typed operand is. + // MEASURED on real Elasticsearch 8.18.3: `DATE_TRUNC(DATE_PARSE(col,fmt), MONTH)` was a + // compile error -- `member method [java.time.LocalDate, truncatedTo/1] not found`. The + // day-field methods below (`withDayOfYear`, `withDayOfMonth`, `with(DayOfWeek)`) all exist on + // `LocalDate`; only the time truncation does not, and on a date-only value it has nothing to + // truncate, so omitting it is the same value and not an approximation. + // `expr` is what this function is applied TO, which is the only thing that says whether the + // value carries a time. `in` is `SQLTypes.Temporal` for every DATE_TRUNC and says nothing. + // + // ⚠️ `baseType` is right in BOTH contexts, and that is not an accident: an ingest script now + // parses `ctx.` into a `ZonedDateTime` whatever the column was declared as, which is + // the same collapse `SQLTypeUtils.runtimeType` already applies for a query. One rule, one + // type — an ingest-only arm here would be an "except" with nothing behind it. + val truncateTime = + if (expr.baseType == SQLTypes.Date) "" else ".truncatedTo(ChronoUnit.DAYS)" unit match { - case TimeUnit.YEARS => ".withDayOfYear(1).truncatedTo(ChronoUnit.DAYS)" - case TimeUnit.MONTHS => ".withDayOfMonth(1).truncatedTo(ChronoUnit.DAYS)" - case TimeUnit.WEEKS => ".with(DayOfWeek.SUNDAY).truncatedTo(ChronoUnit.DAYS)" + case TimeUnit.YEARS => s".withDayOfYear(1)$truncateTime" + case TimeUnit.MONTHS => s".withDayOfMonth(1)$truncateTime" + case TimeUnit.WEEKS => s".with(DayOfWeek.SUNDAY)$truncateTime" case TimeUnit.QUARTERS => context match { case Some(ctx) => ctx.addParam(identifier) match { case Some(p) => val quarter = - s"$p.withMonth(((($p.getMonthValue() - 1) / 3) * 3) + 1).withDayOfMonth(1).truncatedTo(ChronoUnit.DAYS)" + s"$p.withMonth(((($p.getMonthValue() - 1) / 3) * 3) + 1).withDayOfMonth(1)$truncateTime" val quarterExpr = if (identifier.nullable) { s"$p != null ? $quarter : null" @@ -645,6 +688,15 @@ package object time { context match { case Some(ctx) => identifier.baseType match { + // ⚠️ DEAD for every real column, and deliberately left that way. No Elasticsearch + // mapping reports VARCHAR -- `SQLTypes(String)` yields `Text` or `Keyword` -- so + // case-object equality never matches here. Widening it to `_: SQLVarchar` was + // MEASURED and REVERTED: binding the parse as a parameter evaluates it EAGERLY, + // ahead of the null guard that `FunctionN.painless` puts around the reference, so + // a document merely MISSING the field ran `LocalDate.parse(null, ...)` and failed + // the shard. The inlined form below keeps the parse inside the guard, and the + // assembly binds the guarded expression itself when a later function needs to + // chain onto it. case SQLTypes.Varchar => ctx.addParam(LiteralParam(s"LocalDate.parse($arg, $param)")) match { case Some(p) => return p @@ -654,7 +706,16 @@ package object time { } case _ => } - s"LocalDate.parse($arg, $param)" + // 🔴 In an INGEST script the result is ASSIGNED to `ctx.`, and Elasticsearch + // refuses a `java.time.LocalDate` there — `illegal_argument_exception: unexpected value + // type [class java.time.LocalDate]`, measured, and the computed column vanished. A + // `ZonedDateTime` IS accepted and serialises as ISO-8601, which a `date` field parses. + // Same collapse the operand side applies: in a processor every temporal value is a + // ZonedDateTime, so there is one rule rather than one per position. + if (context.exists(_.isProcessor)) + s"LocalDate.parse($arg, $param).atStartOfDay(ZoneId.of('Z'))" + else + s"LocalDate.parse($arg, $param)" case _ => throw new IllegalArgumentException("DateParse requires exactly one argument") } @@ -705,24 +766,41 @@ package object time { override def toPainlessCall(callArgs: List[String], context: Option[PainlessContext]): String = callArgs match { case arg :: Nil => + // 🔴 A STRING operand is not a `TemporalAccessor`, and `DateTimeFormatter.format` takes + // one. MEASURED on real Elasticsearch 8.18.3, both spellings were a script COMPILE error: + // + // DATE_FORMAT('2025-01-10', '%Y') -> Cannot cast from [java.lang.String] to + // DATETIME_FORMAT(, '%Y') [java.time.temporal.TemporalAccessor] + // + // The documented spelling `DATE_FORMAT('2025-01-10'::DATE, '%Y-%m-%d')` worked only + // because the CAST inserted the parse, which is why no published example ever failed. + // The operand is parsed here through the SAME `coerce` arms the cast uses -- one + // derivation, so the accepted format set cannot differ between the two spellings. + // + // ⚠️ The type test is `_: SQLVarchar`, not `SQLTypes.Varchar`. Case-object equality is + // what made the old branch DEAD for every real column: `SQLTypes(String)` maps an + // Elasticsearch string field to `Text` or `Keyword` and never to the `Varchar` object. + // Same defect, same fix, as the ` -> ` arms in story 21.5. + // ⚠️ `nullable = identifier.nullable`, not `false`: `coerce`'s temporal arms guard their + // own parse only when told the operand can be null, and the parse is bound as its own + // parameter, i.e. EVALUATED before the guard `FunctionN.painless` puts around the format + // call. Passing `false` here made a document merely MISSING the field run + // `LocalDate.parse(null, ...)` and fail the shard. + val operand = identifier.baseType match { + case _: SQLVarchar => + SQLTypeUtils + .coerce(arg, identifier.baseType, inputType, identifier.nullable, context) + case _ => arg + } context match { case Some(ctx) => - identifier.baseType match { - case SQLTypes.Varchar => - ctx.addParam(LiteralParam(s"$param.format($arg)")) match { - case Some(p) => return p - case _ => - } - case _ => - ctx.addParam(LiteralParam(param)) match { - case Some(p) => return s"$p.format($arg)" - case _ => - } - + ctx.addParam(LiteralParam(param)) match { + case Some(p) => return s"$p.format($operand)" + case _ => } case _ => } - s"$param.format($arg)" + s"$param.format($operand)" case _ => throw new IllegalArgumentException("DateParse requires exactly one argument") } @@ -812,6 +890,15 @@ package object time { context match { case Some(ctx) => identifier.baseType match { + // ⚠️ DEAD for every real column, and deliberately left that way. No Elasticsearch + // mapping reports VARCHAR -- `SQLTypes(String)` yields `Text` or `Keyword` -- so + // case-object equality never matches here. Widening it to `_: SQLVarchar` was + // MEASURED and REVERTED: binding the parse as a parameter evaluates it EAGERLY, + // ahead of the null guard that `FunctionN.painless` puts around the reference, so + // a document merely MISSING the field ran `LocalDate.parse(null, ...)` and failed + // the shard. The inlined form below keeps the parse inside the guard, and the + // assembly binds the guarded expression itself when a later function needs to + // chain onto it. case SQLTypes.Varchar => ctx.addParam(LiteralParam(s"ZonedDateTime.parse($arg, $zonedParam)")) match { case Some(p) => return p @@ -869,24 +956,41 @@ package object time { override def toPainlessCall(callArgs: List[String], context: Option[PainlessContext]): String = callArgs match { case arg :: Nil => + // 🔴 A STRING operand is not a `TemporalAccessor`, and `DateTimeFormatter.format` takes + // one. MEASURED on real Elasticsearch 8.18.3, both spellings were a script COMPILE error: + // + // DATE_FORMAT('2025-01-10', '%Y') -> Cannot cast from [java.lang.String] to + // DATETIME_FORMAT(, '%Y') [java.time.temporal.TemporalAccessor] + // + // The documented spelling `DATE_FORMAT('2025-01-10'::DATE, '%Y-%m-%d')` worked only + // because the CAST inserted the parse, which is why no published example ever failed. + // The operand is parsed here through the SAME `coerce` arms the cast uses -- one + // derivation, so the accepted format set cannot differ between the two spellings. + // + // ⚠️ The type test is `_: SQLVarchar`, not `SQLTypes.Varchar`. Case-object equality is + // what made the old branch DEAD for every real column: `SQLTypes(String)` maps an + // Elasticsearch string field to `Text` or `Keyword` and never to the `Varchar` object. + // Same defect, same fix, as the ` -> ` arms in story 21.5. + // ⚠️ `nullable = identifier.nullable`, not `false`: `coerce`'s temporal arms guard their + // own parse only when told the operand can be null, and the parse is bound as its own + // parameter, i.e. EVALUATED before the guard `FunctionN.painless` puts around the format + // call. Passing `false` here made a document merely MISSING the field run + // `LocalDate.parse(null, ...)` and fail the shard. + val operand = identifier.baseType match { + case _: SQLVarchar => + SQLTypeUtils + .coerce(arg, identifier.baseType, inputType, identifier.nullable, context) + case _ => arg + } context match { case Some(ctx) => - identifier.baseType match { - case SQLTypes.Varchar => - ctx.addParam(LiteralParam(s"$param.format($arg)")) match { - case Some(p) => return p - case _ => - } - case _ => - ctx.addParam(LiteralParam(param)) match { - case Some(p) => return s"$p.format($arg)" - case _ => - } - + ctx.addParam(LiteralParam(param)) match { + case Some(p) => return s"$p.format($operand)" + case _ => } case _ => } - s"$param.format($arg)" + s"$param.format($operand)" case _ => throw new IllegalArgumentException("DateParse requires exactly one argument") } diff --git a/sql/src/main/scala/app/softnetwork/elastic/sql/package.scala b/sql/src/main/scala/app/softnetwork/elastic/sql/package.scala index 1eef212d..f787685b 100644 --- a/sql/src/main/scala/app/softnetwork/elastic/sql/package.scala +++ b/sql/src/main/scala/app/softnetwork/elastic/sql/package.scala @@ -1328,6 +1328,11 @@ package object sql { if (name.trim.nonEmpty) SQLTypes.Any else this.baseType + /** The type the column was DECLARED as, where that differs from what a query hands Painless. + * Defaults to `baseType`; only a schema-resolved identifier can tell them apart. + */ + def declaredType: SQLType = baseType + override def painless(context: Option[PainlessContext]): String = { // A context-free rendering of an AGGREGATE is a bucket-pipeline rendering (`bucket_selector` // for HAVING, `bucket_script` for arithmetic over aggregates): there is no document to read, @@ -1357,10 +1362,43 @@ package object sql { } case _ => // do nothing } + + /** 🔴 In an INGEST script the operand is `ctx.` — the RAW JSON value — so a temporal + * function applied to it ran `String.get(ChronoField)` and threw; `ignore_failure: true` + * swallowed the throw and the computed column was silently ABSENT. Measured on real + * Elasticsearch 8.18.3 for `YEAR`, `MONTH`, `DATE_TRUNC` and `DATE_ADD`, and + * `DATE_DIFF(birthdate, CURRENT_DATE, YEAR)` is a PUBLISHED example. + * + * The value is parsed into a temporal FIRST, with the shape decided at runtime because + * Elasticsearch accepts both an ISO string and epoch millis into a `date` field. It is the + * base rather than a method because a processor parameter drops `painlessMethods`. + * + * Only when a temporal is actually required: a `keyword` is a `String` in both contexts, so + * `UPPER(a)` and `CAST(zip AS BIGINT)` stay byte-identical. + */ + val processorBase: Option[String] = + context.filter(_.isProcessor).flatMap { _ => + orderedFunctions.headOption + .collect { + // ... and only when the operand is CHAINED. A function that takes this identifier as + // an ARGUMENT (`DATE_FORMAT`, `DATE_PARSE`) coerces it on the argument path instead, + // and doing both emits a parse nobody reads. + case f: TransformFunction[_, _] + if !f.args.exists(_ == this) && + (f.in == SQLTypes.Temporal || f.in == SQLTypes.Date || + f.in == SQLTypes.Time || f.in == SQLTypes.DateTime || + f.in == SQLTypes.Timestamp) => + f + } + .flatMap(_ => SQLTypeUtils.processorTemporal(processParamName, declaredType)) + } val base = context match { case Some(ctx) => - ctx.addParam(this).getOrElse("") + processorBase + .flatMap(e => ctx.addParam(LiteralParam(e))) + .orElse(ctx.addParam(this)) + .getOrElse("") case _ => if (nullable) checkNotNull @@ -1465,6 +1503,16 @@ package object sql { override def baseType: SQLType = col.map(c => SQLTypeUtils.runtimeType(c.dataType)).getOrElse(super.baseType) + /** The DECLARED type, which an INGEST script needs and `baseType` cannot give it. + * + * `runtimeType` collapses every temporal declaration to `Timestamp`, because that is what a + * QUERY gets from `doc['f'].value` whatever the column was declared as. An ingest script reads + * `ctx.` — the raw JSON — so the declaration is the only thing that says which temporal + * the value should become: a DATE column carrying `"2025-01-10"` must parse as a `LocalDate`, + * and `ZonedDateTime.parse` REFUSES a date without a time (measured on ES 8.18.3). + */ + override def declaredType: SQLType = col.map(_.dataType).getOrElse(baseType) + def update(request: SingleSearch): Identifier = { val bucketPath: String = request.groupBy match { diff --git a/sql/src/main/scala/app/softnetwork/elastic/sql/type/SQLTypeUtils.scala b/sql/src/main/scala/app/softnetwork/elastic/sql/type/SQLTypeUtils.scala index 2784cf38..6213643b 100644 --- a/sql/src/main/scala/app/softnetwork/elastic/sql/type/SQLTypeUtils.scala +++ b/sql/src/main/scala/app/softnetwork/elastic/sql/type/SQLTypeUtils.scala @@ -244,13 +244,9 @@ object SQLTypeUtils { case _ => // do nothing } case SQLTypes.Any if ctx.isProcessor => - to match { - case SQLTypes.DateTime | SQLTypes.Timestamp => - val expr = identifier.painless(context) - val from = SQLTypes.BigInt - val ret = coerce(expr, from, to, identifier.nullable, context) - return ret - case _ => // do nothing + processorTemporal(identifier.painless(context), identifier.declaredType) match { + case Some(parsed) => return parsed + case None => // do nothing } case _ => // do nothing } @@ -634,6 +630,57 @@ object SQLTypeUtils { * `painlessType` is the single existing derivation of "the Painless type of an SQLType", so the * question is asked of it rather than by re-listing the primitives here. */ + /** The runtime shape of `ctx.` in an INGEST script, resolved at RUNTIME rather than + * guessed. + * + * 🔴 In a query the operand of a temporal function is what Elasticsearch hands Painless for a + * `date` field — a temporal object. In an ingest script it is `ctx.`: the RAW JSON value + * of the document being indexed, so `String` has no `get(ChronoField)`, no `plus`, no + * `withDayOfMonth`. The script threw, `ignore_failure: true` swallowed it, and the computed + * column was simply ABSENT from the stored document — measured on real Elasticsearch 8.18.3 for + * `YEAR`, `MONTH`, `DATE_TRUNC`, `DATE_ADD`/`DATE_SUB` and `DATE_DIFF`, the last of which is a + * PUBLISHED example (`documentation/sql/ddl_statements.md`). + * + * ⚠️ The previous code answered the shape question one way, without a test: it hard-coded `from + * = BigInt`, i.e. it ASSUMED epoch millis and emitted `Instant.ofEpochMilli(...)`, which throws + * for a document written with an ISO string — and ISO strings are what every documented example + * ingests. It also covered only `DateTime`/`Timestamp` targets, so `YEAR`, `MONTH` and + * `DATE_TRUNC` (whose input is `Temporal`/`Date`) got no coercion at all. + * + * Elasticsearch accepts BOTH shapes into a `date` field, so neither guess is right and the + * decision belongs at runtime (the lead's ruling, story 21.8 Part C). `instanceof` costs one + * type check per document and removes a whole class of silently-missing columns. Verified on + * real ingest pipelines against `"2025-01-10"`, `"2025-01-10 14:30:00"`, + * `"2025-01-10T14:30:00Z"` and `1736467200000`. + * + * The DECLARED type chooses the temporal the value becomes, and the string branch reuses the + * very ` -> ` arms a CAST uses, so the accepted format set cannot differ + * between `CAST(col AS DATE)` and a date function over the same column. + * + * Returns `None` — leaving the emission byte-identical — for any non-temporal declared type: a + * `keyword` is a `String` in BOTH contexts, which is why `UPPER(a)` and `CAST(zip AS BIGINT)` + * always worked and must not move. + */ + private[sql] def processorTemporal(expr: String, declared: SQLType): Option[String] = { + val epoch = s"Instant.ofEpochMilli($expr).atZone(ZoneId.of('Z'))" + val parsed = declared match { + // A DATE is written date-only, and `ZonedDateTime.parse` REFUSES a date with no time + // (measured), so it is parsed as a `LocalDate` and then given the UTC start of day. + case SQLTypes.Date => + Some( + coerce(expr, SQLTypes.Varchar, SQLTypes.Date, nullable = false, None) + + ".atStartOfDay(ZoneId.of('Z'))" + ) + case SQLTypes.DateTime | SQLTypes.Timestamp | SQLTypes.Temporal => + Some(coerce(expr, SQLTypes.Varchar, SQLTypes.Timestamp, nullable = false, None)) + // 🔴 TIME is deliberately absent: Elasticsearch has no time-of-day type, so no column is + // ever declared one from a real mapping, and inventing an emission for it would be a guess + // of exactly the kind this method exists to remove. + case _ => None + } + parsed.map(p => s"($expr instanceof String ? $p : $epoch)") + } + private val painlessPrimitives: Set[String] = Set("byte", "short", "int", "long", "float", "double", "boolean", "char") diff --git a/sql/src/test/scala/app/softnetwork/elastic/sql/function/StandaloneCallAssemblySpec.scala b/sql/src/test/scala/app/softnetwork/elastic/sql/function/StandaloneCallAssemblySpec.scala new file mode 100644 index 00000000..4ee5d3d5 --- /dev/null +++ b/sql/src/test/scala/app/softnetwork/elastic/sql/function/StandaloneCallAssemblySpec.scala @@ -0,0 +1,450 @@ +package app.softnetwork.elastic.sql.function + +import app.softnetwork.elastic.sql.parser.Parser +import app.softnetwork.elastic.sql.query.SingleSearch +import app.softnetwork.elastic.sql.schema.{Column, Table} +import app.softnetwork.elastic.sql.`type`.SQLTypes +import app.softnetwork.elastic.sql.{PainlessContext, PainlessContextType} +import org.scalatest.flatspec.AnyFlatSpec +import org.scalatest.matchers.should.Matchers + +import java.io.File +import scala.io.Source + +/** Story 21.8 Part C — the chain assembly concatenated the operand in front of a STANDALONE call. + * + * `TransformFunction.toPainless`'s context-free branch appended the function's own rendering to + * the operand on the assumption that the rendering is a method CHAINED onto it, i.e. that it + * begins with `.`. True of most of the date family — `.plus(…)`, `.truncatedTo(…)`, + * `.get(ChronoField.YEAR)` — and false of exactly the four whose call is standalone, because a + * standalone call already carries the operand in its own ARGUMENTS. MEASURED on `origin/main` + * `e6891135`: + * {{{ + * DATE_PARSE('2025-01-10','yyyy-MM-dd') + * -> "2025-01-10"LocalDate.parse("2025-01-10", DateTimeFormatter.ofPattern("yyyy-MM-dd")) + * DATE_PARSE(name,'yyyy-MM-dd') + * -> (def e0 = (doc['name']…); e0 != null ? e0def arg0 = (doc['name']… + * }}} + * `e0def` — two tokens fused. Painless that cannot compile, for a literal operand and a column + * operand alike, and it PROPAGATED: `YEAR(DATE_PARSE(name,'yyyy-MM-dd'))` and + * `DATE_ADD(DATE_PARSE(name,…), INTERVAL 1 DAY)` carried the fusion outward, which the defect + * record did not have. + * + * 🔴 It is fixed at the ASSEMBLY and guarded over the FAMILY (the story's AD-1 / AC-5), not + * patched in the four `toPainlessCall`s. Each of those is correct in isolation; encoding the + * assembly's assumption into them would leave the next standalone-call function broken on arrival. + * The `case Some(ctx)` branch beside it already made exactly this decision with its `else p`, so + * the fix also collapses two renderings onto one rule instead of letting them disagree. + * + * 🔴 Blast radius, measured and NOT what the record assumed. `painless(None)` is the CONTEXT-FREE + * rendering; every shipped surface that reaches these four functions renders them with a + * `PainlessContext` (script fields, script sorts, the materialized-view projection), and that + * branch was always right. The context-free sites in shipped code — `bucket_selector`, + * `bucket_script`, geo distance, top_hits script fields — either carry aggregates or drop the + * field. So this is a public-API correctness fix with no live query failure behind it, which is + * why the lead deferred it out of 21.8's first pass; it is NOT a claim that some query was + * failing. + * + * 🔴 Every emission pinned below was EXECUTED on a real Elasticsearch 8.18.3 as the `source` of a + * `script_fields` entry over a real index, because "this is syntactically valid Painless" is a + * claim only Elasticsearch settles — this project's standing rule, and the confirmation the defect + * record itself flagged as OWED and inferred. Discharged: the two pre-fix strings come back + * `script_exception :: compile error`, and the post-fix ones return `2025-01-10` / `2025` and + * `null` for a document missing the field. The pre-fix strings are deliberately NOT pinned + * anywhere — they are the corruption this fixes. + * + * ⚠️ One pinned emission below is pinned as FUSION-FREE and explicitly NOT as executable; the + * reason is at that test, and it is a pre-existing defect of a different kind. + */ +class StandaloneCallAssemblySpec extends AnyFlatSpec with Matchers { + + private val schema: Table = Table( + "t", + columns = List( + Column("name", SQLTypes.Keyword), + Column("created", SQLTypes.Date), + Column("ts", SQLTypes.Timestamp) + ) + ) + + /** A schema-CARRYING parse. Without it every identifier's `baseType` stays `Any`, no conversion + * arm is reachable and a broken assembly looks identical to a fixed one (#306 / story 21.5). + */ + private def field(sql: String) = + Parser(sql) match { + case Right(ss: SingleSearch) => ss.update(Some(schema)).select.fields.head + case other => fail(s"[$sql] expected a SingleSearch, got $other") + } + + private def painlessOf(sql: String): String = field(sql).painless(None) + + private def scriptOf(sql: String): String = { + val ctx = PainlessContext(PainlessContextType.Query) + val body = field(sql).painless(Some(ctx)) + s"$ctx$body" + } + + // -- the four standalone-call functions, both operand kinds ------------------------------------ + + "DATE_PARSE over a literal" should "emit the parse alone, not the literal in front of it" in { + // Executed on ES 8.18.3: returns 2025-01-10. + painlessOf("SELECT DATE_PARSE('2025-01-10', 'yyyy-MM-dd') FROM t") shouldBe + """LocalDate.parse("2025-01-10", DateTimeFormatter.ofPattern("yyyy-MM-dd"))""" + } + + "DATE_PARSE over a column" should "emit the null-guarded parse alone" in { + // Executed on ES 8.18.3 with the doc access substituted by a bound value: returns 2025-01-10, + // and null for a missing field rather than failing the shard. + painlessOf("SELECT DATE_PARSE(name, 'yyyy-MM-dd') FROM t") shouldBe + "def arg0 = (doc['name'].size() == 0 ? null : doc['name'].value); " + + """(arg0 == null) ? null : LocalDate.parse(arg0, DateTimeFormatter.ofPattern("yyyy-MM-dd"))""" + } + + "DATETIME_PARSE" should "emit the parse alone for both operand kinds" in { + painlessOf( + "SELECT DATETIME_PARSE('2025-01-10 10:00:00', 'yyyy-MM-dd HH:mm:ss') FROM t" + ) shouldBe + """ZonedDateTime.parse("2025-01-10 10:00:00", """ + + """DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss").withZone(ZoneId.of('Z')))""" + painlessOf("SELECT DATETIME_PARSE(name, 'yyyy-MM-dd HH:mm:ss') FROM t") shouldBe + "def arg0 = (doc['name'].size() == 0 ? null : doc['name'].value); (arg0 == null) ? null : " + + """ZonedDateTime.parse(arg0, """ + + """DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss").withZone(ZoneId.of('Z')))""" + } + + "DATE_FORMAT and DATETIME_FORMAT" should "emit the format call alone for both operand kinds" in { + // A TEMPORAL operand is formatted directly; these two are byte-identical to `origin/main`. + painlessOf("SELECT DATE_FORMAT(created, 'yyyy') FROM t") shouldBe + "def arg0 = (doc['created'].size() == 0 ? null : doc['created'].value); " + + """(arg0 == null) ? null : DateTimeFormatter.ofPattern("yyyy").format(arg0)""" + painlessOf("SELECT DATETIME_FORMAT(ts, 'yyyy') FROM t") shouldBe + "def arg0 = (doc['ts'].size() == 0 ? null : doc['ts'].value); " + + """(arg0 == null) ? null : DateTimeFormatter.ofPattern("yyyy").format(arg0)""" + } + + it should "PARSE a string operand instead of handing a String to format()" in { + // 🔴 Found by executing the assembly's own output, which is what AC-4's real-index rule is + // for. `DateTimeFormatter.format` takes a `TemporalAccessor`; a `String` is not one, so + // MEASURED on ES 8.18.3 both of these were a script COMPILE error before this story: + // + // Cannot cast from [java.lang.String] to [java.time.temporal.TemporalAccessor] + // + // Live on the production path too, and for a KEYWORD COLUMN as well as a literal. The + // documented spelling `DATE_FORMAT('2025-01-10'::DATE, '%Y-%m-%d')` escaped it only because + // the CAST inserted the parse — which is also why the operand is parsed here through the SAME + // `coerce` arms, so the two spellings cannot accept different format sets. + // + // Executed on ES 8.18.3: both return "2025". + painlessOf("SELECT DATE_FORMAT('2025-01-10', 'yyyy') FROM t") shouldBe + """DateTimeFormatter.ofPattern("yyyy").format(""" + + """LocalDate.parse(("2025-01-10").replace("/", "-"), """ + + """DateTimeFormatter.ofPattern("yyyy-MM-dd")))""" + scriptOf("SELECT DATE_FORMAT(name, 'yyyy') FROM t") shouldBe + "def param1 = (doc['name'].size() == 0 ? null : doc['name'].value); " + + """def param2 = (param1 != null ? LocalDate.parse((param1).replace("/", "-"), """ + + """DateTimeFormatter.ofPattern("yyyy-MM-dd")) : null); """ + + """def param3 = DateTimeFormatter.ofPattern("yyyy"); """ + + "(param1 == null) ? null : param3.format(param2)" + } + + it should "keep that parse INSIDE a null guard" in { + // The parse is bound as its own parameter, so it is EVALUATED before the guard around the + // format call — a document merely MISSING the field would run `LocalDate.parse(null, …)` and + // fail the shard. Executed on ES 8.18.3 against a document without the field: returns null. + // (The first draft passed `nullable = false` here and did exactly that.) + scriptOf("SELECT DATE_FORMAT(name, 'yyyy') FROM t") should include("param1 != null ?") + } + + // -- a call CHAINED onto a guarded standalone call --------------------------------------------- + + "a chained call after a guarded parse" should "land inside the guard, boxed" in { + // 🔴 The second defect the real-index pass found, and the one that was live on every client. + // `.get(ChronoField.YEAR)` returns a PRIMITIVE, and appending it after the parse's own guard + // gave `(param1 == null) ? null : LocalDate.parse(…).get(…)`, which ES 8.18.3 rejects with + // `class_cast_exception: Cannot cast from [int] to [java.lang.Object]` — Painless cannot unify + // a primitive with `null`. So `YEAR(DATE_PARSE(col, fmt))` and `MONTH(...)` did not run at all. + // + // ⚠️ Neither boxing the whole expression nor the null-safe `?.` fixes it: measured, Painless + // types `cond ? null : X` as `Object`, so `(…)?.get(…)` fails with `member method + // [java.lang.Object, get/1] not found`. The method must land INSIDE the guard, which is why + // the guarded expression is bound to a name first. + // + // Executed on ES 8.18.3: 2025 for a document that has the field, null for one that does not. + scriptOf("SELECT YEAR(DATE_PARSE(name, 'yyyy-MM-dd')) FROM t") shouldBe + "def param1 = (doc['name'].size() == 0 ? null : doc['name'].value); " + + """def param2 = (param1 == null) ? null : LocalDate.parse(param1, """ + + """DateTimeFormatter.ofPattern("yyyy-MM-dd")); """ + + "(param2 == null) ? null : (def)(param2.get(ChronoField.YEAR))" + } + + "DATE_TRUNC over a date-only operand" should "not try to truncate a time it does not have" in { + // 🔴 The third: `truncatedTo` is not a member of `java.time.LocalDate`, so + // `DATE_TRUNC(DATE_PARSE(col, fmt), MONTH)` was `member method [java.time.LocalDate, + // truncatedTo/1] not found`. `withDayOfMonth` exists on `LocalDate` and a date-only value has + // no time to truncate, so omitting the truncation is the same value, not an approximation. + // Executed on ES 8.18.3: 2025-01-01, and null for a document without the field. + scriptOf("SELECT DATE_TRUNC(DATE_PARSE(name, 'yyyy-MM-dd'), MONTH) FROM t") shouldBe + "def param1 = (doc['name'].size() == 0 ? null : doc['name'].value); " + + """def param2 = (param1 == null) ? null : LocalDate.parse(param1, """ + + """DateTimeFormatter.ofPattern("yyyy-MM-dd")); """ + + "(param2 == null) ? null : (def)(param2.withDayOfMonth(1))" + } + + "a THREE-function chain" should "guard at every step, not just the first" in { + // 🔴 The case that caught a mistake in the first version of this fix. Binding the guarded + // expression as a PARAMETER made it findable, so the next function appended its method to a + // parameter whose value is a ternary -- outside the guard again, one level up: + // + // (param2 == null) ? null : (def)(param2.truncatedTo(…)).get(ChronoField.YEAR) + // + // It also proves the bridge fixture was pinning a script Elasticsearch REJECTS: the + // `origin/main` emission for this exact statement returns `class_cast_exception: Cannot cast + // from [int] to [java.lang.Object]` on ES 8.18.3, and no test ever ran it. Executed after the + // fix: 2025 for a document with the field, null for one without. + scriptOf( + "SELECT YEAR(DATE_TRUNC(DATETIME_PARSE(name, 'yyyy-MM-dd HH:mm:ss'), MINUTE)) FROM t" + ) shouldBe + "def param1 = (doc['name'].size() == 0 ? null : doc['name'].value); " + + """def param2 = (param1 == null) ? null : ZonedDateTime.parse(param1, """ + + """DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss").withZone(ZoneId.of('Z'))); """ + + "def param3 = (param2 == null) ? null : (def)(param2.truncatedTo(ChronoUnit.MINUTES)); " + + "(param3 == null) ? null : (def)(param3.get(ChronoField.YEAR))" + } + + "a NON-nullable operand" should "keep appending its method, with no extra binding" in { + // The scope of the re-binding rule: it exists only to get a method inside a guard, so where + // there is no guard it must not fire. `LAST_DAY(...)` over a non-nullable operand is + // byte-identical to `origin/main`, and it is what caught the first, unscoped version. + scriptOf("SELECT LAST_DAY(created) FROM t") should not include "(def)(" + } + + it should "still truncate an operand that HAS a time" in { + // The other half of the rule, byte-identical to `origin/main`: an Elasticsearch `date` field + // resolves to a ZonedDateTime, which does carry a time. + scriptOf("SELECT DATE_TRUNC(ts, MONTH) FROM t") should include( + ".withDayOfMonth(1).truncatedTo(ChronoUnit.DAYS)" + ) + } + + "the fusion" should "no longer propagate outward through a wrapping function" in { + // 🔴 Not in the defect record, found re-measuring: the malformed operand+call was embedded in + // whatever wrapped it, so ONE broken assembly corrupted every chain that contained it. + // + // ⚠️ This emission is pinned because the FUSION is gone, and deliberately NOT claimed to run: + // it is still the unboxed `(cond) ? null : ` shape the test above fixes ON THE + // CONTEXT-BEARING PATH. It cannot be fixed here, and the reason is structural rather than an + // oversight: the context-free renderer guards by emitting STATEMENTS (`def arg0 = …;`), so + // there is no bound name to hang the method on and no way to reach inside the guard. Making + // that path executable means changing how it guards, which is a different change with no + // production surface behind it — every shipped surface that reaches these functions renders + // WITH a context. Pinned so the day someone does change it, this test says so. + painlessOf("SELECT YEAR(DATE_PARSE(name, 'yyyy-MM-dd')) FROM t") shouldBe + "def arg0 = (doc['name'].size() == 0 ? null : doc['name'].value); (arg0 == null) ? null : " + + """LocalDate.parse(arg0, DateTimeFormatter.ofPattern("yyyy-MM-dd")).get(ChronoField.YEAR)""" + } + + // -- the production rendering is untouched ----------------------------------------------------- + + "the context-bearing rendering" should "be byte-identical to before the fix" in { + // The branch that every shipped surface takes was already correct, and the whole point of + // taking the decision at the assembly is that the two renderings now agree BY CONSTRUCTION. + // These bytes are the pre-fix ones, captured on the baseline. + scriptOf("SELECT DATE_PARSE(name, 'yyyy-MM-dd') FROM t") shouldBe + "def param1 = (doc['name'].size() == 0 ? null : doc['name'].value); (param1 == null) ? null : " + + """LocalDate.parse(param1, DateTimeFormatter.ofPattern("yyyy-MM-dd"))""" + scriptOf("SELECT DATE_FORMAT(created, 'yyyy') FROM t") shouldBe + "def param1 = (doc['created'].size() == 0 ? null : doc['created'].value); " + + """def param2 = DateTimeFormatter.ofPattern("yyyy"); """ + + "(param1 == null) ? null : param2.format(param1)" + } + + "a CHAINED call" should "still take the operand as its prefix" in { + // The other half of the rule: `.get(…)`, `.truncatedTo(…)`, `.plus(…)` are methods ON the + // operand, so dropping the base there would emit a call with no receiver. Byte-identical. + painlessOf("SELECT YEAR(created) FROM t") shouldBe + "(doc['created'].size() == 0 ? null : doc['created'].value).get(ChronoField.YEAR)" + painlessOf("SELECT DATE_TRUNC(ts, MONTH) FROM t") shouldBe + "(doc['ts'].size() == 0 ? null : doc['ts'].value).withDayOfMonth(1).truncatedTo(ChronoUnit.DAYS)" + } + + // -- AC-5: the invariant, over the whole family ------------------------------------------------ + + /** One statement per date/time `TransformFunction`. The list is not the guard — the source scan + * at the bottom is: it fails when a new function joins the family without joining this list. + * + * `DATE_DIFF` is deliberately absent and its absence is CHECKED rather than assumed: `DateDiff` + * is a `BinaryFunction`, so it binds its own arguments and is never handed to + * `TransformFunction.toPainless` at all. The scan below derives the family from the source, and + * `DateDiff`'s declaration does not reach `TransformFunction` — so if that ever changes, the + * coverage test reds and this list has to grow. + */ + private val family: List[String] = List( + "SELECT DATE_TRUNC(ts, MONTH) FROM t", + "SELECT YEAR(created) FROM t", + "SELECT EXTRACT(YEAR FROM ts) FROM t", + "SELECT LAST_DAY(created) FROM t", + "SELECT DATE_ADD(created, INTERVAL 1 DAY) FROM t", + "SELECT DATE_SUB(created, INTERVAL 1 DAY) FROM t", + "SELECT DATE_PARSE(name, 'yyyy-MM-dd') FROM t", + "SELECT DATE_FORMAT(created, 'yyyy') FROM t", + "SELECT DATETIME_ADD(ts, INTERVAL 1 HOUR) FROM t", + "SELECT DATETIME_SUB(ts, INTERVAL 1 HOUR) FROM t", + "SELECT DATETIME_PARSE(name, 'yyyy-MM-dd HH:mm:ss') FROM t", + "SELECT DATETIME_FORMAT(ts, 'yyyy') FROM t", + "SELECT created + INTERVAL 1 DAY FROM t", + "SELECT created - INTERVAL 1 DAY FROM t" + ) + + private def transformsOf(sql: String): List[TransformFunction[_, _]] = + FunctionUtils.transformFunctions(field(sql).identifier).collect { + case tf: TransformFunction[_, _] => tf + } + + private val Sentinel = "__OPERAND__" + + behavior of "every date and time transform function" + + it should "never append a standalone call to the operand (AC-5)" in { + // The invariant, asserted on the ASSEMBLY rather than inferred from a rendered string: if the + // function's own rendering does not start with `.`, it is a complete expression that already + // carries the operand, so it must be emitted at position 0 — never appended to anything. + // + // Falsified: reverting the one-line assembly fix reds this for DATE_PARSE, DATE_FORMAT, + // DATETIME_PARSE and DATETIME_FORMAT and leaves the ten chained members green. + family.foreach { sql => + transformsOf(sql).foreach { fn => + val call = fn.painless(None) + if (!call.startsWith(".")) { + val assembled = fn.toPainless(Sentinel, 7, None) + withClue(s"[$sql] ${fn.getClass.getSimpleName} assembled=[$assembled] call=[$call] ") { + assembled.indexOf(call) should (be(-1) or be(0)) + } + } + } + } + } + + it should "leave no operand fused to the token that follows it" in { + // The same property read off the emission rather than off the assembly, and it needs BOTH + // names to be honest. The sentinel stands in for the operand where it is inlined; where the + // assembly binds it first (`def e7 = ;`) the operand is thereafter called `e7`, and + // THAT is where the original corruption showed — `e0def arg0 = …`, two tokens fused into one + // identifier that no lexer can flag. A first draft of this test scanned the sentinel only and + // stayed GREEN under the mutation it claims to catch; a gate that its own falsification cannot + // red is not a gate. + // + // The follow set is Painless's own: an operand may be followed by an operator, a separator, a + // closing bracket, a member access or nothing at all — never by a bare word. + val mayFollow = ".,;)]}?: \t=!<>+-*/%&|^".toSet + def isIdentChar(c: Char): Boolean = c.isLetterOrDigit || c == '_' || c == '$' + def fusions(text: String, token: String): List[String] = { + var out = List.empty[String] + var i = text.indexOf(token) + while (i >= 0) { + val before = i == 0 || !isIdentChar(text.charAt(i - 1)) + val next = i + token.length + if (before && next < text.length && !mayFollow.contains(text.charAt(next))) + out = out :+ text.substring(i, scala.math.min(next + 12, text.length)) + i = text.indexOf(token, next) + } + out + } + family.foreach { sql => + transformsOf(sql).foreach { fn => + val assembled = fn.toPainless(Sentinel, 7, None) + val found = fusions(assembled, Sentinel) ++ fusions(assembled, "e7") + withClue(s"[$sql] ${fn.getClass.getSimpleName} in [$assembled] ") { + found shouldBe empty + } + } + } + } + + it should "have exercised at least one transform per family statement" in { + // Guards the guard: a statement that yields no `TransformFunction` makes both loops above + // iterate zero times and pass vacuously. + family.foreach { sql => + withClue(s"[$sql] ") { transformsOf(sql) should not be empty } + } + } + + // -- anti-drift: the family list may not fall behind the source -------------------------------- + + private val timeSourceCandidates = Seq( + new File("sql/src/main/scala/app/softnetwork/elastic/sql/function/time/package.scala"), + new File("src/main/scala/app/softnetwork/elastic/sql/function/time/package.scala") + ) + + private def timeSource: Option[File] = timeSourceCandidates.find(_.isFile) + + private def read(f: File): String = { + val src = Source.fromFile(f, "UTF-8") + try src.mkString + finally src.close() + } + + /** Every `case class` in the time package that is a `TransformFunction`, resolved through the + * traits in the same file rather than hard-coded: a declaration counts when its `extends` text + * names `TransformFunction[` directly, or names a trait already known to be one. Iterated to a + * fixpoint so `SQLAddInterval extends AddInterval extends IntervalFunction extends + * TransformFunction` is reached. + */ + private def transformCaseClasses: Set[String] = { + val text = timeSource.map(read).getOrElse("") + // A declaration is its header: from `case class X` / `sealed trait X` up to the body's `{`. + val decl = """(?s)\b(case class|sealed trait|trait)\s+(\w+)[^{]*""".r + val decls = decl + .findAllMatchIn(text) + .map(m => (m.group(1), m.group(2), m.matched)) + .toList + var known = Set("TransformFunction") + var changed = true + while (changed) { + val next = known ++ decls.collect { + case (_, name, header) if known.exists(k => header.contains(s"$k[")) => name + } + changed = next != known + known = next + } + decls.collect { case ("case class", name, _) if known.contains(name) => name }.toSet + } + + /** The runtime class AND every class it inherits from. + * + * 🔴 Not the simple name alone: `YEAR(x)` instantiates `Year`, which extends + * `TimeFieldExtract(YEAR)`, which extends the declared `case class Extract`. Comparing leaf + * names only would have reported `Extract` as uncovered while the very statement that exercises + * its assembly was in the list — a guard failing on its own bookkeeping rather than on a real + * gap. + */ + private def exercisedNames: Set[String] = + family + .flatMap(transformsOf) + .flatMap { fn => + Iterator + .iterate[Class[_]](fn.getClass)(_.getSuperclass) + .takeWhile(_ != null) + .map(_.getSimpleName) + } + .toSet + + behavior of "the family list" + + // NOT `assume`: a cancelled test is not a failure and `sbt test` still exits 0, so the scan + // would pass while having scanned nothing. + it should "have the time package on disk to scan" in { + withClue(s"working directory = ${new File(".").getAbsolutePath} - ") { + timeSource shouldBe defined + } + } + + it should "have found more than one transform case class to compare against" in { + transformCaseClasses.size should be > 5 + } + + it should "cover every transform function declared in the time package" in { + // The reason this file can claim to guard a FAMILY rather than four cases: a new date function + // reds this test on the day it is added, until it is exercised above. + (transformCaseClasses -- exercisedNames) shouldBe empty + } +} diff --git a/sql/src/test/scala/app/softnetwork/elastic/sql/parser/ParserSpec.scala b/sql/src/test/scala/app/softnetwork/elastic/sql/parser/ParserSpec.scala index fd0cb442..7f5f3f06 100644 --- a/sql/src/test/scala/app/softnetwork/elastic/sql/parser/ParserSpec.scala +++ b/sql/src/test/scala/app/softnetwork/elastic/sql/parser/ParserSpec.scala @@ -1476,11 +1476,38 @@ class ParserSpec extends AnyFlatSpec with Matchers { .script .map(p => p.source) .getOrElse("") should include( + // ⚠️ `cols` is the RAW parsed column list, not `ct.schema.columns`: the schema is + // attached by `Table.update()` when the schema is built (story 21.8 Part G), so this + // pin is the UNRESOLVED processor and its operand is still the raw `ctx.birthdate`. + // The resolved form -- the one `CREATE TABLE` actually deploys -- is asserted below. + // + // What DID move here is the ingest clock. The old emission read the timestamp from + // `ctx` under the `_ingest` key, and that is NULL on ES 6.8, 7.17, 8.18 AND 9.0 -- + // verified on real indices, not `_simulate` -- so `CURRENT_DATE` inside `SCRIPT AS` has + // never worked on any supported version and this column was silently absent. + // `System.currentTimeMillis()` is whitelisted on all four and is already the unit the + // surrounding `Instant.ofEpochMilli(...)` expects. `CURRENT_DATE` also stops narrowing + // to a `LocalDate` in a processor, so both sides of `between` are the same type. """def param1 = ctx.birthdate; - |def param2 = ZonedDateTime.ofInstant(Instant.ofEpochMilli(ctx['_ingest']['timestamp']), ZoneId.of('Z')).toLocalDate(); + |def param2 = ZonedDateTime.ofInstant(Instant.ofEpochMilli(System.currentTimeMillis()), ZoneId.of('Z')); |def param3 = Long.valueOf(ChronoUnit.YEARS.between(param1, param2)); |ctx.age = (param1 == null) ? null : param3""".stripMargin.replaceAll("\n", " ") ) + // The RESOLVED processor, which is what `CREATE TABLE` deploys. The operand is parsed + // first and its runtime shape decided by `instanceof`, because `ctx.birthdate` is the raw + // JSON value of the document being indexed. This exact pipeline was run on all four + // majors: {"birthdate":"1990-05-20"} and {"birthdate":643161600000} both store `age: 36`. + ct.schema.columns + .find(_.name == "age") + .flatMap(_.script) + .map(_.source) + .getOrElse("") should include( + """def param1 = ctx.birthdate; + |def param2 = (param1 instanceof String ? LocalDate.parse((param1).replace("/", "-"), DateTimeFormatter.ofPattern("yyyy-MM-dd")).atStartOfDay(ZoneId.of('Z')) : Instant.ofEpochMilli(param1).atZone(ZoneId.of('Z'))); + |def param3 = ZonedDateTime.ofInstant(Instant.ofEpochMilli(System.currentTimeMillis()), ZoneId.of('Z')); + |def param4 = Long.valueOf(ChronoUnit.YEARS.between(param2, param3)); + |ctx.age = (param1 == null) ? null : param4""".stripMargin.replaceAll("\n", " ") + ) cols.find(_.name == "ingested_at").get.defaultValue.map(_.value) shouldBe Some( "_ingest.timestamp" ) @@ -1491,16 +1518,16 @@ class ParserSpec extends AnyFlatSpec with Matchers { println(schema.defaultPipeline.ddl) val json = schema.defaultPipeline.json println(json) - json shouldBe """{"description":"CREATE OR REPLACE PIPELINE users_ddl_default_pipeline WITH PROCESSORS (name SET DEFAULT 'anonymous', age INT SCRIPT AS (DATE_DIFF(birthdate, CURRENT_DATE, YEAR)), ingested_at SET DEFAULT _ingest.timestamp, profile.seniority INT SCRIPT AS (DATE_DIFF(profile.join_date, CURRENT_DATE, DAY)), PARTITION BY birthdate (MONTH), PRIMARY KEY (id))","processors":[{"set":{"description":"name SET DEFAULT 'anonymous'","field":"name","ignore_failure":true,"value":"anonymous","if":"ctx.name == null"}},{"script":{"description":"age INT SCRIPT AS (DATE_DIFF(birthdate, CURRENT_DATE, YEAR))","lang":"painless","source":"def param1 = ctx.birthdate; def param2 = ZonedDateTime.ofInstant(Instant.ofEpochMilli(ctx['_ingest']['timestamp']), ZoneId.of('Z')).toLocalDate(); def param3 = Long.valueOf(ChronoUnit.YEARS.between(param1, param2)); ctx.age = (param1 == null) ? null : param3","ignore_failure":true}},{"set":{"description":"ingested_at SET DEFAULT _ingest.timestamp","field":"ingested_at","ignore_failure":true,"value":"{{_ingest.timestamp}}","if":"ctx.ingested_at == null"}},{"script":{"description":"profile.seniority INT SCRIPT AS (DATE_DIFF(profile.join_date, CURRENT_DATE, DAY))","lang":"painless","source":"def param1 = ctx.profile?.join_date; def param2 = ZonedDateTime.ofInstant(Instant.ofEpochMilli(ctx['_ingest']['timestamp']), ZoneId.of('Z')).toLocalDate(); def param3 = Long.valueOf(ChronoUnit.DAYS.between(param1, param2)); ctx.profile.seniority = (param1 == null) ? null : param3","ignore_failure":true}},{"date_index_name":{"description":"PARTITION BY birthdate (MONTH)","field":"birthdate","date_rounding":"M","date_formats":["yyyy-MM"],"index_name_prefix":"users-","ignore_failure":true}},{"set":{"description":"PRIMARY KEY (id)","field":"_id","value":"{{id}}","ignore_failure":false,"ignore_empty_value":false}}]}""" + json shouldBe """{"description":"CREATE OR REPLACE PIPELINE users_ddl_default_pipeline WITH PROCESSORS (name SET DEFAULT 'anonymous', age INT SCRIPT AS (DATE_DIFF(birthdate, CURRENT_DATE, YEAR)), ingested_at SET DEFAULT _ingest.timestamp, profile.seniority INT SCRIPT AS (DATE_DIFF(profile.join_date, CURRENT_DATE, DAY)), PARTITION BY birthdate (MONTH), PRIMARY KEY (id))","processors":[{"set":{"description":"name SET DEFAULT 'anonymous'","field":"name","ignore_failure":true,"value":"anonymous","if":"ctx.name == null"}},{"script":{"description":"age INT SCRIPT AS (DATE_DIFF(birthdate, CURRENT_DATE, YEAR))","lang":"painless","source":"def param1 = ctx.birthdate; def param2 = (param1 instanceof String ? LocalDate.parse((param1).replace(\"/\", \"-\"), DateTimeFormatter.ofPattern(\"yyyy-MM-dd\")).atStartOfDay(ZoneId.of('Z')) : Instant.ofEpochMilli(param1).atZone(ZoneId.of('Z'))); def param3 = ZonedDateTime.ofInstant(Instant.ofEpochMilli(System.currentTimeMillis()), ZoneId.of('Z')); def param4 = Long.valueOf(ChronoUnit.YEARS.between(param2, param3)); ctx.age = (param1 == null) ? null : param4","ignore_failure":true}},{"set":{"description":"ingested_at SET DEFAULT _ingest.timestamp","field":"ingested_at","ignore_failure":true,"value":"{{_ingest.timestamp}}","if":"ctx.ingested_at == null"}},{"script":{"description":"profile.seniority INT SCRIPT AS (DATE_DIFF(profile.join_date, CURRENT_DATE, DAY))","lang":"painless","source":"def param1 = ctx.profile?.join_date; def param2 = (param1 instanceof String ? LocalDate.parse((param1).replace(\"/\", \"-\"), DateTimeFormatter.ofPattern(\"yyyy-MM-dd\")).atStartOfDay(ZoneId.of('Z')) : Instant.ofEpochMilli(param1).atZone(ZoneId.of('Z'))); def param3 = ZonedDateTime.ofInstant(Instant.ofEpochMilli(System.currentTimeMillis()), ZoneId.of('Z')); def param4 = Long.valueOf(ChronoUnit.DAYS.between(param2, param3)); ctx.profile.seniority = (param1 == null) ? null : param4","ignore_failure":true}},{"date_index_name":{"description":"PARTITION BY birthdate (MONTH)","field":"birthdate","date_rounding":"M","date_formats":["yyyy-MM"],"index_name_prefix":"users-","ignore_failure":true}},{"set":{"description":"PRIMARY KEY (id)","field":"_id","value":"{{id}}","ignore_failure":false,"ignore_empty_value":false}}]}""" val indexMappings = schema.indexMappings println(indexMappings) - indexMappings.toString shouldBe """{"properties":{"id":{"type":"integer"},"name":{"type":"text","fields":{"raw":{"type":"keyword"}},"analyzer":"french","search_analyzer":"french"},"birthdate":{"type":"date"},"age":{"type":"integer"},"ingested_at":{"type":"date"},"profile":{"type":"object","properties":{"bio":{"type":"text"},"followers":{"type":"integer"},"join_date":{"type":"date"},"seniority":{"type":"integer"}}}},"dynamic":false,"_meta":{"primary_key":["id"],"partition_by":{"column":"birthdate","granularity":"M"},"columns":{"id":{"data_type":"INT","not_null":"true","comment":"user identifier"},"name":{"data_type":"VARCHAR","not_null":"false","default_value":"anonymous","multi_fields":{"raw":{"data_type":"KEYWORD","not_null":"false","comment":"sortable"}}},"birthdate":{"data_type":"DATE","not_null":"false"},"age":{"data_type":"INT","not_null":"false","script":{"sql":"DATE_DIFF(birthdate, CURRENT_DATE, YEAR)","column":"age","painless":"def param1 = ctx.birthdate; def param2 = ZonedDateTime.ofInstant(Instant.ofEpochMilli(ctx['_ingest']['timestamp']), ZoneId.of('Z')).toLocalDate(); def param3 = Long.valueOf(ChronoUnit.YEARS.between(param1, param2)); ctx.age = (param1 == null) ? null : param3"}},"ingested_at":{"data_type":"TIMESTAMP","not_null":"false","default_value":"_ingest.timestamp"},"profile":{"data_type":"STRUCT","not_null":"false","comment":"user profile","multi_fields":{"bio":{"data_type":"VARCHAR","not_null":"false"},"followers":{"data_type":"INT","not_null":"false"},"join_date":{"data_type":"DATE","not_null":"false"},"seniority":{"data_type":"INT","not_null":"false","script":{"sql":"DATE_DIFF(profile.join_date, CURRENT_DATE, DAY)","column":"profile.seniority","painless":"def param1 = ctx.profile?.join_date; def param2 = ZonedDateTime.ofInstant(Instant.ofEpochMilli(ctx['_ingest']['timestamp']), ZoneId.of('Z')).toLocalDate(); def param3 = Long.valueOf(ChronoUnit.DAYS.between(param1, param2)); ctx.profile.seniority = (param1 == null) ? null : param3"}}}}},"type":"regular"}}""".stripMargin + indexMappings.toString shouldBe """{"properties":{"id":{"type":"integer"},"name":{"type":"text","fields":{"raw":{"type":"keyword"}},"analyzer":"french","search_analyzer":"french"},"birthdate":{"type":"date"},"age":{"type":"integer"},"ingested_at":{"type":"date"},"profile":{"type":"object","properties":{"bio":{"type":"text"},"followers":{"type":"integer"},"join_date":{"type":"date"},"seniority":{"type":"integer"}}}},"dynamic":false,"_meta":{"primary_key":["id"],"partition_by":{"column":"birthdate","granularity":"M"},"columns":{"id":{"data_type":"INT","not_null":"true","comment":"user identifier"},"name":{"data_type":"VARCHAR","not_null":"false","default_value":"anonymous","multi_fields":{"raw":{"data_type":"KEYWORD","not_null":"false","comment":"sortable"}}},"birthdate":{"data_type":"DATE","not_null":"false"},"age":{"data_type":"INT","not_null":"false","script":{"sql":"DATE_DIFF(birthdate, CURRENT_DATE, YEAR)","column":"age","painless":"def param1 = ctx.birthdate; def param2 = (param1 instanceof String ? LocalDate.parse((param1).replace(\"/\", \"-\"), DateTimeFormatter.ofPattern(\"yyyy-MM-dd\")).atStartOfDay(ZoneId.of('Z')) : Instant.ofEpochMilli(param1).atZone(ZoneId.of('Z'))); def param3 = ZonedDateTime.ofInstant(Instant.ofEpochMilli(System.currentTimeMillis()), ZoneId.of('Z')); def param4 = Long.valueOf(ChronoUnit.YEARS.between(param2, param3)); ctx.age = (param1 == null) ? null : param4"}},"ingested_at":{"data_type":"TIMESTAMP","not_null":"false","default_value":"_ingest.timestamp"},"profile":{"data_type":"STRUCT","not_null":"false","comment":"user profile","multi_fields":{"bio":{"data_type":"VARCHAR","not_null":"false"},"followers":{"data_type":"INT","not_null":"false"},"join_date":{"data_type":"DATE","not_null":"false"},"seniority":{"data_type":"INT","not_null":"false","script":{"sql":"DATE_DIFF(profile.join_date, CURRENT_DATE, DAY)","column":"profile.seniority","painless":"def param1 = ctx.profile?.join_date; def param2 = (param1 instanceof String ? LocalDate.parse((param1).replace(\"/\", \"-\"), DateTimeFormatter.ofPattern(\"yyyy-MM-dd\")).atStartOfDay(ZoneId.of('Z')) : Instant.ofEpochMilli(param1).atZone(ZoneId.of('Z'))); def param3 = ZonedDateTime.ofInstant(Instant.ofEpochMilli(System.currentTimeMillis()), ZoneId.of('Z')); def param4 = Long.valueOf(ChronoUnit.DAYS.between(param2, param3)); ctx.profile.seniority = (param1 == null) ? null : param4"}}}}},"type":"regular"}}""".stripMargin val indexSettings = schema.indexSettings println(indexSettings) indexSettings.toString shouldBe """{"index":{}}""" val pipeline = schema.defaultPipelineNode println(pipeline) - pipeline.toString shouldBe """{"description":"CREATE OR REPLACE PIPELINE users_ddl_default_pipeline WITH PROCESSORS (name SET DEFAULT 'anonymous', age INT SCRIPT AS (DATE_DIFF(birthdate, CURRENT_DATE, YEAR)), ingested_at SET DEFAULT _ingest.timestamp, profile.seniority INT SCRIPT AS (DATE_DIFF(profile.join_date, CURRENT_DATE, DAY)), PARTITION BY birthdate (MONTH), PRIMARY KEY (id))","processors":[{"set":{"description":"name SET DEFAULT 'anonymous'","field":"name","ignore_failure":true,"value":"anonymous","if":"ctx.name == null"}},{"script":{"description":"age INT SCRIPT AS (DATE_DIFF(birthdate, CURRENT_DATE, YEAR))","lang":"painless","source":"def param1 = ctx.birthdate; def param2 = ZonedDateTime.ofInstant(Instant.ofEpochMilli(ctx['_ingest']['timestamp']), ZoneId.of('Z')).toLocalDate(); def param3 = Long.valueOf(ChronoUnit.YEARS.between(param1, param2)); ctx.age = (param1 == null) ? null : param3","ignore_failure":true}},{"set":{"description":"ingested_at SET DEFAULT _ingest.timestamp","field":"ingested_at","ignore_failure":true,"value":"{{_ingest.timestamp}}","if":"ctx.ingested_at == null"}},{"script":{"description":"profile.seniority INT SCRIPT AS (DATE_DIFF(profile.join_date, CURRENT_DATE, DAY))","lang":"painless","source":"def param1 = ctx.profile?.join_date; def param2 = ZonedDateTime.ofInstant(Instant.ofEpochMilli(ctx['_ingest']['timestamp']), ZoneId.of('Z')).toLocalDate(); def param3 = Long.valueOf(ChronoUnit.DAYS.between(param1, param2)); ctx.profile.seniority = (param1 == null) ? null : param3","ignore_failure":true}},{"date_index_name":{"description":"PARTITION BY birthdate (MONTH)","field":"birthdate","date_rounding":"M","date_formats":["yyyy-MM"],"index_name_prefix":"users-","ignore_failure":true}},{"set":{"description":"PRIMARY KEY (id)","field":"_id","value":"{{id}}","ignore_failure":false,"ignore_empty_value":false}}]}""" + pipeline.toString shouldBe """{"description":"CREATE OR REPLACE PIPELINE users_ddl_default_pipeline WITH PROCESSORS (name SET DEFAULT 'anonymous', age INT SCRIPT AS (DATE_DIFF(birthdate, CURRENT_DATE, YEAR)), ingested_at SET DEFAULT _ingest.timestamp, profile.seniority INT SCRIPT AS (DATE_DIFF(profile.join_date, CURRENT_DATE, DAY)), PARTITION BY birthdate (MONTH), PRIMARY KEY (id))","processors":[{"set":{"description":"name SET DEFAULT 'anonymous'","field":"name","ignore_failure":true,"value":"anonymous","if":"ctx.name == null"}},{"script":{"description":"age INT SCRIPT AS (DATE_DIFF(birthdate, CURRENT_DATE, YEAR))","lang":"painless","source":"def param1 = ctx.birthdate; def param2 = (param1 instanceof String ? LocalDate.parse((param1).replace(\"/\", \"-\"), DateTimeFormatter.ofPattern(\"yyyy-MM-dd\")).atStartOfDay(ZoneId.of('Z')) : Instant.ofEpochMilli(param1).atZone(ZoneId.of('Z'))); def param3 = ZonedDateTime.ofInstant(Instant.ofEpochMilli(System.currentTimeMillis()), ZoneId.of('Z')); def param4 = Long.valueOf(ChronoUnit.YEARS.between(param2, param3)); ctx.age = (param1 == null) ? null : param4","ignore_failure":true}},{"set":{"description":"ingested_at SET DEFAULT _ingest.timestamp","field":"ingested_at","ignore_failure":true,"value":"{{_ingest.timestamp}}","if":"ctx.ingested_at == null"}},{"script":{"description":"profile.seniority INT SCRIPT AS (DATE_DIFF(profile.join_date, CURRENT_DATE, DAY))","lang":"painless","source":"def param1 = ctx.profile?.join_date; def param2 = (param1 instanceof String ? LocalDate.parse((param1).replace(\"/\", \"-\"), DateTimeFormatter.ofPattern(\"yyyy-MM-dd\")).atStartOfDay(ZoneId.of('Z')) : Instant.ofEpochMilli(param1).atZone(ZoneId.of('Z'))); def param3 = ZonedDateTime.ofInstant(Instant.ofEpochMilli(System.currentTimeMillis()), ZoneId.of('Z')); def param4 = Long.valueOf(ChronoUnit.DAYS.between(param2, param3)); ctx.profile.seniority = (param1 == null) ? null : param4","ignore_failure":true}},{"date_index_name":{"description":"PARTITION BY birthdate (MONTH)","field":"birthdate","date_rounding":"M","date_formats":["yyyy-MM"],"index_name_prefix":"users-","ignore_failure":true}},{"set":{"description":"PRIMARY KEY (id)","field":"_id","value":"{{id}}","ignore_failure":false,"ignore_empty_value":false}}]}""" // Reconstruct EsIndex val mappings = mapper.createObjectNode() mappings.set("mappings", indexMappings) @@ -3902,7 +3929,7 @@ class ParserSpec extends AnyFlatSpec with Matchers { case Some(updatedAtProc) => updatedAtProc .asInstanceOf[ScriptProcessor] - .source shouldBe "def param1 = ZonedDateTime.ofInstant(Instant.ofEpochMilli(ctx['_ingest']['timestamp']), ZoneId.of('Z')); ctx.updated_at = param1" + .source shouldBe "def param1 = ZonedDateTime.ofInstant(Instant.ofEpochMilli(System.currentTimeMillis()), ZoneId.of('Z')); ctx.updated_at = param1" case None => fail("Expected processor for updated_at") } case _ => fail("Expected Update") diff --git a/sql/src/test/scala/app/softnetwork/elastic/sql/schema/DdlScriptSchemaSpec.scala b/sql/src/test/scala/app/softnetwork/elastic/sql/schema/DdlScriptSchemaSpec.scala index 594fcb06..580ccc05 100644 --- a/sql/src/test/scala/app/softnetwork/elastic/sql/schema/DdlScriptSchemaSpec.scala +++ b/sql/src/test/scala/app/softnetwork/elastic/sql/schema/DdlScriptSchemaSpec.scala @@ -75,16 +75,53 @@ class DdlScriptSchemaSpec extends AnyFlatSpec with Matchers { // -- AC-G3: the ingest-context guard ---------------------------------------------------------- - "a TEMPORAL-source arm" should "NOT fire in ingest context, byte for byte" in { + "a TEMPORAL-source arm" should "still decline in ingest context" in { // 🔴 In an ingest script the operand is `ctx.` — the RAW JSON scalar — not the temporal // object a query's `doc['f'].value` yields, so an arm keyed on a temporal SOURCE must decline. - // Attaching the schema is what makes those arms reachable for the first time, which turns that - // guard from theoretical into load-bearing. The expected bytes are the PRE-fix emission, - // captured on the baseline: this column's script must not have moved at all. + // That guard is unchanged and still load-bearing: no `.toInstant()` chain is emitted here. + val source = createSource( + "CREATE TABLE t (created DATE, y INTEGER SCRIPT AS (YEAR(created)))", + "y" + ) + source should not include ".toInstant()" + } + + it should "PARSE the raw value first, deciding its shape at runtime (story 21.8 Part C)" in { + // ⚠️ This pin REPLACES story 21.8's AC-G3 "byte for byte" expectation, deliberately. That + // expectation recorded a script which — MEASURED on real ingest pipelines — threw + // `String.get(ChronoField)`, was swallowed by `ignore_failure: true`, and left the column + // silently ABSENT. Pinning bytes proved they had not moved; it could not notice they did not + // work. Part C fixes the level above the guard: the operand is parsed into a temporal BEFORE + // the function is applied. + // + // The shape is decided at RUNTIME rather than guessed, because Elasticsearch accepts BOTH an + // ISO string and epoch millis into a `date` field (the lead's ruling). The previous code had + // answered that question one way, hard-coded and untested, and was wrong for every documented + // example. Verified as an ingest pipeline on ES 6.8.23, 7.17.29, 8.18.3 and 9.0.3 against + // {"created":"2025-01-10"}, {"created":"2025/01/10"} and {"created":1736467200000}: all three + // store `y: 2025`. createSource( "CREATE TABLE t (created DATE, y INTEGER SCRIPT AS (YEAR(created)))", "y" - ) shouldBe "def param1 = ctx.created.get(ChronoField.YEAR); ctx.y = param1" + ) shouldBe + "def param1 = (ctx.created instanceof String ? " + + """LocalDate.parse((ctx.created).replace("/", "-"), """ + + """DateTimeFormatter.ofPattern("yyyy-MM-dd")).atStartOfDay(ZoneId.of('Z')) """ + + ": Instant.ofEpochMilli(ctx.created).atZone(ZoneId.of('Z'))).get(ChronoField.YEAR); " + + "ctx.y = param1" + } + + it should "leave a NON-temporal column untouched" in { + // The scope of the parse: a `keyword` is a `String` in a query and in `ctx` alike, so nothing + // about it needs deciding at runtime. These two are byte-identical to `origin/main`. + createSource( + "CREATE TABLE t (zip_code KEYWORD, zip_n BIGINT SCRIPT AS (CAST(zip_code AS BIGINT)))", + "zip_n" + ) should not include "instanceof" + createSource( + "CREATE TABLE t (a KEYWORD, b KEYWORD SCRIPT AS (UPPER(a)))", + "b" + ) shouldBe "def param1 = ctx.a; ctx.b = (param1 == null) ? null : param1.toUpperCase()" } "a string function over a sibling KEYWORD" should "be unchanged" in { diff --git a/sql/src/test/scala/app/softnetwork/elastic/sql/schema/IngestTemporalSpec.scala b/sql/src/test/scala/app/softnetwork/elastic/sql/schema/IngestTemporalSpec.scala new file mode 100644 index 00000000..d88d6ca1 --- /dev/null +++ b/sql/src/test/scala/app/softnetwork/elastic/sql/schema/IngestTemporalSpec.scala @@ -0,0 +1,161 @@ +package app.softnetwork.elastic.sql.schema + +import app.softnetwork.elastic.sql.parser.Parser +import app.softnetwork.elastic.sql.query.CreateTable +import org.scalatest.flatspec.AnyFlatSpec +import org.scalatest.matchers.should.Matchers + +/** Story 21.8 Part C — a date function over a date COLUMN was unusable in an ingest script. + * + * `CREATE TABLE t (created DATE, y INTEGER SCRIPT AS (YEAR(created)))` emitted + * `ctx.created.get(ChronoField.YEAR)`. In an ingest script `ctx.` is the RAW JSON value of + * the document being indexed — the string `"2025-01-10"`, not a temporal object — so `String` has + * no `get(ChronoField)`, the script threw, `ignore_failure: true` swallowed the throw, and the + * computed column was simply ABSENT from the stored document. Measured on real Elasticsearch for + * `YEAR`, `MONTH`, `DATE_TRUNC`, `DATE_ADD`/`DATE_SUB`, `DATE_FORMAT` and `DATE_DIFF` — and + * `DATE_DIFF(birthdate, CURRENT_DATE, YEAR)` is a PUBLISHED example + * (`documentation/sql/ddl_statements.md`, and the REPL testkit's `users` table). + * + * 🔴 Three separate things had to be true for that example to work, and only the first was known: + * + * 1. the OPERAND must be parsed into a temporal. Its runtime shape is decided by `instanceof` + * rather than guessed, because Elasticsearch accepts BOTH an ISO string and epoch millis into + * a `date` field (the lead's ruling). The previous code answered that question one way, + * hard-coded as `from = SQLTypes.BigInt`, with no test — and wrong for every documented + * example, which ingests strings; 2. the ingest CLOCK was `ctx['_ingest']['timestamp']`, + * which is NULL. Verified on REAL indices, not `_simulate`, on ES 6.8.23, 7.17.29, 8.18.3 and + * 9.0.3 — so `CURRENT_DATE`, `CURRENT_TIMESTAMP`, `NOW` and `TODAY` inside `SCRIPT AS` have + * never worked on any supported version. `System.currentTimeMillis()` is whitelisted on all + * four, and is already the unit the surrounding `Instant.ofEpochMilli(...)` expects; 3. every + * temporal value in a processor must be the SAME Java type. `ChronoUnit.between` refuses a + * `ZonedDateTime` paired with a `LocalDate`, and Elasticsearch refuses a `LocalDate` assigned + * back into `ctx.` at all (`illegal_argument_exception: unexpected value type [class + * java.time.LocalDate]`). So a processor collapses to `ZonedDateTime` — the same collapse + * `SQLTypeUtils.runtimeType` already applies to a query. + * + * ⚠️ Every emission below was executed as a real ingest pipeline on ES 6.8.23, 7.17.29, 8.18.3 AND + * 9.0.3, against both document shapes. `DATE_DIFF(birthdate, CURRENT_DATE, YEAR)` stores `age: 36` + * for `{"birthdate":"1990-05-20"}` and for `{"birthdate":643161600000}` on all four. + * + * KNOWN AND UNCHANGED: a document MISSING the source field still leaves the computed column + * absent. `instanceof` on null throws and `ignore_failure: true` swallows it, which is the same + * outcome — and the same mechanism — as before this story. + */ +class IngestTemporalSpec extends AnyFlatSpec with Matchers { + + private def source(sql: String, column: String): String = + Parser(sql) match { + case Right(ct: CreateTable) => + ct.schema.columns + .find(_.name == column) + .flatMap(_.script) + .map(_.source) + .getOrElse(fail(s"no script processor for [$column] in [$sql]")) + case other => fail(s"[$sql] expected a CreateTable, got $other") + } + + private val family = List( + "CREATE TABLE t (created DATE, c INTEGER SCRIPT AS (YEAR(created)))", + "CREATE TABLE t (created DATE, c INTEGER SCRIPT AS (MONTH(created)))", + "CREATE TABLE t (created DATE, c DATE SCRIPT AS (DATE_TRUNC(created, MONTH)))", + "CREATE TABLE t (created DATE, c DATE SCRIPT AS (DATE_ADD(created, INTERVAL 1 DAY)))", + "CREATE TABLE t (created DATE, c DATE SCRIPT AS (DATE_SUB(created, INTERVAL 1 DAY)))", + "CREATE TABLE t (created DATE, c KEYWORD SCRIPT AS (DATE_FORMAT(created, '%Y')))", + "CREATE TABLE t (created DATE, c INTEGER SCRIPT AS (DATE_DIFF(created, CURRENT_DATE, YEAR)))", + "CREATE TABLE t (ts TIMESTAMP, c INTEGER SCRIPT AS (YEAR(ts)))", + "CREATE TABLE t (ts TIMESTAMP, c TIMESTAMP SCRIPT AS (DATE_TRUNC(ts, MONTH)))" + ) + + behavior of "a date function over a date column in SCRIPT AS" + + it should "parse the raw value before applying the function" in { + // The family, not the one case that was reported. Every member reaches `ctx.` and every + // member needs the same decision, so the guard is over the list rather than a single example. + family.foreach { sql => + withClue(s"[$sql] -> ${source(sql, "c")} ") { + source(sql, "c") should include("instanceof String") + } + } + } + + it should "resolve BOTH shapes Elasticsearch accepts, not one of them" in { + // The point of `instanceof`: an ISO string AND epoch millis. Asserting only the string branch + // would pass with the old hard-coded epoch guess reinstated the other way round. + family.foreach { sql => + val s = source(sql, "c") + withClue(s"[$sql] -> $s ") { + // The string branch parses (`LocalDate.parse` for a DATE, `ZonedDateTime.parse` for a + // TIMESTAMP -- the declared type chooses, because `ZonedDateTime.parse` REFUSES a + // date with no time), and the other branch reads epoch millis. + s should include(".parse(") + s should include("Instant.ofEpochMilli") + } + } + } + + it should "yield one temporal type, so values pair and can be stored" in { + // `ChronoUnit.between` refuses mixed types and Elasticsearch refuses a `LocalDate` in `ctx`. + // A DATE parses through `LocalDate` and is then given the UTC start of day; nothing leaves a + // bare `LocalDate` behind. + family.foreach { sql => + val s = source(sql, "c") + withClue(s"[$sql] -> $s ") { + if (s.contains("LocalDate.parse")) s should include("atStartOfDay(ZoneId.of('Z'))") + s should not include ".toLocalDate()" + } + } + } + + it should "leave a non-temporal column byte-identical" in { + // The scope: a `keyword` is a `String` in a query and in `ctx` alike. + source( + "CREATE TABLE t (zip_code KEYWORD, zip_n BIGINT SCRIPT AS (CAST(zip_code AS BIGINT)))", + "zip_n" + ) should not include "instanceof" + source("CREATE TABLE t (a KEYWORD, b KEYWORD SCRIPT AS (UPPER(a)))", "b") should not include + "instanceof" + } + + behavior of "the ingest clock" + + it should "not read a key that is null on every supported Elasticsearch" in { + // 🔴 Falsifiable both ways: the old accessor must be gone AND the new one present. Asserting + // only its absence would pass with the clock deleted entirely. + val s = source( + "CREATE TABLE t (n INTEGER, c TIMESTAMP SCRIPT AS (CURRENT_TIMESTAMP))", + "c" + ) + s should not include "_ingest" + s should include("System.currentTimeMillis()") + } + + it should "keep the ZonedDateTime for CURRENT_DATE too" in { + // Narrowing to a `LocalDate` here is what made `ChronoUnit.YEARS.between` refuse the pair in + // the published DATE_DIFF example. A query is unaffected and still narrows -- asserted in + // ParserSpec, whose query-side pins did not move. + val s = source( + "CREATE TABLE t (birthdate DATE, age INTEGER SCRIPT AS (DATE_DIFF(birthdate, CURRENT_DATE, YEAR)))", + "age" + ) + s should not include ".toLocalDate()" + s should include("ChronoUnit.YEARS.between") + } + + it should "emit the published example exactly as it was executed" in { + // The whole point, pinned end to end. This byte string was run as an ingest pipeline on ES + // 6.8.23, 7.17.29, 8.18.3 and 9.0.3: `{"birthdate":"1990-05-20"}` and + // `{"birthdate":643161600000}` both store `age: 36`. + source( + "CREATE TABLE t (birthdate DATE, age INTEGER SCRIPT AS (DATE_DIFF(birthdate, CURRENT_DATE, YEAR)))", + "age" + ) shouldBe + "def param1 = ctx.birthdate; " + + "def param2 = (param1 instanceof String ? " + + """LocalDate.parse((param1).replace("/", "-"), DateTimeFormatter.ofPattern("yyyy-MM-dd"))""" + + ".atStartOfDay(ZoneId.of('Z')) : Instant.ofEpochMilli(param1).atZone(ZoneId.of('Z'))); " + + "def param3 = ZonedDateTime.ofInstant(Instant.ofEpochMilli(System.currentTimeMillis()), " + + "ZoneId.of('Z')); " + + "def param4 = Long.valueOf(ChronoUnit.YEARS.between(param2, param3)); " + + "ctx.age = (param1 == null) ? null : param4" + } +} diff --git a/testkit/src/main/scala/app/softnetwork/elastic/client/GatewayApiIntegrationSpec.scala b/testkit/src/main/scala/app/softnetwork/elastic/client/GatewayApiIntegrationSpec.scala index de7da7eb..1ef128a6 100644 --- a/testkit/src/main/scala/app/softnetwork/elastic/client/GatewayApiIntegrationSpec.scala +++ b/testkit/src/main/scala/app/softnetwork/elastic/client/GatewayApiIntegrationSpec.scala @@ -23,7 +23,8 @@ import app.softnetwork.elastic.sql.`type`.SQLTypes import app.softnetwork.elastic.sql.health.HealthStatus import app.softnetwork.elastic.sql.policy.EnrichPolicyTaskStatus -import java.time.LocalDate +import java.time.temporal.ChronoUnit +import java.time.{LocalDate, ZoneOffset, ZonedDateTime} // --------------------------------------------------------------------------- // Base test trait — to be mixed with ElasticDockerTestKit @@ -1494,19 +1495,28 @@ trait GatewayApiIntegrationSpec extends GatewayIntegrationTestKit { * - AS DATE was `(Date, Date)`, the IDENTITY arm ⇒ the un-truncated timestamp came back; * - AS TIME had no `(Date, Time)` arm at all ⇒ the fallback returned the timestamp whole. */ - /** 🔴 The INGEST path, which this suite has never exercised end to end. + /** 🔴 The INGEST path, end to end. This test EARNED its keep: it was written to record that the + * computed column did not compute, it said so loudly when that changed, and it is now the + * end-to-end proof that it does. * - * `users` is created with an ingest `DATEDIFF` column but nothing is ever inserted into it, and - * the `age` assertions elsewhere belong to `dql_users`, which has no script column. So no test - * has ever observed what an ingest script actually computes — which is why story 21.5's ingest - * guard rested on inference rather than measurement. + * What it measures is the RUNTIME TYPE of `ctx.`. `ctx.d` is the raw JSON value of + * the document being indexed, NOT the temporal object `doc['d'].value` hands a query — that part + * of the earlier reading was right, and `SQLTypeUtils.coerce` still guards its temporal arms on + * `isProcessorContext` for exactly that reason. What was wrong was the conclusion drawn from it: + * that `DATEDIFF(d, CURRENT_DATE, DAY)` therefore CANNOT compute at ingest. It can, once the + * operand is parsed first — and until story 21.8 Part C it did not, so `ignore_failure` left the + * column unset and the value was silently missing from every stored document. * - * What is being measured: the RUNTIME TYPE of `ctx.`. `SQLTypeUtils.coerce` guards - * its temporal arms on `isProcessorContext` because `ctx.d` is the raw JSON value of the - * document being indexed, NOT the temporal object `doc['d'].value` hands a query. If that is - * right, `DATEDIFF(d, CURRENT_DATE, DAY)` cannot compute at ingest — `ChronoUnit.DAYS.between` - * gets a String — and the processor's `ignore_failure` leaves the column unset. If it is wrong, - * `days` comes back a number and the guard is wrong; this test says which. + * Two more things had to be true and neither was visible from the emission alone: + * + * - `CURRENT_DATE` read `ctx['_ingest']['timestamp']`, which is NULL on ES 6.8, 7.17, 8.18 AND + * 9.0 — so the ingest clock had never worked on any supported version; + * - both sides of `ChronoUnit.between` must be the same Java type, so a processor keeps + * `ZonedDateTime` throughout instead of narrowing `CURRENT_DATE` to a `LocalDate`. + * + * ⚠️ The expected value is COMPUTED, not pinned: it is a distance from `now`, so a literal would + * have been correct for one day. It is derived the way the ingest script derives it, and the ±1 + * tolerance covers an ingest and an assertion that straddle UTC midnight. */ it should "record what an ingest script sees for a DATE column (ctx runtime type)" in { val create = @@ -1538,10 +1548,17 @@ trait GatewayApiIntegrationSpec extends GatewayIntegrationTestKit { String.valueOf(scalarOf(row, "d")) should startWith("2024-03-15") scalarOf(row, "label") shouldBe "x" - val days = row.get("days").map(v => scalarOf(row, "days")).orNull - withClue(s"ingest-computed days = [$days] (null/absent => ctx.d is NOT a temporal object): ") { - // The measurement. Asserted, not merely printed, so a change in either direction is loud. - Option(days) shouldBe None + val days = row.get("days").map(_ => scalarOf(row, "days")).orNull + val expected = ChronoUnit.DAYS.between( + LocalDate.of(2024, 3, 15).atStartOfDay(ZoneOffset.UTC), + ZonedDateTime.now(ZoneOffset.UTC) + ) + withClue(s"ingest-computed days = [$days], expected ~$expected: ") { + // The measurement. Asserted, not merely printed, so a change in either direction is loud — + // which is how this test caught story 21.8 Part C landing. + Option(days) should not be empty + val actual = String.valueOf(days).toDouble.toLong + actual.toDouble shouldBe expected.toDouble +- 1.0d } }