From b15ce1aebd110bc90ab4ab778090fe6b6ab3554d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Manciot?= Date: Sun, 6 Sep 2026 03:27:59 +0200 Subject: [PATCH 1/5] fix(sql,core): resolve temporal literals against the mapped date format before rendering the query A WHERE comparison against a `date`-mapped column forwarded the string literal verbatim into the range/term query, so the SQL-standard spelling BI tools render ('2026-06-04 00:00:00.000000') was rejected by Elasticsearch's default `strict_date_optional_time||epoch_millis` format while only the T-separated ISO form matched: Superset's relative-date filter could not run, on JDBC and Flight SQL alike. The literals a WHERE clause compares against a date column (= <> != >= > <= <, BETWEEN, IN) are now resolved ONCE per statement, on the AST, against that column's mapping format (`query.TemporalLiterals`): the space form is validated with java.time and rewritten with a T (fraction digits and zone preserved); a literal a custom `format` already parses is left alone so a `yyyy-MM-dd HH:mm:ss` column keeps working; epoch numbers and date math are untouched; a literal that cannot be a date under a fully-understood format is rejected with a 400 naming the literal and the field instead of a raw search_phase_execution_exception. keyword/text columns, LIKE/RLIKE patterns, function-wrapped columns, TIME columns, date_nanos and HAVING are never touched, and a keyword column compared to a date-shaped string emits byte-identical JSON. No executing SELECT path attached a schema before (only CTAS's Table.mergeWithSearch did), so the pass is invoked from core at every SingleSearch -> ElasticQuery seam - SearchApi.search / searchAsync (single and each UNION ALL request), the inner-hits search, ScrollApi.scroll - through the IndicesApi schema cache, with a per-client negative cache for sources whose schema cannot be loaded. GatewayApi.run (REPL, JDBC, Flight SQL sidecar and its JOIN legs) is covered transitively. Schema-absent paths (no candidate literal, several sources, wildcard, JOIN-alias identifiers, unloadable schema) forward the literal verbatim as before. Tests: TemporalLiteralsSpec (sql, 25), TemporalLiteralQuerySpec (bridge template + es6 copy, 6), TemporalLiteralSearchSpec (core, 10, Docker-free), TemporalLiteralSpec testkit template + five client subclasses green on ES 6.8 rest/jest, 7.17, 8.18, 9.0; falsified against the baseline core seams (7/10 core, 4/7 on real ES 8.18 red with the original symptom). Story BIDC-4 Closes #276 Co-Authored-By: Claude Fable 5.1 --- .../sql/TemporalLiteralQuerySpec.scala | 103 +++++ .../elastic/client/ScrollApi.scala | 8 +- .../elastic/client/SearchApi.scala | 144 ++++++- .../client/TemporalLiteralSearchSpec.scala | 244 +++++++++++ documentation/sql/dql_statements.md | 28 ++ .../sql/TemporalLiteralQuerySpec.scala | 103 +++++ .../JestClientTemporalLiteralSpec.scala | 19 + ...stHighLevelClientTemporalLiteralSpec.scala | 19 + ...stHighLevelClientTemporalLiteralSpec.scala | 19 + .../JavaClientTemporalLiteralSpec.scala | 19 + .../JavaClientTemporalLiteralSpec.scala | 19 + .../elastic/sql/query/TemporalLiterals.scala | 398 ++++++++++++++++++ .../sql/query/TemporalLiteralsSpec.scala | 368 ++++++++++++++++ .../elastic/client/TemporalLiteralSpec.scala | 238 +++++++++++ 14 files changed, 1722 insertions(+), 7 deletions(-) create mode 100644 bridge/src/test/scala/app/softnetwork/elastic/sql/TemporalLiteralQuerySpec.scala create mode 100644 core/src/test/scala/app/softnetwork/elastic/client/TemporalLiteralSearchSpec.scala create mode 100644 es6/bridge/src/test/scala/app/softnetwork/elastic/sql/TemporalLiteralQuerySpec.scala create mode 100644 es6/jest/src/test/scala/app/softnetwork/elastic/client/JestClientTemporalLiteralSpec.scala create mode 100644 es6/rest/src/test/scala/app/softnetwork/elastic/client/RestHighLevelClientTemporalLiteralSpec.scala create mode 100644 es7/rest/src/test/scala/app/softnetwork/elastic/client/RestHighLevelClientTemporalLiteralSpec.scala create mode 100644 es8/java/src/test/scala/app/softnetwork/elastic/client/JavaClientTemporalLiteralSpec.scala create mode 100644 es9/java/src/test/scala/app/softnetwork/elastic/client/JavaClientTemporalLiteralSpec.scala create mode 100644 sql/src/main/scala/app/softnetwork/elastic/sql/query/TemporalLiterals.scala create mode 100644 sql/src/test/scala/app/softnetwork/elastic/sql/query/TemporalLiteralsSpec.scala create mode 100644 testkit/src/main/scala/app/softnetwork/elastic/client/TemporalLiteralSpec.scala diff --git a/bridge/src/test/scala/app/softnetwork/elastic/sql/TemporalLiteralQuerySpec.scala b/bridge/src/test/scala/app/softnetwork/elastic/sql/TemporalLiteralQuerySpec.scala new file mode 100644 index 000000000..0bd4d5bf5 --- /dev/null +++ b/bridge/src/test/scala/app/softnetwork/elastic/sql/TemporalLiteralQuerySpec.scala @@ -0,0 +1,103 @@ +package app.softnetwork.elastic.sql + +import app.softnetwork.elastic.sql.bridge._ +import app.softnetwork.elastic.sql.parser.Parser +import app.softnetwork.elastic.sql.query.{SingleSearch, TemporalLiterals} +import app.softnetwork.elastic.sql.schema.{Column, Table => SchemaTable} +import app.softnetwork.elastic.sql.`type`.SQLTypes +import com.sksamuel.elastic4s.requests.searches.SearchBodyBuilderFn +import org.scalatest.flatspec.AnyFlatSpec +import org.scalatest.matchers.should.Matchers + +import java.time.ZonedDateTime +import scala.collection.immutable.ListMap + +/** Issue #276 -- the GENERATED Elasticsearch query once temporal literals have been resolved + * against the mapped `date` columns. A parse-level assertion cannot see what reaches `rangeQuery` + * / `termQuery`; this spec does. The keyword negative control (AC 3) is a byte-identical + * comparison of the emitted JSON with and without the schema. + */ +class TemporalLiteralQuerySpec extends AnyFlatSpec with Matchers { + + implicit def timestamp: Long = ZonedDateTime.parse("2025-12-31T00:00:00Z").toInstant.toEpochMilli + + private val schema: SchemaTable = SchemaTable( + "events", + columns = List( + Column("id", SQLTypes.Keyword), + Column("event_ts", SQLTypes.Date), + Column("label", SQLTypes.Keyword), + Column( + "fmt_ts", + SQLTypes.Date, + options = ListMap("format" -> StringValue("yyyy-MM-dd HH:mm:ss")) + ) + ) + ) + + private def single(sql: String): SingleSearch = Parser(sql) match { + case Right(s: SingleSearch) => s + case other => fail(s"$sql did not parse as a SingleSearch: $other") + } + + private def json(search: SingleSearch): String = + SearchBodyBuilderFn(requestToElasticSearchRequest(search).search).string + + private def resolved(sql: String): SingleSearch = + TemporalLiterals(single(sql), schema) match { + case Right(s) => s + case Left(reason) => fail(s"$sql was rejected: $reason") + } + + "a range comparison against a date column" should "emit the T-separated literal" in { + json(resolved("SELECT id FROM events WHERE event_ts >= '2026-06-04 00:00:00.000000'")) shouldBe + """{"query":{"bool":{"filter":[{"range":{"event_ts":{"gte":"2026-06-04T00:00:00.000000"}}}]}},"_source":{"includes":["id"]}}""" + } + + "equality, BETWEEN and IN against a date column" should "emit normalised term, range and terms values" in { + json(resolved("SELECT id FROM events WHERE event_ts = '2026-06-04 00:00:00'")) shouldBe + """{"query":{"bool":{"filter":[{"term":{"event_ts":{"value":"2026-06-04T00:00:00"}}}]}},"_source":{"includes":["id"]}}""" + + json( + resolved( + "SELECT id FROM events WHERE event_ts BETWEEN '2026-06-01 00:00:00' AND '2026-06-30 23:59:59'" + ) + ) shouldBe + """{"query":{"bool":{"filter":[{"range":{"event_ts":{"gte":"2026-06-01T00:00:00","lte":"2026-06-30T23:59:59"}}}]}},"_source":{"includes":["id"]}}""" + + json( + resolved( + "SELECT id FROM events WHERE event_ts IN ('2026-06-04 00:00:00', '2026-07-01 00:00:00')" + ) + ) shouldBe + """{"query":{"bool":{"filter":[{"terms":{"event_ts":["2026-06-04T00:00:00","2026-07-01T00:00:00"]}}]}},"_source":{"includes":["id"]}}""" + } + + "a keyword column compared to a date-shaped string" should "emit byte-identical JSON with and without the schema" in { + val parsed = single("SELECT id FROM events WHERE label = '2026-06-04 00:00:00'") + val withSchema = TemporalLiterals(parsed, schema) match { + case Right(s) => s + case Left(reason) => fail(reason) + } + withSchema should be theSameInstanceAs parsed + json(withSchema) shouldBe json(parsed) + json(parsed) shouldBe + """{"query":{"bool":{"filter":[{"term":{"label":{"value":"2026-06-04 00:00:00"}}}]}},"_source":{"includes":["id"]}}""" + } + + "an epoch-millis literal against a date column" should "be forwarded untouched" in { + json(resolved("SELECT id FROM events WHERE event_ts >= '1780531200000'")) shouldBe + """{"query":{"bool":{"filter":[{"range":{"event_ts":{"gte":"1780531200000"}}}]}},"_source":{"includes":["id"]}}""" + } + + "a custom-format date column" should "keep the space form its format already parses" in { + json(resolved("SELECT id FROM events WHERE fmt_ts >= '2026-06-04 00:00:00'")) shouldBe + """{"query":{"bool":{"filter":[{"range":{"fmt_ts":{"gte":"2026-06-04 00:00:00"}}}]}},"_source":{"includes":["id"]}}""" + } + + "an already ISO literal" should "emit the same JSON as before" in { + val parsed = + single("SELECT id FROM events WHERE event_ts < '2026-06-04T00:00:00' AND label = 'x'") + json(resolved(parsed.sql)) shouldBe json(parsed) + } +} diff --git a/core/src/main/scala/app/softnetwork/elastic/client/ScrollApi.scala b/core/src/main/scala/app/softnetwork/elastic/client/ScrollApi.scala index 53c719296..8aa982215 100644 --- a/core/src/main/scala/app/softnetwork/elastic/client/ScrollApi.scala +++ b/core/src/main/scala/app/softnetwork/elastic/client/ScrollApi.scala @@ -274,7 +274,13 @@ trait ScrollApi extends ElasticClientHelpers { } // Single search - case single: SingleSearch => + case parsed: SingleSearch => + // #276 -- resolve temporal literals against the mapped `date` columns before ANY branch + // renders the query (the window-enrichment branch derives its queries from `single`). + val single = resolveTemporalLiterals(parsed) match { + case ElasticSuccess(resolved) => resolved + case ElasticFailure(error) => return Source.failed(error) + } // #238 — an explicit LIMIT keeps the sequential PIT path on EVERY branch, including the // window-enrichment branch below (createBaseQuery keeps the LIMIT — AC 6). val config0 = if (single.limit.isDefined) config.copy(maxSlices = Some(1)) else config diff --git a/core/src/main/scala/app/softnetwork/elastic/client/SearchApi.scala b/core/src/main/scala/app/softnetwork/elastic/client/SearchApi.scala index e0f0a96be..d8f0bafac 100644 --- a/core/src/main/scala/app/softnetwork/elastic/client/SearchApi.scala +++ b/core/src/main/scala/app/softnetwork/elastic/client/SearchApi.scala @@ -36,6 +36,7 @@ import app.softnetwork.elastic.sql.query.{ SelectStatement, SingleSearch } +import app.softnetwork.elastic.sql.query.TemporalLiterals import com.fasterxml.jackson.databind.JsonNode import com.typesafe.config.ConfigFactory import org.json4s.Formats @@ -73,6 +74,111 @@ trait SearchApi extends ElasticConversion with ElasticClientHelpers { else fields.map(f => f.fieldAlias.map(_.alias).getOrElse(f.sourceField)) } + /** Issue #276 -- resolve the string literals a WHERE clause compares against `date`-mapped + * columns (see [[TemporalLiterals]]) BEFORE the statement is rendered into an Elasticsearch + * query. Runs ONCE per statement, on the AST -- never per hit. + * + * Covered entry points: every `SingleSearch -> ElasticQuery` seam -- [[search]] / + * [[searchAsync]] (the single statement and each `UNION ALL` request), the inner-hits search and + * `ScrollApi.scroll` -- hence `GatewayApi.run` (REPL, JDBC driver, Flight SQL sidecar and its + * cross-index JOIN legs, which re-enter `gateway.run`) transitively. + * + * Schema-absent path = verbatim (#276 AD-S4-1.2): the lookup is skipped when the WHERE carries + * no candidate literal, when the FROM does not name exactly ONE concrete index (several sources, + * a `*` wildcard or a `,` list), when this client is not an [[IndicesApi]], and when the schema + * cannot be loaded (alias without a template, unknown index, cluster error or a thrown lookup) + * -- the literal is then forwarded exactly as before. The schema comes from + * [[IndicesApi.loadSchema]] 's 5-minute cache, so a miss costs one `GET ` per index per + * TTL. A literal that cannot be a date under a fully-understood mapping format is a `400` that + * names the literal and the field, instead of Elasticsearch's + * `search_phase_execution_exception`. + * + * `loadSchema` caches successes only, so a source it cannot resolve -- an ALIAS on the es8/es9 + * clients (`executeGetIndex` looks the alias up as a key and finds nothing), an unknown index + * without a template -- would otherwise cost one failed lookup (two round trips + WARN lines) + * per statement. [[temporalLiteralSchemaMisses]] remembers such a source for + * [[temporalLiteralSchemaMissTtlMs]] and skips the lookup; the literal is verbatim either way. + */ + protected def resolveTemporalLiterals(single: SingleSearch): ElasticResult[SingleSearch] = { + if (!TemporalLiterals.hasCandidates(single)) return ElasticResult.success(single) + single.sources.distinct match { + case Seq(source) if !source.contains("*") && !source.contains(",") => + this match { + case indices: IndicesApi if !temporalLiteralSchemaMissed(source) => + Try(indices.loadSchema(source)) match { + case Success(ElasticSuccess(schema)) => + temporalLiteralSchemaMisses.remove(source) + TemporalLiterals(single, schema) match { + case Right(resolved) => + if (resolved ne single) + logger.debug( + s"Temporal literals resolved against the mapping of '$source':${resolved.where + .map(_.sql) + .getOrElse("")}" + ) + ElasticResult.success(resolved) + case Left(reason) => + logger.error(s"❌ $reason") + ElasticResult.failure( + ElasticError( + message = reason, + statusCode = Some(400), + index = Some(source), + operation = Some("search") + ) + ) + } + case Success(ElasticFailure(error)) => + temporalLiteralSchemaMisses.put(source, System.currentTimeMillis()) + logger.debug( + s"Schema of '$source' unavailable (${error.message}) - temporal literals forwarded verbatim" + ) + ElasticResult.success(single) + case Failure(e) => + temporalLiteralSchemaMisses.put(source, System.currentTimeMillis()) + logger.debug( + s"Schema lookup for '$source' failed with ${e.getClass.getName} - temporal literals forwarded verbatim" + ) + ElasticResult.success(single) + } + case _ => ElasticResult.success(single) + } + case _ => ElasticResult.success(single) + } + } + + /** Sources whose schema could not be loaded, with the time of the miss (see + * [[resolveTemporalLiterals]]). Per client instance, like the schema cache it shadows. + */ + private val temporalLiteralSchemaMisses = + new java.util.concurrent.ConcurrentHashMap[String, java.lang.Long]() + + /** How long a failed schema lookup is remembered -- the schema cache's own default TTL. */ + protected def temporalLiteralSchemaMissTtlMs: Long = 5 * 60 * 1000L + + private def temporalLiteralSchemaMissed(source: String): Boolean = + Option(temporalLiteralSchemaMisses.get(source)).exists { missedAt => + System.currentTimeMillis() - missedAt < temporalLiteralSchemaMissTtlMs + } + + /** [[resolveTemporalLiterals]] over every request of a `UNION ALL`; the first rejection wins. */ + protected def resolveTemporalLiterals(multiple: MultiSearch): ElasticResult[MultiSearch] = { + val zero: ElasticResult[Seq[SingleSearch]] = ElasticResult.success(Seq.empty) + multiple.requests.foldLeft(zero) { + case (ElasticSuccess(acc), request) => + resolveTemporalLiterals(request) match { + case ElasticSuccess(resolved) => ElasticResult.success(acc :+ resolved) + case ElasticFailure(error) => ElasticResult.failure(error) + } + case (failure, _) => failure + } match { + case ElasticSuccess(resolved) => + val unchanged = resolved.zip(multiple.requests).forall { case (a, b) => a eq b } + ElasticResult.success(if (unchanged) multiple else multiple.copy(requests = resolved)) + case ElasticFailure(error) => ElasticResult.failure(error) + } + } + // ======================================================================== // PUBLIC METHODS // ======================================================================== @@ -107,7 +213,12 @@ trait SearchApi extends ElasticConversion with ElasticClientHelpers { ) ) } - case single: SingleSearch => + case parsed: SingleSearch => + // #276 -- resolve temporal literals against the mapped `date` columns BEFORE rendering + val single = resolveTemporalLiterals(parsed) match { + case ElasticSuccess(resolved) => resolved + case ElasticFailure(error) => return ElasticResult.failure(error) + } val elasticQuery = ElasticQuery( single, collection.immutable.Seq(single.sources: _*), @@ -135,7 +246,11 @@ trait SearchApi extends ElasticConversion with ElasticClientHelpers { ) } - case multiple: MultiSearch => + case parsed: MultiSearch => + val multiple = resolveTemporalLiterals(parsed) match { + case ElasticSuccess(resolved) => resolved + case ElasticFailure(error) => return ElasticResult.failure(error) + } val elasticQueries = ElasticQueries( multiple.requests.map { query => ElasticQuery( @@ -452,7 +567,12 @@ trait SearchApi extends ElasticConversion with ElasticClientHelpers { ) } - case single: SingleSearch => + case parsed: SingleSearch => + // #276 -- resolve temporal literals against the mapped `date` columns BEFORE rendering + val single = resolveTemporalLiterals(parsed) match { + case ElasticSuccess(resolved) => resolved + case ElasticFailure(error) => return Future.successful(ElasticResult.failure(error)) + } val elasticQuery = ElasticQuery( single, collection.immutable.Seq(single.sources: _*) @@ -473,7 +593,11 @@ trait SearchApi extends ElasticConversion with ElasticClientHelpers { ) } - case multiple: MultiSearch => + case parsed: MultiSearch => + val multiple = resolveTemporalLiterals(parsed) match { + case ElasticSuccess(resolved) => resolved + case ElasticFailure(error) => return Future.successful(ElasticResult.failure(error)) + } val elasticQueries = ElasticQueries( multiple.requests.map { query => ElasticQuery( @@ -989,14 +1113,22 @@ trait SearchApi extends ElasticConversion with ElasticClientHelpers { ): ElasticResult[Seq[(U, Seq[I])]] = { implicit def timestamp: Long = System.currentTimeMillis() sql.statement match { - case Some(single: SingleSearch) => + case Some(parsed: SingleSearch) => + val single = resolveTemporalLiterals(parsed) match { + case ElasticSuccess(resolved) => resolved + case ElasticFailure(error) => return ElasticResult.failure(error) + } val elasticQuery = ElasticQuery( single, collection.immutable.Seq(single.sources: _*) ) singleSearchWithInnerHits[U, I](elasticQuery, innerField) - case Some(multiple: MultiSearch) => + case Some(parsed: MultiSearch) => + val multiple = resolveTemporalLiterals(parsed) match { + case ElasticSuccess(resolved) => resolved + case ElasticFailure(error) => return ElasticResult.failure(error) + } val elasticQueries = ElasticQueries( multiple.requests.map { query => ElasticQuery( diff --git a/core/src/test/scala/app/softnetwork/elastic/client/TemporalLiteralSearchSpec.scala b/core/src/test/scala/app/softnetwork/elastic/client/TemporalLiteralSearchSpec.scala new file mode 100644 index 000000000..c692f6eb2 --- /dev/null +++ b/core/src/test/scala/app/softnetwork/elastic/client/TemporalLiteralSearchSpec.scala @@ -0,0 +1,244 @@ +/* + * Copyright 2025 SOFTNETWORK + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package app.softnetwork.elastic.client + +import akka.actor.ActorSystem +import akka.stream.scaladsl.Sink +import app.softnetwork.elastic.client.result._ +import app.softnetwork.elastic.sql.PainlessContextType +import app.softnetwork.elastic.sql.query.{SelectStatement, SingleSearch} +import app.softnetwork.elastic.sql.schema.{Column, Schema, Table} +import app.softnetwork.elastic.sql.`type`.SQLTypes +import com.fasterxml.jackson.databind.{JsonNode, ObjectMapper} +import org.scalatest.BeforeAndAfterAll +import org.scalatest.flatspec.AnyFlatSpec +import org.scalatest.matchers.should.Matchers +import org.slf4j.{Logger, LoggerFactory} + +import scala.concurrent.duration._ +import scala.concurrent.{Await, ExecutionContext, Future} +import scala.language.implicitConversions + +/** Issue #276 -- the core seam: `SearchApi.search` / `searchAsync` / `ScrollApi.scroll` resolve the + * temporal literals of a statement against the mapped `date` columns BEFORE rendering the + * Elasticsearch query. Network-free: a `NopeClientApi` records the `ElasticQuery` it would have + * sent and counts schema lookups; the schema cache is seeded through the public `updateSchema`. + * + * Covered entry points (AC 5): `search` (single + UNION ALL), `searchAsync`, `scroll`. The + * schema-absent path (no schema, several sources, wildcard source) is asserted verbatim, and the + * lookup is asserted SKIPPED when the WHERE carries no candidate literal. + */ +class TemporalLiteralSearchSpec extends AnyFlatSpec with Matchers with BeforeAndAfterAll { + + private val testLogger: Logger = LoggerFactory.getLogger(getClass) + + implicit val system: ActorSystem = ActorSystem("temporal-literal-search-spec") + implicit val ec: ExecutionContext = system.dispatcher + implicit val context: ConversionContext = NativeContext + + override def afterAll(): Unit = { + Await.result(system.terminate(), 10.seconds) + super.afterAll() + } + + private val schema: Schema = Table( + "events", + columns = List( + Column("id", SQLTypes.Keyword), + Column("event_ts", SQLTypes.Date), + Column("label", SQLTypes.Keyword), + Column("amount", SQLTypes.Int) + ) + ) + + /** Records the query the client would send, and how many schema lookups it made. + * + * `NopeClientApi` renders every statement as `match_all`; this client renders the SQL of the + * statement that REACHED the conversion instead (as a JSON document, so `validateJson` accepts + * it) -- which is exactly what the assertions below need to see. Statements carry a `LIMIT` so + * the one-shot path is taken (#209 routes an un-LIMITed row query through scroll). + */ + private class RecordingClient extends NopeClientApi { + override protected def logger: Logger = testLogger + + /** Set to 0 by a test to expire the negative cache immediately. */ + @volatile var missTtlMs: Long = 5 * 60 * 1000L + override protected def temporalLiteralSchemaMissTtlMs: Long = missTtlMs + + override private[client] implicit def singleSearchToJsonQuery( + sqlSearch: SingleSearch + )(implicit + timestamp: Long, + contextType: PainlessContextType = PainlessContextType.Query + ): String = new ObjectMapper().createObjectNode().put("sql", sqlSearch.sql).toString + + @volatile var lastQuery: Option[ElasticQuery] = None + @volatile var lastMultiQuery: Option[ElasticQueries] = None + @volatile var schemaLookups: Int = 0 + + override def loadSchema(index: String): ElasticResult[Schema] = { + schemaLookups += 1 + super.loadSchema(index) + } + + override private[client] def executeSingleSearch( + elasticQuery: ElasticQuery + ): ElasticResult[Option[JsonNode]] = { + lastQuery = Some(elasticQuery) + ElasticResult.success(None) + } + + override private[client] def executeSingleSearchAsync(elasticQuery: ElasticQuery)(implicit + ec: ExecutionContext + ): Future[ElasticResult[Option[JsonNode]]] = Future { + lastQuery = Some(elasticQuery) + ElasticResult.success(None) + } + + override private[client] def executeMultiSearch( + elasticQueries: ElasticQueries + ): ElasticResult[Option[JsonNode]] = { + lastMultiQuery = Some(elasticQueries) + ElasticResult.success(None) + } + } + + private def seeded(): RecordingClient = { + val client = new RecordingClient + client.updateSchema("events", schema) + client + } + + private val spaceForm = "2026-06-04 00:00:00.000000" + private val isoForm = "2026-06-04T00:00:00.000000" + + "search" should "render the T-separated literal for a date column when the schema is known" in { + val client = seeded() + client.search(SelectStatement(s"SELECT id FROM events WHERE event_ts >= '$spaceForm' LIMIT 5")) + val query = client.lastQuery.getOrElse(fail("no query was rendered")).query + query should include(isoForm) + query should not include spaceForm + client.schemaLookups shouldBe 1 + } + + it should "leave a keyword column compared to a date-shaped string untouched" in { + val client = seeded() + client.search(SelectStatement(s"SELECT id FROM events WHERE label = '$spaceForm' LIMIT 5")) + client.lastQuery.getOrElse(fail("no query was rendered")).query should include(spaceForm) + } + + it should "forward the literal verbatim when the schema cannot be loaded, and remember the miss" in { + val client = new RecordingClient // no seed: getIndex and getTemplate both answer None + client.search(SelectStatement(s"SELECT id FROM events WHERE event_ts >= '$spaceForm' LIMIT 5")) + client.lastQuery.getOrElse(fail("no query was rendered")).query should include(spaceForm) + client.schemaLookups shouldBe 1 + // the failed lookup is remembered: a second statement on the same source costs no round trip + client.search(SelectStatement(s"SELECT id FROM events WHERE event_ts < '$spaceForm' LIMIT 5")) + client.lastQuery.getOrElse(fail("no query was rendered")).query should include(spaceForm) + client.schemaLookups shouldBe 1 + // ... until the miss expires and the schema is available + client.updateSchema("events", schema) + client.missTtlMs = 0L + client.search(SelectStatement(s"SELECT id FROM events WHERE event_ts < '$spaceForm' LIMIT 5")) + client.lastQuery.getOrElse(fail("no query was rendered")).query should include(isoForm) + } + + it should "not look the schema up at all when the WHERE carries no candidate literal" in { + val client = seeded() + client.search(SelectStatement("SELECT id FROM events WHERE amount > 10 LIMIT 5")) + client.lastQuery shouldBe defined + client.schemaLookups shouldBe 0 + client.search(SelectStatement("SELECT id FROM events LIMIT 5")) + client.schemaLookups shouldBe 0 + } + + it should "forward the literal verbatim over several sources or a wildcard source" in { + val client = seeded() + client.search( + SelectStatement(s"SELECT id FROM events, archive WHERE event_ts >= '$spaceForm' LIMIT 5") + ) + client.lastQuery.getOrElse(fail("no query was rendered")).query should include(spaceForm) + client.search(SelectStatement(s"SELECT id FROM events* WHERE event_ts >= '$spaceForm' LIMIT 5")) + client.lastQuery.getOrElse(fail("no query was rendered")).query should include(spaceForm) + client.schemaLookups shouldBe 0 + } + + it should "reject an unparseable literal against a date column with a 400 naming the literal and the field" in { + val client = seeded() + client.search( + SelectStatement("SELECT id FROM events WHERE event_ts >= 'not-a-date' LIMIT 5") + ) match { + case ElasticFailure(error) => + error.statusCode shouldBe Some(400) + error.operation shouldBe Some("search") + error.index shouldBe Some("events") + error.message should include("'not-a-date'") + error.message should include("'event_ts'") + error.message should not include "search_phase_execution_exception" + case ElasticSuccess(other) => fail(s"expected a rejection, got $other") + } + client.lastQuery shouldBe None // never reached the client + } + + it should "resolve every request of a UNION ALL" in { + val client = seeded() + client.search( + SelectStatement( + s"SELECT id FROM events WHERE event_ts >= '$spaceForm' UNION ALL " + + "SELECT id FROM events WHERE event_ts < '2026-06-01 00:00:00'" + ) + ) + val multi = client.lastMultiQuery.getOrElse(fail("no multi-search was rendered")).multiQuery + multi should include(isoForm) + multi should include("2026-06-01T00:00:00") + multi should not include spaceForm + } + + "searchAsync" should "render the T-separated literal as well" in { + val client = seeded() + Await.result( + client.searchAsync( + SelectStatement(s"SELECT id FROM events WHERE event_ts >= '$spaceForm' LIMIT 5") + ), + 10.seconds + ) + client.lastQuery.getOrElse(fail("no query was rendered")).query should include(isoForm) + } + + it should "reject an unparseable literal with a 400 without touching the client" in { + val client = seeded() + Await.result( + client.searchAsync(SelectStatement("SELECT id FROM events WHERE event_ts = 'nope' LIMIT 5")), + 10.seconds + ) match { + case ElasticFailure(error) => + error.statusCode shouldBe Some(400) + error.message should include("'nope'") + case ElasticSuccess(other) => fail(s"expected a rejection, got $other") + } + client.lastQuery shouldBe None + } + + "scroll" should "fail the stream with the same 400 for an unparseable literal" in { + val client = seeded() + val stream = client.scroll(SelectStatement("SELECT id FROM events WHERE event_ts >= 'nope'")) + val error = the[ElasticError] thrownBy Await.result(stream.runWith(Sink.seq), 10.seconds) + error.statusCode shouldBe Some(400) + error.message should include("'nope'") + error.message should include("'event_ts'") + } +} diff --git a/documentation/sql/dql_statements.md b/documentation/sql/dql_statements.md index 754bdf14f..b4f9a178e 100644 --- a/documentation/sql/dql_statements.md +++ b/documentation/sql/dql_statements.md @@ -286,6 +286,34 @@ WHERE age BETWEEN 20 AND 50 AND (name LIKE 'A%' OR name RLIKE '.*o.*'); ``` +### Temporal literals against `date` columns + +A string literal compared to a column mapped as `date` (with `=`, `<>`, `!=`, `<`, `<=`, `>`, `>=`, +`BETWEEN` or `IN`) is resolved against the column's mapping **format** before the query is sent to +Elasticsearch, so the SQL-standard spelling a BI tool emits selects the same rows as the ISO one: + +```sql +WHERE event_ts >= '2026-06-04 00:00:00.000000' -- what Superset / SQLAlchemy render +WHERE event_ts >= '2026-06-04 00:00:00' +WHERE event_ts >= '2026-06-04T00:00:00' -- what Elasticsearch's default format accepts +``` + +- Under the default format (`strict_date_optional_time||epoch_millis`) the space separator is + rewritten to `T`; fraction digits and a trailing zone offset are preserved. ISO literals, + date-only literals, epoch milliseconds and date math (`now-1d/d`) are forwarded verbatim. +- A column with a custom `format` (for example `yyyy-MM-dd HH:mm:ss`) keeps working as before: a + literal its format already parses is never rewritten. +- A literal that cannot be a date under the default format fails with an error naming the literal + and the field (HTTP 400) instead of a raw Elasticsearch `search_phase_execution_exception`. +- `keyword`/`text` columns, `LIKE`/`RLIKE` patterns, function-wrapped columns (`YEAR(event_ts)`), + `date_nanos` columns and `HAVING` conditions are never touched. +- The resolution needs the index mapping, loaded through the schema cache (one lookup per index + every 5 minutes, and only for statements whose `WHERE` compares a string literal to a column). + When the statement reads several indices or a wildcard, joins another index (`JOIN` sources keep + their literals), or the mapping cannot be loaded (an index alias, for instance), the literal is + forwarded verbatim as in previous releases; a failed mapping lookup is remembered for 5 minutes + so it is not retried on every statement. + --- ## ORDER BY diff --git a/es6/bridge/src/test/scala/app/softnetwork/elastic/sql/TemporalLiteralQuerySpec.scala b/es6/bridge/src/test/scala/app/softnetwork/elastic/sql/TemporalLiteralQuerySpec.scala new file mode 100644 index 000000000..73477a239 --- /dev/null +++ b/es6/bridge/src/test/scala/app/softnetwork/elastic/sql/TemporalLiteralQuerySpec.scala @@ -0,0 +1,103 @@ +package app.softnetwork.elastic.sql + +import app.softnetwork.elastic.sql.bridge._ +import app.softnetwork.elastic.sql.parser.Parser +import app.softnetwork.elastic.sql.query.{SingleSearch, TemporalLiterals} +import app.softnetwork.elastic.sql.schema.{Column, Table => SchemaTable} +import app.softnetwork.elastic.sql.`type`.SQLTypes +import com.sksamuel.elastic4s.http.search.SearchBodyBuilderFn +import org.scalatest.flatspec.AnyFlatSpec +import org.scalatest.matchers.should.Matchers + +import java.time.ZonedDateTime +import scala.collection.immutable.ListMap + +/** Issue #276 -- the GENERATED Elasticsearch query once temporal literals have been resolved + * against the mapped `date` columns. A parse-level assertion cannot see what reaches `rangeQuery` + * / `termQuery`; this spec does. The keyword negative control (AC 3) is a byte-identical + * comparison of the emitted JSON with and without the schema. + */ +class TemporalLiteralQuerySpec extends AnyFlatSpec with Matchers { + + implicit def timestamp: Long = ZonedDateTime.parse("2025-12-31T00:00:00Z").toInstant.toEpochMilli + + private val schema: SchemaTable = SchemaTable( + "events", + columns = List( + Column("id", SQLTypes.Keyword), + Column("event_ts", SQLTypes.Date), + Column("label", SQLTypes.Keyword), + Column( + "fmt_ts", + SQLTypes.Date, + options = ListMap("format" -> StringValue("yyyy-MM-dd HH:mm:ss")) + ) + ) + ) + + private def single(sql: String): SingleSearch = Parser(sql) match { + case Right(s: SingleSearch) => s + case other => fail(s"$sql did not parse as a SingleSearch: $other") + } + + private def json(search: SingleSearch): String = + SearchBodyBuilderFn(requestToElasticSearchRequest(search).search).string() + + private def resolved(sql: String): SingleSearch = + TemporalLiterals(single(sql), schema) match { + case Right(s) => s + case Left(reason) => fail(s"$sql was rejected: $reason") + } + + "a range comparison against a date column" should "emit the T-separated literal" in { + json(resolved("SELECT id FROM events WHERE event_ts >= '2026-06-04 00:00:00.000000'")) shouldBe + """{"query":{"bool":{"filter":[{"range":{"event_ts":{"gte":"2026-06-04T00:00:00.000000"}}}]}},"_source":{"includes":["id"]}}""" + } + + "equality, BETWEEN and IN against a date column" should "emit normalised term, range and terms values" in { + json(resolved("SELECT id FROM events WHERE event_ts = '2026-06-04 00:00:00'")) shouldBe + """{"query":{"bool":{"filter":[{"term":{"event_ts":{"value":"2026-06-04T00:00:00"}}}]}},"_source":{"includes":["id"]}}""" + + json( + resolved( + "SELECT id FROM events WHERE event_ts BETWEEN '2026-06-01 00:00:00' AND '2026-06-30 23:59:59'" + ) + ) shouldBe + """{"query":{"bool":{"filter":[{"range":{"event_ts":{"gte":"2026-06-01T00:00:00","lte":"2026-06-30T23:59:59"}}}]}},"_source":{"includes":["id"]}}""" + + json( + resolved( + "SELECT id FROM events WHERE event_ts IN ('2026-06-04 00:00:00', '2026-07-01 00:00:00')" + ) + ) shouldBe + """{"query":{"bool":{"filter":[{"terms":{"event_ts":["2026-06-04T00:00:00","2026-07-01T00:00:00"]}}]}},"_source":{"includes":["id"]}}""" + } + + "a keyword column compared to a date-shaped string" should "emit byte-identical JSON with and without the schema" in { + val parsed = single("SELECT id FROM events WHERE label = '2026-06-04 00:00:00'") + val withSchema = TemporalLiterals(parsed, schema) match { + case Right(s) => s + case Left(reason) => fail(reason) + } + withSchema should be theSameInstanceAs parsed + json(withSchema) shouldBe json(parsed) + json(parsed) shouldBe + """{"query":{"bool":{"filter":[{"term":{"label":{"value":"2026-06-04 00:00:00"}}}]}},"_source":{"includes":["id"]}}""" + } + + "an epoch-millis literal against a date column" should "be forwarded untouched" in { + json(resolved("SELECT id FROM events WHERE event_ts >= '1780531200000'")) shouldBe + """{"query":{"bool":{"filter":[{"range":{"event_ts":{"gte":"1780531200000"}}}]}},"_source":{"includes":["id"]}}""" + } + + "a custom-format date column" should "keep the space form its format already parses" in { + json(resolved("SELECT id FROM events WHERE fmt_ts >= '2026-06-04 00:00:00'")) shouldBe + """{"query":{"bool":{"filter":[{"range":{"fmt_ts":{"gte":"2026-06-04 00:00:00"}}}]}},"_source":{"includes":["id"]}}""" + } + + "an already ISO literal" should "emit the same JSON as before" in { + val parsed = + single("SELECT id FROM events WHERE event_ts < '2026-06-04T00:00:00' AND label = 'x'") + json(resolved(parsed.sql)) shouldBe json(parsed) + } +} diff --git a/es6/jest/src/test/scala/app/softnetwork/elastic/client/JestClientTemporalLiteralSpec.scala b/es6/jest/src/test/scala/app/softnetwork/elastic/client/JestClientTemporalLiteralSpec.scala new file mode 100644 index 000000000..9fee421c4 --- /dev/null +++ b/es6/jest/src/test/scala/app/softnetwork/elastic/client/JestClientTemporalLiteralSpec.scala @@ -0,0 +1,19 @@ +/* + * Copyright 2025 SOFTNETWORK + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package app.softnetwork.elastic.client + +class JestClientTemporalLiteralSpec extends TemporalLiteralSpec diff --git a/es6/rest/src/test/scala/app/softnetwork/elastic/client/RestHighLevelClientTemporalLiteralSpec.scala b/es6/rest/src/test/scala/app/softnetwork/elastic/client/RestHighLevelClientTemporalLiteralSpec.scala new file mode 100644 index 000000000..7504a5c4f --- /dev/null +++ b/es6/rest/src/test/scala/app/softnetwork/elastic/client/RestHighLevelClientTemporalLiteralSpec.scala @@ -0,0 +1,19 @@ +/* + * Copyright 2025 SOFTNETWORK + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package app.softnetwork.elastic.client + +class RestHighLevelClientTemporalLiteralSpec extends TemporalLiteralSpec diff --git a/es7/rest/src/test/scala/app/softnetwork/elastic/client/RestHighLevelClientTemporalLiteralSpec.scala b/es7/rest/src/test/scala/app/softnetwork/elastic/client/RestHighLevelClientTemporalLiteralSpec.scala new file mode 100644 index 000000000..7504a5c4f --- /dev/null +++ b/es7/rest/src/test/scala/app/softnetwork/elastic/client/RestHighLevelClientTemporalLiteralSpec.scala @@ -0,0 +1,19 @@ +/* + * Copyright 2025 SOFTNETWORK + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package app.softnetwork.elastic.client + +class RestHighLevelClientTemporalLiteralSpec extends TemporalLiteralSpec diff --git a/es8/java/src/test/scala/app/softnetwork/elastic/client/JavaClientTemporalLiteralSpec.scala b/es8/java/src/test/scala/app/softnetwork/elastic/client/JavaClientTemporalLiteralSpec.scala new file mode 100644 index 000000000..6ed98b3b1 --- /dev/null +++ b/es8/java/src/test/scala/app/softnetwork/elastic/client/JavaClientTemporalLiteralSpec.scala @@ -0,0 +1,19 @@ +/* + * Copyright 2025 SOFTNETWORK + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package app.softnetwork.elastic.client + +class JavaClientTemporalLiteralSpec extends TemporalLiteralSpec diff --git a/es9/java/src/test/scala/app/softnetwork/elastic/client/JavaClientTemporalLiteralSpec.scala b/es9/java/src/test/scala/app/softnetwork/elastic/client/JavaClientTemporalLiteralSpec.scala new file mode 100644 index 000000000..6ed98b3b1 --- /dev/null +++ b/es9/java/src/test/scala/app/softnetwork/elastic/client/JavaClientTemporalLiteralSpec.scala @@ -0,0 +1,19 @@ +/* + * Copyright 2025 SOFTNETWORK + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package app.softnetwork.elastic.client + +class JavaClientTemporalLiteralSpec extends TemporalLiteralSpec diff --git a/sql/src/main/scala/app/softnetwork/elastic/sql/query/TemporalLiterals.scala b/sql/src/main/scala/app/softnetwork/elastic/sql/query/TemporalLiterals.scala new file mode 100644 index 000000000..892ef2766 --- /dev/null +++ b/sql/src/main/scala/app/softnetwork/elastic/sql/query/TemporalLiterals.scala @@ -0,0 +1,398 @@ +/* + * Copyright 2025 SOFTNETWORK + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package app.softnetwork.elastic.sql.query + +import app.softnetwork.elastic.sql._ +import app.softnetwork.elastic.sql.operator._ +import app.softnetwork.elastic.sql.schema.{Column, Schema} +import app.softnetwork.elastic.sql.`type`.{SQLTemporal, SQLTime, SQLType} + +import java.time.format.DateTimeFormatter +import java.time.{LocalDate, LocalTime, ZoneOffset} +import scala.util.Try +import scala.util.matching.Regex + +/** Issue #276 -- resolve the string literals a WHERE clause compares against a `date`-mapped column + * into a spelling the column's mapping `format` accepts. + * + * A BI tool renders a timestamp as `'2026-06-04 00:00:00.000000'` (the SQL standard spelling); + * Elasticsearch's default `date` format (`strict_date_optional_time||epoch_millis`) accepts only + * the `T`-separated ISO-8601 form, so the range/term query built from the verbatim literal was + * rejected with a raw `search_phase_execution_exception`. The rewrite happens ONCE per statement, + * on the AST, against the MAPPED type -- never per hit and never through a "does this string look + * like a date?" heuristic in the bridge (softclient4es-arrow#139's `tryParseAsDateTime` lesson). + * + * What is rewritten: the `StringValue` operand of a `GenericExpression` whose operator is one of + * `= <> != >= > <= <`, both bounds of a literal `BETWEEN`, and every member of a string `IN` list + * -- when the left-hand side is a FUNCTION-FREE identifier that resolves through `schema.find` to + * a temporal column other than `TIME`, and belongs to the statement's own source (an identifier + * qualified with a cross-index JOIN alias is left alone: the schema in hand is not its table's). + * `LIKE` / `RLIKE` patterns, function-wrapped columns (`YEAR(ts) = 2026` runs as a Painless + * script, not a range query), `HAVING`, `CASE` conditions and `keyword`/`text` columns are never + * touched. + * + * The rule per literal, in order (see [[FieldFormat]] for the format vocabulary): + * 1. a number (`epoch_millis` / `epoch_second`) or a date-math expression (`now-1d/d`, + * `2026-06-04||/M`) is forwarded verbatim; + * 1. a literal one of the mapping's CUSTOM patterns already parses is forwarded verbatim -- a + * `format: "yyyy-MM-dd HH:mm:ss"` column parses the space form TODAY and must keep working; + * 1. if the mapping accepts an ISO optional-time built-in: a calendar literal (`yyyy-MM-dd`, + * optionally followed by a `T` or a SPACE, a time and a zone) is validated with `java.time`; + * the SQL space form is then rewritten with a `T` (fraction digits preserved, a lower-case + * `z` upper-cased, nothing else changes) and an already-ISO one is forwarded verbatim; other + * ISO shapes (`yyyy`, `yyyy-MM`, ordinal and week dates) are forwarded verbatim; + * 1. anything else -- an unrecognised shape or an invalid calendar value -- is REJECTED with a + * message naming the literal, the field and the format, but only when every alternative of + * the format is one this object fully emulates (the ISO optional-time and epoch names). With + * a custom or unrecognised built-in in the list we cannot prove Elasticsearch rejects the + * literal, so it is forwarded verbatim. + * + * The schema-absent path (no schema attached, statement over several indices, wildcard source, + * schema lookup failure) is the CALLER's decision and means "forward verbatim" -- this object + * never guesses a type. + */ +object TemporalLiterals { + + /** Elasticsearch's default `date` mapping format. */ + val DefaultDateFormat: String = "strict_date_optional_time||epoch_millis" + + /** Longest literal echoed back in a rejection message; longer ones are cut head + tail. */ + val MaxLiteralExcerpt: Int = 120 + + /** The ISO optional-time built-ins: date mandatory, `T`-separated time optional. */ + private val IsoOptionalTimeFormats: Set[String] = + Set("strict_date_optional_time", "date_optional_time", "strict_date_optional_time_nanos") + + private val EpochFormats: Set[String] = Set("epoch_millis", "epoch_second") + + /** A built-in format NAME (`basic_date_time`, `date_hour_minute`, ...) as opposed to a custom + * pattern, which always carries an upper-case letter, a separator or a quote. + */ + private val BuiltInName: Regex = "^[a-z][a-z0-9_]*$".r + + private val NumericLiteral: Regex = "^-?\\d+(?:\\.\\d+)?$".r + + /** Elasticsearch date math anchored on `now`: `now`, `now-1d`, `now+1h/h`, `now/d`. */ + private val NowDateMath: Regex = "^now(?:[+-]\\d+[yMwdhHms]|/[yMwdhHms])*$".r + + // `strict_date_optional_time` grammar, read as a lenient SUPERSET so that a spelling + // Elasticsearch accepts is never rejected here. + private val IsoTime = "\\d{2}(?::\\d{2}(?::\\d{2}(?:[.,]\\d{1,9})?)?)?" + private val IsoZone = "(?:[Zz]|[+-]\\d{2}(?::?\\d{2})?)?" + + /** A calendar date, optionally followed by a `T` OR a SPACE, a time and a zone. */ + private val CalendarLiteral: Regex = + ("^(\\d{4}-\\d{2}-\\d{2})(?:([ T])(" + IsoTime + ")(" + IsoZone + "))?$").r + + /** The other shapes the ISO optional-time parsers accept (year, year-month, the ordinal + * `yyyy-DDD` and week `yyyy-Www[-e]` dates of the Joda parser behind ES 6.8, a date followed + * directly by a zone) -- forwarded verbatim, not validated. + */ + private val OtherIsoLiteral: Regex = + ("^(?:\\d{4}(?:-\\d{2})?|\\d{4}-\\d{3}|\\d{4}-W\\d{2}(?:-\\d)?|\\d{4}-\\d{2}-\\d{2})" + IsoZone + "$").r + + /** A `date` column's effective mapping `format`, split on `||`. */ + final case class FieldFormat(spec: String) { + + val alternatives: Seq[String] = + spec.split("\\|\\|").toSeq.map(_.trim).filter(_.nonEmpty) + + private def isIso(alternative: String): Boolean = IsoOptionalTimeFormats.contains(alternative) + + private def isEpoch(alternative: String): Boolean = EpochFormats.contains(alternative) + + /** At least one alternative accepts the `T`-separated ISO form we normalise to. */ + val acceptsIsoOptionalTime: Boolean = alternatives.exists(isIso) + + /** Every alternative is one this object can emulate exactly -- the ONLY case in which a literal + * may be rejected rather than forwarded verbatim. + */ + val fullyUnderstood: Boolean = + alternatives.nonEmpty && alternatives.forall(a => isIso(a) || isEpoch(a)) + + /** The alternatives that are custom `DateTimeFormatter` patterns (not built-in names). */ + val customPatterns: Seq[String] = + alternatives.filterNot(a => + isIso(a) || isEpoch(a) || BuiltInName.pattern.matcher(a).matches() + ) + + private lazy val customFormatters: Seq[DateTimeFormatter] = + customPatterns.flatMap { pattern => + // Elasticsearch 7.x accepted a leading `8` to select the java.time parser during the Joda + // migration; it is not part of the pattern. + val javaPattern = + if (pattern.length > 1 && pattern.charAt(0) == '8') pattern.substring(1) else pattern + Try(DateTimeFormatter.ofPattern(javaPattern)).toOption + } + + /** True when one of the custom patterns parses the literal -- Elasticsearch will too. */ + def acceptsAsCustom(literal: String): Boolean = + customFormatters.exists(formatter => Try(formatter.parse(literal)).isSuccess) + } + + object FieldFormat { + + /** The column's explicit `format` option, else Elasticsearch's default. */ + def of(column: Column): FieldFormat = + column.options.get("format") match { + case Some(value) => + FieldFormat(Option(value.value).map(_.toString).getOrElse(DefaultDateFormat)) + case None => FieldFormat(DefaultDateFormat) + } + } + + /** `now`-anchored or `||`-suffixed date math is resolved by Elasticsearch itself. The `||` form + * is accepted leniently (its date part is parsed by the field's format, whatever that is). + */ + def isDateMath(literal: String): Boolean = + NowDateMath.pattern.matcher(literal).matches() || literal.contains("||") + + /** Normalise ONE literal compared against `field`. + * + * @return + * `Right(None)` to forward the literal verbatim, `Right(Some(v))` to replace it by `v`, + * `Left(reason)` to reject the statement with a message naming the literal and the field. + */ + def normalizeLiteral( + literal: String, + field: String, + format: FieldFormat + ): Either[String, Option[String]] = { + if (NumericLiteral.pattern.matcher(literal).matches() || isDateMath(literal)) Right(None) + else if (format.acceptsAsCustom(literal)) Right(None) + else if (!format.acceptsIsoOptionalTime) Right(None) + else + literal match { + case CalendarLiteral(date, null, _, _) => + if (validDate(date)) Right(None) else rejectOrForward(literal, field, format) + case CalendarLiteral(date, separator, time, zone) => + if (validDate(date) && validTime(time) && validZone(zone)) { + if (separator == " ") Right(Some(date + "T" + time + zone.toUpperCase)) + else Right(None) + } else rejectOrForward(literal, field, format) + case _ if OtherIsoLiteral.pattern.matcher(literal).matches() => Right(None) + case _ => rejectOrForward(literal, field, format) + } + } + + private def rejectOrForward( + literal: String, + field: String, + format: FieldFormat + ): Either[String, Option[String]] = + if (format.fullyUnderstood) Left(rejection(literal, field, format)) else Right(None) + + private def validDate(date: String): Boolean = Try(LocalDate.parse(date)).isSuccess + + private def validTime(time: String): Boolean = { + val isoTime = time.replace(',', '.') + val parseableTime = if (isoTime.length == 2) isoTime + ":00" else isoTime + Try(LocalTime.parse(parseableTime)).isSuccess + } + + private def validZone(zone: String): Boolean = + zone.isEmpty || Try(ZoneOffset.of(zone.toUpperCase)).isSuccess + + /** The literal as echoed in a rejection: control characters and line separators collapsed to a + * space (the message reaches a log record, a `SQLException` and a terminal), the length bounded + * head + tail so a hostile or accidental multi-kilobyte literal cannot flood either. + */ + private[query] def excerpt(literal: String): String = { + val flat = literal.replaceAll("[\\p{Cntrl}\\u0085\\u2028\\u2029]+", " ") + if (flat.length <= MaxLiteralExcerpt) flat + else { + val head = MaxLiteralExcerpt * 2 / 3 + val tail = MaxLiteralExcerpt - head - 3 + flat.substring(0, head) + "..." + flat.substring(flat.length - tail) + } + } + + private def rejection(literal: String, field: String, format: FieldFormat): String = + s"Cannot parse '${excerpt(literal)}' as a date/time value for date field '$field' " + + s"(mapping format '${format.spec}'): expected an ISO-8601 date or timestamp " + + "('yyyy-MM-dd', 'yyyy-MM-ddTHH:mm:ss[.fraction][zone]'), the SQL spelling " + + "'yyyy-MM-dd HH:mm:ss[.fraction][zone]', epoch milliseconds, or a date-math expression" + + /** `DATE` / `DATETIME` / `TIMESTAMP` / `TEMPORAL` -- every `date` mapping resolves to one of + * these. `TIME` is excluded: Elasticsearch has no time-only mapping and a bare time is not an + * ISO optional-time literal. `date_nanos` resolves to `ANY` on the AST and is therefore never a + * candidate (excluded by construction -- see the unit test that pins it). + */ + private def isTemporalColumn(dataType: SQLType): Boolean = dataType match { + case _: SQLTime => false + case _: SQLTemporal => true + case _ => false + } + + /** Only a bare column reaches `rangeQuery` / `termQuery` with the literal as value. */ + private def plainColumn(identifier: GenericIdentifier): Boolean = + identifier.functions.isEmpty && identifier.name.nonEmpty + + /** The identifier's column in `schema`, unless the identifier belongs to a cross-index JOIN + * source (`joinSources`): that table's mapping is not the one in hand. + */ + private def temporalColumn( + identifier: GenericIdentifier, + schema: Schema, + joinSources: Set[String] + ): Option[Column] = + if (identifier.table.exists(joinSources.contains)) None + else schema.find(identifier.name).filter(column => isTemporalColumn(column.dataType)) + + private def rangeOrEquality(operator: ComparisonOperator): Boolean = operator match { + case EQ | NE | DIFF | GE | GT | LE | LT => true + case _ => false + } + + /** True when the WHERE clause carries at least one candidate operand -- the caller skips the + * schema lookup entirely otherwise, so a statement without a temporal-looking predicate costs + * nothing. + */ + def hasCandidates(search: SingleSearch): Boolean = + search.where.flatMap(_.criteria).exists(candidate) + + private def candidate(criteria: Criteria): Boolean = criteria match { + case Predicate(left, _, right, _, _) => candidate(left) || candidate(right) + case relation: ElasticRelation => candidate(relation.criteria) + case e: GenericExpression => + (e.identifier, e.operator, e.value) match { + case (id: GenericIdentifier, op: ComparisonOperator, _: StringValue) => + rangeOrEquality(op) && plainColumn(id) + case _ => false + } + case b: BetweenExpr => + (b.identifier, b.fromTo) match { + case (id: GenericIdentifier, _: LiteralFromTo) => plainColumn(id) + case _ => false + } + case in: InExpr[_, _] => + (in.identifier, in.values) match { + case (id: GenericIdentifier, _: StringValues) => plainColumn(id) + case _ => false + } + case _ => false + } + + /** Resolve every temporal literal of `search`'s WHERE clause against `schema`. + * + * @return + * `Right(search)` (the SAME instance when nothing changed), or `Left(reason)`. + */ + def apply(search: SingleSearch, schema: Schema): Either[String, SingleSearch] = + search.where.flatMap(_.criteria) match { + case Some(criteria) => + val joinSources: Set[String] = search.from.joinAliases.values.map(_._1).toSet + rewrite(criteria, schema, joinSources).map { rewritten => + if (rewritten eq criteria) search + else search.copy(where = Some(Where(Some(rewritten)))) + } + case None => Right(search) + } + + private def rewrite( + criteria: Criteria, + schema: Schema, + joinSources: Set[String] + ): Either[String, Criteria] = + criteria match { + case p: Predicate => + for { + left <- rewrite(p.leftCriteria, schema, joinSources) + right <- rewrite(p.rightCriteria, schema, joinSources) + } yield + if ((left eq p.leftCriteria) && (right eq p.rightCriteria)) p + else p.copy(leftCriteria = left, rightCriteria = right) + + case n: ElasticNested => + rewrite(n.criteria, schema, joinSources) + .map(c => if (c eq n.criteria) n else n.copy(criteria = c)) + + case n: ElasticChild => + rewrite(n.criteria, schema, joinSources) + .map(c => if (c eq n.criteria) n else n.copy(criteria = c)) + + case n: ElasticParent => + rewrite(n.criteria, schema, joinSources) + .map(c => if (c eq n.criteria) n else n.copy(criteria = c)) + + case e: GenericExpression => + (e.identifier, e.operator, e.value) match { + case (id: GenericIdentifier, op: ComparisonOperator, literal: StringValue) + if rangeOrEquality(op) && plainColumn(id) => + temporalColumn(id, schema, joinSources) match { + case Some(column) => + normalizeLiteral(literal.value, id.name, FieldFormat.of(column)).map { + case Some(normalized) => e.copy(value = StringValue(normalized)) + case None => e + } + case None => Right(e) + } + case _ => Right(e) + } + + case b: BetweenExpr => + (b.identifier, b.fromTo) match { + case (id: GenericIdentifier, LiteralFromTo(from, to)) if plainColumn(id) => + temporalColumn(id, schema, joinSources) match { + case Some(column) => + val format = FieldFormat.of(column) + for { + lower <- normalizeLiteral(from.value, id.name, format) + upper <- normalizeLiteral(to.value, id.name, format) + } yield + if (lower.isEmpty && upper.isEmpty) b + else + b.copy(fromTo = + LiteralFromTo( + StringValue(lower.getOrElse(from.value)), + StringValue(upper.getOrElse(to.value)) + ) + ) + case None => Right(b) + } + case _ => Right(b) + } + + case in: InExpr[_, _] => + (in.identifier, in.values) match { + case (id: GenericIdentifier, strings: StringValues) if plainColumn(id) => + temporalColumn(id, schema, joinSources) match { + case Some(column) => + val format = FieldFormat.of(column) + val zero: Either[String, (Seq[StringValue], Boolean)] = Right((Seq.empty, false)) + strings.values + .foldLeft(zero) { + case (Left(reason), _) => Left(reason) + case (Right((acc, changed)), value) => + normalizeLiteral(value.value, id.name, format).map { + case Some(normalized) => (acc :+ StringValue(normalized), true) + case None => (acc :+ value, changed) + } + } + .map { case (values, changed) => + val result: Criteria = + if (changed) InExpr(id, StringValues(values), in.maybeNot) else in + result + } + case None => Right(in) + } + case _ => Right(in) + } + + case other => Right(other) + } +} diff --git a/sql/src/test/scala/app/softnetwork/elastic/sql/query/TemporalLiteralsSpec.scala b/sql/src/test/scala/app/softnetwork/elastic/sql/query/TemporalLiteralsSpec.scala new file mode 100644 index 000000000..eb04cce56 --- /dev/null +++ b/sql/src/test/scala/app/softnetwork/elastic/sql/query/TemporalLiteralsSpec.scala @@ -0,0 +1,368 @@ +package app.softnetwork.elastic.sql.query + +import app.softnetwork.elastic.schema.Index +import app.softnetwork.elastic.sql._ +import app.softnetwork.elastic.sql.parser.Parser +import app.softnetwork.elastic.sql.query.TemporalLiterals.FieldFormat +import app.softnetwork.elastic.sql.schema.{Column, Table => SchemaTable} +import app.softnetwork.elastic.sql.`type`.SQLTypes +import org.scalatest.flatspec.AnyFlatSpec +import org.scalatest.matchers.should.Matchers + +import scala.collection.immutable.ListMap + +/** Issue #276 -- the AST-level normalisation of string literals compared against `date`-mapped + * columns. The bridge emission is asserted in `TemporalLiteralQuerySpec` (bridge module) and the + * core seam in `TemporalLiteralSearchSpec` (core module); this spec pins the RULES. + */ +class TemporalLiteralsSpec extends AnyFlatSpec with Matchers { + + private val default = FieldFormat(TemporalLiterals.DefaultDateFormat) + private val custom = FieldFormat("yyyy-MM-dd HH:mm:ss") + private val mixed = FieldFormat("yyyy-MM-dd HH:mm:ss||strict_date_optional_time") + private val opaque = FieldFormat("basic_date_time||epoch_millis") + private val nanos = FieldFormat("strict_date_optional_time_nanos||epoch_millis") + + private val field = "event_ts" + + private def norm(literal: String, format: FieldFormat = default): Either[String, Option[String]] = + TemporalLiterals.normalizeLiteral(literal, field, format) + + // ---- FieldFormat ----------------------------------------------------------------------------- + + "FieldFormat" should "classify Elasticsearch's default date format" in { + default.alternatives shouldBe Seq("strict_date_optional_time", "epoch_millis") + default.acceptsIsoOptionalTime shouldBe true + default.fullyUnderstood shouldBe true + default.customPatterns shouldBe empty + } + + it should "classify a custom pattern as neither ISO-accepting nor fully understood" in { + custom.acceptsIsoOptionalTime shouldBe false + custom.fullyUnderstood shouldBe false + custom.customPatterns shouldBe Seq("yyyy-MM-dd HH:mm:ss") + custom.acceptsAsCustom("2026-06-04 00:00:00") shouldBe true + custom.acceptsAsCustom("2026-06-04T00:00:00") shouldBe false + } + + it should "classify a mixed list" in { + mixed.acceptsIsoOptionalTime shouldBe true + mixed.fullyUnderstood shouldBe false + mixed.customPatterns shouldBe Seq("yyyy-MM-dd HH:mm:ss") + } + + it should "treat an unrecognised built-in name as opaque (never a custom pattern)" in { + opaque.acceptsIsoOptionalTime shouldBe false + opaque.fullyUnderstood shouldBe false + opaque.customPatterns shouldBe empty + } + + it should "understand the date_nanos default format" in { + nanos.acceptsIsoOptionalTime shouldBe true + nanos.fullyUnderstood shouldBe true + } + + it should "strip the Elasticsearch 7 java-time '8' prefix from a custom pattern" in { + FieldFormat("8yyyy-MM-dd").acceptsAsCustom("2026-06-04") shouldBe true + } + + it should "ignore an unparseable custom pattern instead of failing" in { + FieldFormat("yyyy-MM-dd {").acceptsAsCustom("2026-06-04") shouldBe false + } + + it should "read a column's format option, else fall back to the default" in { + FieldFormat.of(Column("ts", SQLTypes.Date)) shouldBe default + FieldFormat.of( + Column("ts", SQLTypes.Date, options = ListMap("format" -> StringValue("yyyy-MM-dd HH:mm:ss"))) + ) shouldBe custom + } + + // ---- normalizeLiteral ------------------------------------------------------------------------ + + "normalizeLiteral" should "rewrite the SQL space form to the T form under the default format" in { + norm("2026-06-04 00:00:00.000000") shouldBe Right(Some("2026-06-04T00:00:00.000000")) + norm("2026-06-04 00:00:00") shouldBe Right(Some("2026-06-04T00:00:00")) + norm("2026-06-04 00:00") shouldBe Right(Some("2026-06-04T00:00")) + norm("2026-06-04 10") shouldBe Right(Some("2026-06-04T10")) + norm("2026-06-04 00:00:00.123+02:00") shouldBe Right(Some("2026-06-04T00:00:00.123+02:00")) + norm("2026-06-04 00:00:00+0200") shouldBe Right(Some("2026-06-04T00:00:00+0200")) + norm("2026-06-04 00:00:00Z") shouldBe Right(Some("2026-06-04T00:00:00Z")) + norm("2026-06-04 00:00:00z") shouldBe Right(Some("2026-06-04T00:00:00Z")) // zone upper-cased + norm("2026-06-04 23:59:59,5") shouldBe Right(Some("2026-06-04T23:59:59,5")) + norm("2026-06-04 00:00:00.123456789") shouldBe Right(Some("2026-06-04T00:00:00.123456789")) + } + + it should "leave every ISO spelling untouched" in { + Seq( + "2026-06-04T00:00:00", + "2026-06-04T00:00:00.000000", + "2026-06-04T00:00:00Z", + "2026-06-04T10:30:15.123456789+01:00", + "2026-06-04T10", + "2026-06-04", + "2026-06", + "2026-155", + "2026-W23-4" + ).foreach(literal => withClue(literal)(norm(literal) shouldBe Right(None))) + } + + it should "leave epoch numbers and date math untouched" in { + Seq( + "1780531200000", + "-1", + "1780531200", + "1780531200000.5", + "now", + "now-1d/d", + "now+1h", + "now+1M/M", + "now/d", + "2026-06-04||/M", + "2026-06-04 00:00:00||+1d" + ).foreach(literal => withClue(literal)(norm(literal) shouldBe Right(None))) + } + + it should "reject an unparseable literal under the default format, naming the literal and the field" in { + Seq( + "not-a-date", + "nowhere", // starts with `now` but is not date math + "NOW-1d", // Elasticsearch date math is lower-case + "2026-13-45 99:99:99", + "2026-06-04 24:00:00", + "2026-13-45T00:00:00", // T form with an invalid calendar value + "2026-02-30", // date-only with an invalid calendar value + "", + "04/06/2026", + "2026-06-04 00:00:00 UTC", + "2026-06-04 00:00:00" // two spaces + ).foreach { literal => + norm(literal) match { + case Left(reason) => + withClue(literal) { + reason should include(s"'$literal'") + reason should include(s"'$field'") + reason should include(TemporalLiterals.DefaultDateFormat) + reason should not startWith "Internal parser error" + } + case other => fail(s"'$literal' should be rejected, got $other") + } + } + } + + it should "bound and sanitise the literal echoed in the rejection" in { + val long = "x" * 500 + norm(long) match { + case Left(reason) => + reason should include("...") + // the fixed wording plus at most MaxLiteralExcerpt characters of the literal + reason.length should be < (TemporalLiterals.MaxLiteralExcerpt + 400) + reason should not include long + case other => fail(s"expected a rejection, got $other") + } + norm("bad\nliteral\u0007here") match { + case Left(reason) => + reason should not include "\n" + reason should not include "\u0007" + reason should include("'bad literal here'") + case other => fail(s"expected a rejection, got $other") + } + TemporalLiterals.excerpt("short") shouldBe "short" + } + + it should "never reject under a custom or opaque format" in { + norm("not-a-date", custom) shouldBe Right(None) + norm("2026-06-04 00:00:00", custom) shouldBe Right(None) // parity: the custom pattern parses it + norm("2026-06-04T00:00:00", custom) shouldBe Right(None) // not ours to fix: no ISO alternative + norm("not-a-date", opaque) shouldBe Right(None) + norm("2026-06-04 00:00:00", opaque) shouldBe Right(None) + } + + it should "prefer parity with a custom alternative, and still fix the space form where ISO is accepted" in { + norm("2026-06-04 00:00:00", mixed) shouldBe Right(None) // the custom pattern parses it + norm("2026-06-04 00:00", mixed) shouldBe Right(Some("2026-06-04T00:00")) // custom needs seconds + norm("garbage", mixed) shouldBe Right(None) // not fully understood => never rejected + } + + it should "rewrite the space form under the date_nanos default format too" in { + norm("2026-06-04 00:00:00.123456789", nanos) shouldBe Right( + Some("2026-06-04T00:00:00.123456789") + ) + } + + // ---- statement level --------------------------------------------------------------------------- + + private val schema: SchemaTable = SchemaTable( + "events", + columns = List( + Column("id", SQLTypes.Keyword), + Column("event_ts", SQLTypes.Date), + Column("created", SQLTypes.Timestamp), + Column("t", SQLTypes.Time), + Column("label", SQLTypes.Keyword), + Column("amount", SQLTypes.Int), + Column( + "fmt_ts", + SQLTypes.Date, + options = ListMap("format" -> StringValue("yyyy-MM-dd HH:mm:ss")) + ) + ) + ) + + private def single(sql: String): SingleSearch = Parser(sql) match { + case Right(s: SingleSearch) => s + case other => fail(s"$sql did not parse as a SingleSearch: $other") + } + + private def whereSql(search: SingleSearch): String = search.where.map(_.sql.trim).getOrElse("") + + private def resolved(sql: String, table: SchemaTable = schema): SingleSearch = + TemporalLiterals(single(sql), table) match { + case Right(s) => s + case Left(reason) => fail(s"$sql was rejected: $reason") + } + + private def untouched(sql: String, table: SchemaTable = schema): Unit = { + val parsed = single(sql) + TemporalLiterals(parsed, table) match { + case Right(s) => withClue(sql)(s should be theSameInstanceAs parsed) + case Left(reason) => fail(s"$sql was rejected: $reason") + } + } + + "TemporalLiterals" should "rewrite a range comparison against a date column" in { + val sql = "SELECT id FROM events WHERE event_ts >= '2026-06-04 00:00:00.000000'" + TemporalLiterals.hasCandidates(single(sql)) shouldBe true + whereSql(resolved(sql)) shouldBe "WHERE event_ts >= '2026-06-04T00:00:00.000000'" + } + + it should "rewrite equality, BETWEEN and IN through AND/OR nesting and a table alias" in { + val where = whereSql( + resolved( + "SELECT id FROM events e WHERE (e.event_ts = '2026-06-04 00:00:00' OR e.created BETWEEN " + + "'2026-06-01 00:00:00' AND '2026-06-30 23:59:59') AND e.event_ts IN ('2026-06-04 00:00:00', " + + "'2026-07-01 00:00:00') AND e.amount > 1" + ) + ) + where should include("event_ts = '2026-06-04T00:00:00'") + where should include("BETWEEN '2026-06-01T00:00:00' AND '2026-06-30T23:59:59'") + where should include("IN ('2026-06-04T00:00:00','2026-07-01T00:00:00')") + where should include("amount > 1") + where should not include " 00:00:00'" + } + + it should "rewrite the NOT / != / < spellings as well" in { + whereSql(resolved("SELECT id FROM events WHERE NOT event_ts < '2026-06-04 00:00:00'")) should + include("'2026-06-04T00:00:00'") + whereSql(resolved("SELECT id FROM events WHERE event_ts != '2026-06-04 00:00:00'")) should + include("'2026-06-04T00:00:00'") + whereSql(resolved("SELECT id FROM events WHERE event_ts NOT IN ('2026-06-04 00:00:00')")) should + include("'2026-06-04T00:00:00'") + } + + it should "return the very same statement, and report no candidate, when nothing qualifies" in { + val sql = "SELECT id FROM events WHERE amount > 10 AND label IS NOT NULL" + TemporalLiterals.hasCandidates(single(sql)) shouldBe false + untouched(sql) + TemporalLiterals.hasCandidates(single("SELECT id FROM events")) shouldBe false + TemporalLiterals.hasCandidates( + single("SELECT id FROM events WHERE YEAR(event_ts) = 2026") + ) shouldBe false + } + + it should "leave keyword, TIME, LIKE, function-wrapped, unknown and custom-format columns untouched" in { + untouched("SELECT id FROM events WHERE label = '2026-06-04 00:00:00'") // AC 3 negative control + untouched("SELECT id FROM events WHERE t = '10:30:00'") + untouched("SELECT id FROM events WHERE event_ts LIKE '2026-06-04%'") + untouched("SELECT id FROM events WHERE event_ts RLIKE '2026.*'") + untouched("SELECT id FROM events WHERE unknown_col >= '2026-06-04 00:00:00'") + untouched("SELECT id FROM events WHERE fmt_ts >= '2026-06-04 00:00:00'") // AC 2 parity + untouched("SELECT id FROM events WHERE event_ts >= '2026-06-04T00:00:00'") // already ISO + untouched("SELECT id FROM events WHERE event_ts >= '1780531200000'") // AC 4 epoch millis + untouched("SELECT id FROM events WHERE event_ts > 'now-1d/d'") // date math + } + + it should "reject an unparseable literal against a date column, naming the literal and the field" in { + def rejects(sql: String, literal: String, column: String): Unit = + TemporalLiterals(single(sql), schema) match { + case Left(reason) => + withClue(sql) { + reason should include(s"'$literal'") + reason should include(s"'$column'") + reason should not startWith "Internal parser error" + } + case Right(s) => fail(s"$sql should have been rejected, got ${whereSql(s)}") + } + rejects("SELECT id FROM events WHERE event_ts >= 'not-a-date'", "not-a-date", "event_ts") + rejects( + "SELECT id FROM events WHERE created BETWEEN '2026-06-01 00:00:00' AND 'nope'", + "nope", + "created" + ) + rejects( + "SELECT id FROM events WHERE event_ts IN ('2026-06-04 00:00:00', '2026-06-04 25:00:00')", + "2026-06-04 25:00:00", + "event_ts" + ) + rejects( + "SELECT id FROM events WHERE amount > 1 AND (event_ts = '' OR label = 'x')", + "", + "event_ts" + ) + } + + it should "leave an identifier qualified with a cross-index JOIN alias untouched" in { + // `o` is the statement's own source; `c` is a JOIN source whose mapping is NOT the one in hand. + // A `customers` schema with a `date` column of the same name must not be used for `c.event_ts`. + val sql = + "SELECT o.id FROM events o JOIN customers c ON o.id = c.id " + + "WHERE o.event_ts >= '2026-06-04 00:00:00' AND c.event_ts >= '2026-06-04 00:00:00'" + val where = whereSql(resolved(sql)) + where should include("o.event_ts >= '2026-06-04T00:00:00'") + where should include("c.event_ts >= '2026-06-04 00:00:00'") + } + + it should "resolve columns from a real Elasticsearch mapping, excluding date_nanos by construction" in { + val json = + """{"events":{"aliases":{},"mappings":{"properties":{ + | "id":{"type":"keyword"}, + | "event_ts":{"type":"date"}, + | "fmt_ts":{"type":"date","format":"yyyy-MM-dd HH:mm:ss"}, + | "ts_nanos":{"type":"date_nanos"}, + | "label":{"type":"keyword"}, + | "items":{"type":"nested","properties":{"ts":{"type":"date"},"qty":{"type":"integer"}}} + |}},"settings":{"index":{"number_of_shards":"1","number_of_replicas":"0"}}}}""".stripMargin + val table = Index("events", json).schema + + table.find("event_ts").map(_.dataType) shouldBe Some(SQLTypes.Date) + table.find("fmt_ts").flatMap(_.options.get("format")) shouldBe Some( + StringValue("yyyy-MM-dd HH:mm:ss") + ) + // `date_nanos` is not a SQL type the mapping resolves to: it comes out as ANY, so it is never + // a candidate. Pinned on purpose (AC 4: excluded, not covered). + table.find("ts_nanos").map(_.dataType) shouldBe Some(SQLTypes.Any) + table.find("items.ts").map(_.dataType) shouldBe Some(SQLTypes.Date) + + whereSql( + resolved("SELECT id FROM events WHERE event_ts >= '2026-06-04 00:00:00'", table) + ) shouldBe + "WHERE event_ts >= '2026-06-04T00:00:00'" + untouched("SELECT id FROM events WHERE ts_nanos >= '2026-06-04 00:00:00'", table) + untouched("SELECT id FROM events WHERE fmt_ts >= '2026-06-04 00:00:00'", table) + untouched("SELECT id FROM events WHERE label = '2026-06-04 00:00:00'", table) + } + + it should "walk into an UNNEST nested criteria" in { + val json = + """{"events":{"aliases":{},"mappings":{"properties":{ + | "id":{"type":"keyword"}, + | "items":{"type":"nested","properties":{"ts":{"type":"date"},"qty":{"type":"integer"}}} + |}},"settings":{"index":{"number_of_shards":"1","number_of_replicas":"0"}}}}""".stripMargin + val table = Index("events", json).schema + val sql = + "SELECT id FROM events e JOIN UNNEST(e.items) AS i WHERE i.ts >= '2026-06-04 00:00:00' AND i.qty > 0" + TemporalLiterals.hasCandidates(single(sql)) shouldBe true + val where = whereSql(resolved(sql, table)) + where should include("'2026-06-04T00:00:00'") + where should not include "'2026-06-04 00:00:00'" + } +} diff --git a/testkit/src/main/scala/app/softnetwork/elastic/client/TemporalLiteralSpec.scala b/testkit/src/main/scala/app/softnetwork/elastic/client/TemporalLiteralSpec.scala new file mode 100644 index 000000000..58c6334a0 --- /dev/null +++ b/testkit/src/main/scala/app/softnetwork/elastic/client/TemporalLiteralSpec.scala @@ -0,0 +1,238 @@ +/* + * Copyright 2025 SOFTNETWORK + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package app.softnetwork.elastic.client + +import akka.NotUsed +import akka.actor.ActorSystem +import akka.stream.scaladsl.{Sink, Source} +import app.softnetwork.elastic.client.bulk._ +import app.softnetwork.elastic.client.result._ +import app.softnetwork.elastic.client.spi.ElasticClientFactory +import app.softnetwork.elastic.scalatest.ElasticDockerTestKit +import app.softnetwork.elastic.sql.query.SelectStatement +import app.softnetwork.persistence.generateUUID +import org.scalatest.flatspec.AnyFlatSpecLike +import org.scalatest.matchers.should.Matchers +import org.slf4j.{Logger, LoggerFactory} + +import scala.collection.immutable.ListMap +import scala.concurrent.Await +import scala.concurrent.duration._ +import scala.language.implicitConversions + +/** Regression test for issue #276: a WHERE comparison against a `date`-mapped field must match the + * same rows whichever standard literal spelling the BI tool emits. + * + * Before the fix the SQL-standard space-separated spelling (`'2026-06-04 00:00:00.000000'`, what + * Superset / SQLAlchemy render) was forwarded verbatim and rejected by Elasticsearch's default + * `strict_date_optional_time||epoch_millis` format, while the `T`-separated form matched. Eight + * rows, ids asserted (not only counts). A second index carries a custom `format` that parses the + * space form TODAY: it must keep working (the fix must not invert the defect). A `keyword` column + * holding the same date-shaped string is the negative control: it must be compared verbatim. + * + * Both venues are exercised: the client API (`search`) and the gateway (`run`, the path the JDBC + * driver, the REPL and the Flight SQL sidecar take). + */ +trait TemporalLiteralSpec extends AnyFlatSpecLike with ElasticDockerTestKit with Matchers { + + lazy val log: Logger = LoggerFactory.getLogger(getClass.getName) + + implicit val system: ActorSystem = ActorSystem(generateUUID()) + + implicit val context: ConversionContext = NativeContext + + lazy val client: ElasticClientApi = ElasticClientFactory.create(elasticConfig) + + private val defaultIndex = "temporal_literal_default" + + private val customIndex = "temporal_literal_custom" + + /** Eight timestamps (UTC), ids `e1`..`e8`; `>= 2026-06-04T00:00:00` selects `e4`..`e8`. */ + private val timestamps: Seq[String] = Seq( + "2026-06-01T00:00:00", + "2026-06-02T12:00:00", + "2026-06-03T23:59:59", + "2026-06-04T00:00:00", + "2026-06-04T00:00:01", + "2026-06-05T10:00:00", + "2026-06-10T00:00:00", + "2026-07-01T00:00:00" + ) + + private val fromJune4: Set[String] = Set("e4", "e5", "e6", "e7", "e8") + + override def beforeAll(): Unit = { + super.beforeAll() + + val settings = """{"number_of_shards": 1, "number_of_replicas": 0}""" + + val defaultMapping = + """{ + | "properties": { + | "id": { "type": "keyword" }, + | "event_ts": { "type": "date" }, + | "label": { "type": "keyword" }, + | "amount": { "type": "integer" } + | } + |}""".stripMargin + + val customMapping = + """{ + | "properties": { + | "id": { "type": "keyword" }, + | "event_ts": { "type": "date", "format": "yyyy-MM-dd HH:mm:ss" }, + | "amount": { "type": "integer" } + | } + |}""".stripMargin + + client.createIndex(defaultIndex, settings = settings).get shouldBe true + client.setMapping(defaultIndex, defaultMapping).get shouldBe true + client.createIndex(customIndex, settings = settings).get shouldBe true + client.setMapping(customIndex, customMapping).get shouldBe true + + val defaultDocs = timestamps.zipWithIndex.map { case (ts, i) => + // `label` holds the SQL spelling of the same instant: the keyword negative control + s"""{"id":"e${i + 1}","event_ts":"$ts","label":"${ts.replace('T', ' ')}","amount":${i + 1}}""" + }.toList + + val customDocs = timestamps.zipWithIndex.map { case (ts, i) => + s"""{"id":"e${i + 1}","event_ts":"${ts.replace('T', ' ')}","amount":${i + 1}}""" + }.toList + + implicit def listToSource[T](list: List[T]): Source[T, NotUsed] = + Source.fromIterator(() => list.iterator) + + def load(index: String, docs: List[String]): Unit = { + implicit val bulkOptions: BulkOptions = BulkOptions(defaultIndex = index, logEvery = 1000) + client.bulk[String](docs, identity, idKey = Some(Set("id"))) match { + case ElasticSuccess(_) => // ok + case ElasticFailure(error) => + error.cause.foreach(_.printStackTrace()) + fail(s"Bulk indexing into $index failed: ${error.message}") + } + client.refresh(index) + } + + load(defaultIndex, defaultDocs) + load(customIndex, customDocs) + } + + override def afterAll(): Unit = { + client.deleteIndex(defaultIndex) + client.deleteIndex(customIndex) + super.afterAll() + } + + private def ids(rows: Seq[ListMap[String, Any]]): Set[String] = + rows.flatMap(_.get("id").map(_.toString)).toSet + + /** Client venue: `SearchApi.search`. */ + private def searchIds(sql: String): Set[String] = + client.search(SelectStatement(sql)) match { + case ElasticSuccess(response) => ids(response.results) + case ElasticFailure(error) => fail(s"Query failed: ${error.message}\n$sql") + } + + /** Gateway venue: `GatewayApi.run` -- what the JDBC driver, the REPL and the sidecar call. */ + private def gatewayIds(sql: String): Set[String] = + Await.result(client.run(sql), 60.seconds) match { + case ElasticSuccess(QueryRows(rows, _)) => ids(rows) + case ElasticSuccess(QueryStructured(response, _)) => ids(response.results) + case ElasticSuccess(QueryStream(stream, _)) => + ids(Await.result(stream.map(_._1).runWith(Sink.seq), 60.seconds)) + case ElasticSuccess(other) => fail(s"Unexpected result: $other") + case ElasticFailure(error) => fail(s"Query failed: ${error.message}\n$sql") + } + + // ---- AC 1: the three spellings select the same rows ------------------------------------------- + + "a range comparison against a default-format date field" should "match the same rows for every standard spelling" in { + Seq("2026-06-04 00:00:00.000000", "2026-06-04 00:00:00", "2026-06-04T00:00:00").foreach { + literal => + withClue(literal) { + searchIds(s"SELECT id FROM $defaultIndex WHERE event_ts >= '$literal'") shouldBe fromJune4 + } + } + log.info(s"OK: the three spellings each select ${fromJune4.size} rows on $defaultIndex") + } + + it should "match the same rows through the gateway (JDBC / REPL / sidecar venue)" in { + Seq("2026-06-04 00:00:00.000000", "2026-06-04T00:00:00").foreach { literal => + withClue(literal) { + gatewayIds(s"SELECT id FROM $defaultIndex WHERE event_ts >= '$literal'") shouldBe fromJune4 + } + } + } + + "equality, BETWEEN and IN against a default-format date field" should "accept the space form" in { + searchIds(s"SELECT id FROM $defaultIndex WHERE event_ts = '2026-06-04 00:00:00'") shouldBe + Set("e4") + searchIds( + s"SELECT id FROM $defaultIndex WHERE event_ts BETWEEN '2026-06-02 00:00:00' AND '2026-06-04 00:00:00'" + ) shouldBe Set("e2", "e3", "e4") + searchIds( + s"SELECT id FROM $defaultIndex WHERE event_ts IN ('2026-06-04 00:00:00', '2026-07-01 00:00:00')" + ) shouldBe Set("e4", "e8") + searchIds(s"SELECT id FROM $defaultIndex WHERE event_ts < '2026-06-04 00:00:00'") shouldBe + Set("e1", "e2", "e3") + } + + // ---- AC 4: epoch millis untouched ---------------------------------------------------------------- + + "an epoch-millis literal against a default-format date field" should "still be accepted" in { + // 1780531200000 == 2026-06-04T00:00:00Z + searchIds(s"SELECT id FROM $defaultIndex WHERE event_ts >= '1780531200000'") shouldBe fromJune4 + } + + // ---- AC 3: keyword negative control ------------------------------------------------------------ + + "a keyword column holding a date-shaped string" should "be compared verbatim" in { + // `label` stores the space spelling; a rewrite to the T form would match nothing. + searchIds(s"SELECT id FROM $defaultIndex WHERE label = '2026-06-04 00:00:00'") shouldBe + Set("e4") + searchIds(s"SELECT id FROM $defaultIndex WHERE label = '2026-06-04T00:00:00'") shouldBe + Set.empty[String] + } + + // ---- AC 2: custom-format field keeps working ---------------------------------------------------- + + "a custom-format date field that accepts the space form" should "keep matching the same rows" in { + searchIds(s"SELECT id FROM $customIndex WHERE event_ts >= '2026-06-04 00:00:00'") shouldBe + fromJune4 + searchIds( + s"SELECT id FROM $customIndex WHERE event_ts BETWEEN '2026-06-02 00:00:00' AND '2026-06-04 00:00:00'" + ) shouldBe Set("e2", "e3", "e4") + gatewayIds(s"SELECT id FROM $customIndex WHERE event_ts >= '2026-06-04 00:00:00'") shouldBe + fromJune4 + } + + // ---- AC 6: unparseable literal is a named 400, not a raw shard failure ------------------------- + + "an unparseable literal against a default-format date field" should "fail naming the literal and the field" in { + client.search( + SelectStatement(s"SELECT id FROM $defaultIndex WHERE event_ts >= 'not-a-date'") + ) match { + case ElasticFailure(error) => + error.statusCode shouldBe Some(400) + error.message should include("'not-a-date'") + error.message should include("'event_ts'") + error.message should not include "search_phase_execution_exception" + case ElasticSuccess(response) => + fail(s"expected a rejection, got ${response.results.size} rows") + } + } +} From 6eeffebe424abeb54d067f6bfe60a16737b4d581 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Manciot?= Date: Sun, 6 Sep 2026 06:21:29 +0200 Subject: [PATCH 2/5] fix(sql,core): review follow-up for #276 - conservative rejection, bounded negative cache, DML WHERE Independent review of b15ce1ae returned approve-with-fixes; every finding is addressed here. - R4-1 (HIGH): the non-strict `date_optional_time` is no longer rejection-eligible. Its lenient grammar (`2026-6-4`, `2026-06-04T1:02:03`, 5-digit years) is accepted by every Elasticsearch major and was being turned into a 400. The space-form rewrite still applies under it. - R4-2 (MEDIUM): rejection is conservative. A literal is rejected only when it cannot be an ISO date at all or carries an invalid recognised calendar/time component (`2026-02-30`, `T24:00:00`); any shape the recogniser does not model (zone ids, offsets with seconds, a bare `T`, signed years, ES 6.8's Joda leniencies) is forwarded verbatim and Elasticsearch decides. The accept set is pinned as a table in TemporalLiteralsSpec against the real DateFormatter parsers of ES 6.8.23 / 7.17.29 / 8.18.3 / 9.0.3 (56 literals; every REJECT fails on all four, every REWRITE is accepted on all four). - R4-3 (MEDIUM): the negative schema-miss cache remembers 404 misses only (a transient failure is retried), purges expired entries above 256 and clears above 1024 - the #238 shardCountCache discipline; unit-tested. - R4-4 (MEDIUM): DELETE / UPDATE ... WHERE get the same resolution as SELECT at the five by-query render sites of IndicesApi (SearchApi.resolveTemporalLiterals is private[client]); two integration cases with exact row oracles on a dedicated index. - R4-6 stale "date followed by a zone" claim dropped; R4-7 the head of a `||` date-math literal is normalised (`2026-06-04 10:30:15||+1d` -> `2026-06-04T10:30:15||+1d`); R4-9 docs: only JOIN-alias-qualified columns keep their literals; R4-12 licence headers on the sql/bridge test files. R4-5 (index aliases) stays a documented and release-noted exclusion pending the lead's ruling on getIndex semantics. Story BIDC-4 Closes #276 Co-Authored-By: Claude Fable 5.1 --- .../sql/TemporalLiteralQuerySpec.scala | 16 ++ .../elastic/client/IndicesApi.scala | 92 +++++-- .../elastic/client/SearchApi.scala | 42 ++- .../client/TemporalLiteralSearchSpec.scala | 43 +++ documentation/sql/dql_statements.md | 30 +- .../sql/TemporalLiteralQuerySpec.scala | 16 ++ .../elastic/sql/query/TemporalLiterals.scala | 143 ++++++---- .../sql/query/TemporalLiteralsSpec.scala | 257 ++++++++++++------ .../elastic/client/TemporalLiteralSpec.scala | 25 ++ 9 files changed, 488 insertions(+), 176 deletions(-) diff --git a/bridge/src/test/scala/app/softnetwork/elastic/sql/TemporalLiteralQuerySpec.scala b/bridge/src/test/scala/app/softnetwork/elastic/sql/TemporalLiteralQuerySpec.scala index 0bd4d5bf5..616c17391 100644 --- a/bridge/src/test/scala/app/softnetwork/elastic/sql/TemporalLiteralQuerySpec.scala +++ b/bridge/src/test/scala/app/softnetwork/elastic/sql/TemporalLiteralQuerySpec.scala @@ -1,3 +1,19 @@ +/* + * Copyright 2025 SOFTNETWORK + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + package app.softnetwork.elastic.sql import app.softnetwork.elastic.sql.bridge._ diff --git a/core/src/main/scala/app/softnetwork/elastic/client/IndicesApi.scala b/core/src/main/scala/app/softnetwork/elastic/client/IndicesApi.scala index e73a61f8f..0393e3634 100644 --- a/core/src/main/scala/app/softnetwork/elastic/client/IndicesApi.scala +++ b/core/src/main/scala/app/softnetwork/elastic/client/IndicesApi.scala @@ -794,28 +794,51 @@ trait IndicesApi extends ElasticClientHelpers { ) ) else Right(()) - jsonQuery = delete.where match { + jsonQuery <- (delete.where match { case None => logger.info( s"SQL delete query has no WHERE clause, deleting all documents from index '$index'" ) - """{"query": {"match_all": {}}}""" + Right("""{"query": {"match_all": {}}}""") case Some(where) => implicit val timestamp: Long = System.currentTimeMillis() - val search: String = + // #276 -- the same temporal-literal resolution the equivalent SELECT gets + resolveDmlTemporalLiterals( SingleSearch( from = From(tables = Seq(delete.table)), where = Some(where), deleteByQuery = true - ) - logger.info(s"✅ Converted SQL delete query to search for deleteByQuery: $search") - search - } + ), + "deleteByQuery" + ).map { resolved => + val search: String = resolved + logger.info(s"✅ Converted SQL delete query to search for deleteByQuery: $search") + search + } + }): Either[ElasticError, String] deleted <- runDeleteByQuery(index, jsonQuery, refresh) } yield deleted finalizeDeleteByQuery(index, result) } + /** Issue #276 -- resolve the WHERE clause's temporal literals for a DELETE / UPDATE by-query + * search body exactly as [[SearchApi.resolveTemporalLiterals]] does for a SELECT (same rule, + * same schema-absent boundaries), so `DELETE FROM t WHERE ts >= '2026-06-04 00:00:00'` affects + * the rows the equivalent SELECT matches instead of failing with a raw shard error. + */ + private def resolveDmlTemporalLiterals( + single: SingleSearch, + operation: String + ): Either[ElasticError, SingleSearch] = + this match { + case api: SearchApi => + api.resolveTemporalLiterals(single) match { + case ElasticSuccess(resolved) => Right(resolved) + case ElasticFailure(error) => Left(error.copy(operation = Some(operation))) + } + case _ => Right(single) + } + private def validateDeleteIndex(index: String): Either[ElasticError, Unit] = validateIndexName(index) .toLeft(()) @@ -979,30 +1002,35 @@ trait IndicesApi extends ElasticClientHelpers { }) // 4. Build the JSON query to execute - jsonQuery <- Right(parsed match { + jsonQuery <- (parsed match { case Left(u: Update) => u.where match { case None => logger.info( s"SQL update query has no WHERE clause, updating all documents from index '$index'" ) - """{"query": {"match_all": {}}}""" + Right("""{"query": {"match_all": {}}}""") case Some(where) => implicit val timestamp: Long = System.currentTimeMillis() - val search: String = + // #276 -- the same temporal-literal resolution the equivalent SELECT gets + resolveDmlTemporalLiterals( SingleSearch( from = From(tables = Seq(Table(u.table))), where = Some(where), updateByQuery = true - ) - logger.info(s"✅ Converted SQL update query to search for updateByQuery: $search") - search + ), + "updateByQuery" + ).map { resolved => + val search: String = resolved + logger.info(s"✅ Converted SQL update query to search for updateByQuery: $search") + search + } } case Right(jsonOrConverted) => - jsonOrConverted - }) + Right(jsonOrConverted) + }): Either[ElasticError, String] // 5. Load user pipeline if provided userPipeline <- pipelineId match { @@ -1529,9 +1557,16 @@ trait IndicesApi extends ElasticClientHelpers { ) else { implicit val timestamp: Long = System.currentTimeMillis() - val query: String = search.copy(deleteByQuery = false) - logger.info(s"✅ Converted SQL search query to JSON for updateByQuery: $query") - ElasticSuccess(Right(query)) + resolveDmlTemporalLiterals( + search.copy(deleteByQuery = false), + "updateByQuery" + ) match { + case Right(resolved) => + val query: String = resolved + logger.info(s"✅ Converted SQL search query to JSON for updateByQuery: $query") + ElasticSuccess(Right(query)) + case Left(error) => ElasticFailure(error) + } } case _ => @@ -1649,14 +1684,20 @@ trait IndicesApi extends ElasticClientHelpers { case Some(where) => implicit val timestamp: Long = System.currentTimeMillis() - val search: String = + resolveDmlTemporalLiterals( SingleSearch( from = From(tables = Seq(deleteStmt.table)), where = Some(where), deleteByQuery = true + ), + "deleteByQuery" + ).map { resolved => + val search: String = resolved + logger.info( + s"✅ Converted SQL delete query to search for deleteByQuery: $search" ) - logger.info(s"✅ Converted SQL delete query to search for deleteByQuery: $search") - Right(search) + search + } } case search: SingleSearch => @@ -1672,9 +1713,12 @@ trait IndicesApi extends ElasticClientHelpers { ) else { implicit val timestamp: Long = System.currentTimeMillis() - val query: String = search.copy(deleteByQuery = true) - logger.info(s"✅ Converted SQL search query to search for deleteByQuery: $query") - Right(query) + resolveDmlTemporalLiterals(search.copy(deleteByQuery = true), "deleteByQuery").map { + resolved => + val query: String = resolved + logger.info(s"✅ Converted SQL search query to search for deleteByQuery: $query") + query + } } case _ => diff --git a/core/src/main/scala/app/softnetwork/elastic/client/SearchApi.scala b/core/src/main/scala/app/softnetwork/elastic/client/SearchApi.scala index d8f0bafac..7a26ba4d3 100644 --- a/core/src/main/scala/app/softnetwork/elastic/client/SearchApi.scala +++ b/core/src/main/scala/app/softnetwork/elastic/client/SearchApi.scala @@ -96,10 +96,14 @@ trait SearchApi extends ElasticConversion with ElasticClientHelpers { * `loadSchema` caches successes only, so a source it cannot resolve -- an ALIAS on the es8/es9 * clients (`executeGetIndex` looks the alias up as a key and finds nothing), an unknown index * without a template -- would otherwise cost one failed lookup (two round trips + WARN lines) - * per statement. [[temporalLiteralSchemaMisses]] remembers such a source for - * [[temporalLiteralSchemaMissTtlMs]] and skips the lookup; the literal is verbatim either way. + * per statement. [[temporalLiteralSchemaMisses]] remembers a 404 miss for + * [[temporalLiteralSchemaMissTtlMs]] and skips the lookup; the literal is verbatim either way. A + * NON-404 failure (a transient 5xx, a thrown lookup) is never remembered, so a cluster blip + * cannot disable the resolution for the TTL. + * + * `private[client]`: `IndicesApi` reuses it for the DELETE / UPDATE by-query search bodies. */ - protected def resolveTemporalLiterals(single: SingleSearch): ElasticResult[SingleSearch] = { + private[client] def resolveTemporalLiterals(single: SingleSearch): ElasticResult[SingleSearch] = { if (!TemporalLiterals.hasCandidates(single)) return ElasticResult.success(single) single.sources.distinct match { case Seq(source) if !source.contains("*") && !source.contains(",") => @@ -129,13 +133,12 @@ trait SearchApi extends ElasticConversion with ElasticClientHelpers { ) } case Success(ElasticFailure(error)) => - temporalLiteralSchemaMisses.put(source, System.currentTimeMillis()) + if (error.statusCode.contains(404)) rememberTemporalLiteralSchemaMiss(source) logger.debug( s"Schema of '$source' unavailable (${error.message}) - temporal literals forwarded verbatim" ) ElasticResult.success(single) case Failure(e) => - temporalLiteralSchemaMisses.put(source, System.currentTimeMillis()) logger.debug( s"Schema lookup for '$source' failed with ${e.getClass.getName} - temporal literals forwarded verbatim" ) @@ -147,22 +150,45 @@ trait SearchApi extends ElasticConversion with ElasticClientHelpers { } } - /** Sources whose schema could not be loaded, with the time of the miss (see - * [[resolveTemporalLiterals]]). Per client instance, like the schema cache it shadows. + /** Sources whose schema answered 404, with the time of the miss (see + * [[resolveTemporalLiterals]]). Per client instance, like the schema cache it shadows. Keys are + * caller-supplied FROM names, so the map is bounded the way #238's `shardCountCache` is: above + * [[temporalLiteralSchemaMissPurgeThreshold]] entries an insert also purges the expired ones, + * and if it is STILL above four times the threshold (a client probing thousands of distinct + * unknown names inside one TTL) it is cleared -- the worst case is then today's cost, one failed + * lookup per statement, never unbounded memory. */ private val temporalLiteralSchemaMisses = new java.util.concurrent.ConcurrentHashMap[String, java.lang.Long]() + private val temporalLiteralSchemaMissPurgeThreshold = 256 + /** How long a failed schema lookup is remembered -- the schema cache's own default TTL. */ protected def temporalLiteralSchemaMissTtlMs: Long = 5 * 60 * 1000L + /** Current size of the negative cache (tests). */ + private[client] def temporalLiteralSchemaMissCount: Int = temporalLiteralSchemaMisses.size() + private def temporalLiteralSchemaMissed(source: String): Boolean = Option(temporalLiteralSchemaMisses.get(source)).exists { missedAt => System.currentTimeMillis() - missedAt < temporalLiteralSchemaMissTtlMs } + private def rememberTemporalLiteralSchemaMiss(source: String): Unit = { + val now = System.currentTimeMillis() + temporalLiteralSchemaMisses.put(source, now) + if (temporalLiteralSchemaMisses.size() > temporalLiteralSchemaMissPurgeThreshold) { + val ttl = temporalLiteralSchemaMissTtlMs + temporalLiteralSchemaMisses + .entrySet() + .removeIf((e: java.util.Map.Entry[String, java.lang.Long]) => now - e.getValue >= ttl) + if (temporalLiteralSchemaMisses.size() > temporalLiteralSchemaMissPurgeThreshold * 4) + temporalLiteralSchemaMisses.clear() + } + } + /** [[resolveTemporalLiterals]] over every request of a `UNION ALL`; the first rejection wins. */ - protected def resolveTemporalLiterals(multiple: MultiSearch): ElasticResult[MultiSearch] = { + private[client] def resolveTemporalLiterals(multiple: MultiSearch): ElasticResult[MultiSearch] = { val zero: ElasticResult[Seq[SingleSearch]] = ElasticResult.success(Seq.empty) multiple.requests.foldLeft(zero) { case (ElasticSuccess(acc), request) => diff --git a/core/src/test/scala/app/softnetwork/elastic/client/TemporalLiteralSearchSpec.scala b/core/src/test/scala/app/softnetwork/elastic/client/TemporalLiteralSearchSpec.scala index c692f6eb2..d2f6e1857 100644 --- a/core/src/test/scala/app/softnetwork/elastic/client/TemporalLiteralSearchSpec.scala +++ b/core/src/test/scala/app/softnetwork/elastic/client/TemporalLiteralSearchSpec.scala @@ -157,6 +157,49 @@ class TemporalLiteralSearchSpec extends AnyFlatSpec with Matchers with BeforeAnd client.lastQuery.getOrElse(fail("no query was rendered")).query should include(isoForm) } + it should "remember only 404 misses, never a transient failure" in { + val client = new RecordingClient { + override def loadSchema(index: String): ElasticResult[Schema] = + if (index.startsWith("flaky")) { + schemaLookups += 1 + ElasticResult.failure( + ElasticError( + message = "cluster hiccup", + statusCode = Some(503), + index = Some(index), + operation = Some("loadSchema") + ) + ) + } else super.loadSchema(index) + } + val statement = + SelectStatement(s"SELECT id FROM flaky_events WHERE event_ts >= '$spaceForm' LIMIT 5") + client.search(statement) + client.search(statement) + client.schemaLookups shouldBe 2 // a 503 is retried on the next statement + client.temporalLiteralSchemaMissCount shouldBe 0 + client.lastQuery.getOrElse(fail("no query was rendered")).query should include(spaceForm) + } + + it should "bound the negative cache: purge expired misses above the threshold, clear it above the cap" in { + val expiring = new RecordingClient + expiring.missTtlMs = 0L // every miss is expired at once + (1 to 300).foreach { i => + expiring.search( + SelectStatement(s"SELECT id FROM unknown_$i WHERE event_ts >= '$spaceForm' LIMIT 5") + ) + } + expiring.temporalLiteralSchemaMissCount should be < 257 // the purge ran at least once + + val flooding = new RecordingClient // default TTL: nothing expires, only the cap can act + (1 to 1100).foreach { i => + flooding.search( + SelectStatement(s"SELECT id FROM probe_$i WHERE event_ts >= '$spaceForm' LIMIT 5") + ) + } + flooding.temporalLiteralSchemaMissCount should be <= 1024 + } + it should "not look the schema up at all when the WHERE carries no candidate literal" in { val client = seeded() client.search(SelectStatement("SELECT id FROM events WHERE amount > 10 LIMIT 5")) diff --git a/documentation/sql/dql_statements.md b/documentation/sql/dql_statements.md index b4f9a178e..8e51908a7 100644 --- a/documentation/sql/dql_statements.md +++ b/documentation/sql/dql_statements.md @@ -298,21 +298,29 @@ WHERE event_ts >= '2026-06-04 00:00:00' WHERE event_ts >= '2026-06-04T00:00:00' -- what Elasticsearch's default format accepts ``` -- Under the default format (`strict_date_optional_time||epoch_millis`) the space separator is - rewritten to `T`; fraction digits and a trailing zone offset are preserved. ISO literals, - date-only literals, epoch milliseconds and date math (`now-1d/d`) are forwarded verbatim. +- Under a format that accepts ISO dates (the default `strict_date_optional_time||epoch_millis`, + `date_optional_time`, `strict_date_optional_time_nanos`) the space separator is rewritten to `T`; + fraction digits and a trailing zone are preserved. ISO literals, date-only literals, epoch + numbers and date math (`now-1d/d`, `2026-06-04||/M` -- whose date part is normalised the same way) + are forwarded verbatim. - A column with a custom `format` (for example `yyyy-MM-dd HH:mm:ss`) keeps working as before: a literal its format already parses is never rewritten. -- A literal that cannot be a date under the default format fails with an error naming the literal - and the field (HTTP 400) instead of a raw Elasticsearch `search_phase_execution_exception`. +- Under the default (strict) format a literal that cannot be a date at all (`'not-a-date'`) or + carries an invalid calendar or time value (`'2026-02-30'`, `'2026-06-04 24:00:00'`) fails with + an error naming the literal and the field (HTTP 400) instead of a raw Elasticsearch + `search_phase_execution_exception`. A literal that starts like a date but has a shape the + resolver does not model (a zone id, a signed year, `2026-6-4`) is forwarded verbatim and + Elasticsearch decides; under `date_optional_time` or a custom format nothing is ever rejected. +- The same resolution applies to the `WHERE` clause of `UPDATE` and `DELETE`. - `keyword`/`text` columns, `LIKE`/`RLIKE` patterns, function-wrapped columns (`YEAR(event_ts)`), - `date_nanos` columns and `HAVING` conditions are never touched. + `date_nanos` columns, columns qualified with a `JOIN` alias (the FROM table's own columns are + resolved) and `HAVING` conditions are never touched. - The resolution needs the index mapping, loaded through the schema cache (one lookup per index - every 5 minutes, and only for statements whose `WHERE` compares a string literal to a column). - When the statement reads several indices or a wildcard, joins another index (`JOIN` sources keep - their literals), or the mapping cannot be loaded (an index alias, for instance), the literal is - forwarded verbatim as in previous releases; a failed mapping lookup is remembered for 5 minutes - so it is not retried on every statement. + every 5 minutes, and only for statements whose `WHERE` compares a string literal to a column). It + does not apply when the statement reads several indices or a wildcard, or when the mapping cannot + be loaded -- in particular **through an index alias**, whose mapping the client cannot resolve + today: there the literal is forwarded verbatim as in previous releases, and a failed mapping + lookup is remembered for 5 minutes so it is not retried on every statement. --- diff --git a/es6/bridge/src/test/scala/app/softnetwork/elastic/sql/TemporalLiteralQuerySpec.scala b/es6/bridge/src/test/scala/app/softnetwork/elastic/sql/TemporalLiteralQuerySpec.scala index 73477a239..07b7de684 100644 --- a/es6/bridge/src/test/scala/app/softnetwork/elastic/sql/TemporalLiteralQuerySpec.scala +++ b/es6/bridge/src/test/scala/app/softnetwork/elastic/sql/TemporalLiteralQuerySpec.scala @@ -1,3 +1,19 @@ +/* + * Copyright 2025 SOFTNETWORK + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + package app.softnetwork.elastic.sql import app.softnetwork.elastic.sql.bridge._ diff --git a/sql/src/main/scala/app/softnetwork/elastic/sql/query/TemporalLiterals.scala b/sql/src/main/scala/app/softnetwork/elastic/sql/query/TemporalLiterals.scala index 892ef2766..b4def57bc 100644 --- a/sql/src/main/scala/app/softnetwork/elastic/sql/query/TemporalLiterals.scala +++ b/sql/src/main/scala/app/softnetwork/elastic/sql/query/TemporalLiterals.scala @@ -22,7 +22,7 @@ import app.softnetwork.elastic.sql.schema.{Column, Schema} import app.softnetwork.elastic.sql.`type`.{SQLTemporal, SQLTime, SQLType} import java.time.format.DateTimeFormatter -import java.time.{LocalDate, LocalTime, ZoneOffset} +import java.time.{LocalDate, LocalTime} import scala.util.Try import scala.util.matching.Regex @@ -46,20 +46,30 @@ import scala.util.matching.Regex * touched. * * The rule per literal, in order (see [[FieldFormat]] for the format vocabulary): - * 1. a number (`epoch_millis` / `epoch_second`) or a date-math expression (`now-1d/d`, - * `2026-06-04||/M`) is forwarded verbatim; + * 1. a number (`epoch_millis` / `epoch_second`) or `now`-anchored date math is forwarded + * verbatim; a `||` date-math literal has its HEAD normalised by this same rule and its math + * kept (`2026-06-04 10:30:15||+1d` -> `2026-06-04T10:30:15||+1d`) and is never rejected; * 1. a literal one of the mapping's CUSTOM patterns already parses is forwarded verbatim -- a * `format: "yyyy-MM-dd HH:mm:ss"` column parses the space form TODAY and must keep working; - * 1. if the mapping accepts an ISO optional-time built-in: a calendar literal (`yyyy-MM-dd`, - * optionally followed by a `T` or a SPACE, a time and a zone) is validated with `java.time`; - * the SQL space form is then rewritten with a `T` (fraction digits preserved, a lower-case - * `z` upper-cased, nothing else changes) and an already-ISO one is forwarded verbatim; other - * ISO shapes (`yyyy`, `yyyy-MM`, ordinal and week dates) are forwarded verbatim; - * 1. anything else -- an unrecognised shape or an invalid calendar value -- is REJECTED with a - * message naming the literal, the field and the format, but only when every alternative of - * the format is one this object fully emulates (the ISO optional-time and epoch names). With - * a custom or unrecognised built-in in the list we cannot prove Elasticsearch rejects the - * literal, so it is forwarded verbatim. + * 1. if the mapping accepts an ISO optional-time built-in: a literal of the RECOGNISED calendar + * shape -- `yyyy-MM-dd`, optionally followed by a `T` or ONE SPACE, a time `HH[:mm[:ss[.f]]]` + * and a zone (`Z`, an offset or a zone id) -- has its date and time validated with + * `java.time`; the space form is then rewritten with a `T` (fraction digits and zone kept, a + * lower-case `z` upper-cased); an ISO form is forwarded verbatim. A literal that merely + * STARTS like an ISO date (a signed or 5-digit year, `2026-6-4`, `T1:02:03`, an ordinal or + * week date, a tail this recogniser does not model) is forwarded verbatim: Elasticsearch + * decides; + * 1. a literal is REJECTED -- with a message naming the literal, the field and the format -- + * only when it cannot be an ISO date at all (it does not even start with a year) or carries + * an INVALID recognised calendar/time component (`2026-02-30`, `T24:00:00`), and only when + * every alternative of the format is one whose grammar this recogniser approximates closely + * enough: the STRICT ISO optional-time names and the epoch names. Under the non-strict + * `date_optional_time`, a custom pattern or an unrecognised built-in nothing is rejected. + * + * The accept set was measured against the real Elasticsearch parsers of 6.8.23, 7.17.29, 8.18.3 + * and 9.0.3 (`DateFormatter.forPattern(spec).toDateMathParser()`): every REJECT above fails on all + * four, every VERBATIM is a no-op, every rewritten value is accepted on all four (a zone id after + * the time is 7+ only -- as it is when the user types the `T` form). * * The schema-absent path (no schema attached, statement over several indices, wildcard source, * schema lookup failure) is the CALLER's decision and means "forward verbatim" -- this object @@ -73,10 +83,19 @@ object TemporalLiterals { /** Longest literal echoed back in a rejection message; longer ones are cut head + tail. */ val MaxLiteralExcerpt: Int = 120 - /** The ISO optional-time built-ins: date mandatory, `T`-separated time optional. */ + /** The ISO optional-time built-ins: date mandatory, `T`-separated time optional. The space-form + * rewrite applies under any of them (the lenient parser accepts the `T` form too -- measured). + */ private val IsoOptionalTimeFormats: Set[String] = Set("strict_date_optional_time", "date_optional_time", "strict_date_optional_time_nanos") + /** The STRICT names -- the only ones whose grammar the recogniser below approximates closely + * enough to REJECT a literal. The non-strict `date_optional_time` also takes `2026-6-4`, + * `2026-06-04T1:02:03` and `12026-06-04` (measured on ES 6.8 / 7.17 / 8.18 / 9.0). + */ + private val StrictIsoFormats: Set[String] = + Set("strict_date_optional_time", "strict_date_optional_time_nanos") + private val EpochFormats: Set[String] = Set("epoch_millis", "epoch_second") /** A built-in format NAME (`basic_date_time`, `date_hour_minute`, ...) as opposed to a custom @@ -84,26 +103,26 @@ object TemporalLiterals { */ private val BuiltInName: Regex = "^[a-z][a-z0-9_]*$".r - private val NumericLiteral: Regex = "^-?\\d+(?:\\.\\d+)?$".r + /** A number, read leniently: ES 6.8's Joda parser also takes `+1` and `1e3`. */ + private val NumericLiteral: Regex = "^[+-]?\\d+(?:\\.\\d+)?(?:[eE][+-]?\\d+)?$".r /** Elasticsearch date math anchored on `now`: `now`, `now-1d`, `now+1h/h`, `now/d`. */ private val NowDateMath: Regex = "^now(?:[+-]\\d+[yMwdhHms]|/[yMwdhHms])*$".r - // `strict_date_optional_time` grammar, read as a lenient SUPERSET so that a spelling - // Elasticsearch accepts is never rejected here. + /** Starts like an ISO date: an optionally signed year of at least four digits. */ + private val DateLikeStart: Regex = "^[+-]?\\d{4}".r + private val IsoTime = "\\d{2}(?::\\d{2}(?::\\d{2}(?:[.,]\\d{1,9})?)?)?" - private val IsoZone = "(?:[Zz]|[+-]\\d{2}(?::?\\d{2})?)?" - /** A calendar date, optionally followed by a `T` OR a SPACE, a time and a zone. */ - private val CalendarLiteral: Regex = - ("^(\\d{4}-\\d{2}-\\d{2})(?:([ T])(" + IsoTime + ")(" + IsoZone + "))?$").r + /** `Z`, an offset (`+01`, `+0100`, `+01:00`, `+01:00:00`) or a zone id (`UTC`, `Europe/Paris`). + */ + private val IsoZone = "Z|z|[+-]\\d{2}(?::?\\d{2}(?::?\\d{2})?)?|[A-Za-z][A-Za-z0-9_+\\-/]*" - /** The other shapes the ISO optional-time parsers accept (year, year-month, the ordinal - * `yyyy-DDD` and week `yyyy-Www[-e]` dates of the Joda parser behind ES 6.8, a date followed - * directly by a zone) -- forwarded verbatim, not validated. + /** The RECOGNISED calendar shape: a strict `yyyy-MM-dd`, optionally followed by a `T` or ONE + * space, an optional time and an optional zone. Groups: date, separator, time, zone. */ - private val OtherIsoLiteral: Regex = - ("^(?:\\d{4}(?:-\\d{2})?|\\d{4}-\\d{3}|\\d{4}-W\\d{2}(?:-\\d)?|\\d{4}-\\d{2}-\\d{2})" + IsoZone + "$").r + private val CalendarLiteral: Regex = + ("^(\\d{4}-\\d{2}-\\d{2})(?:([ T])(" + IsoTime + ")?(" + IsoZone + ")?)?$").r /** A `date` column's effective mapping `format`, split on `||`. */ final case class FieldFormat(spec: String) { @@ -113,16 +132,18 @@ object TemporalLiterals { private def isIso(alternative: String): Boolean = IsoOptionalTimeFormats.contains(alternative) + private def isStrictIso(alternative: String): Boolean = StrictIsoFormats.contains(alternative) + private def isEpoch(alternative: String): Boolean = EpochFormats.contains(alternative) /** At least one alternative accepts the `T`-separated ISO form we normalise to. */ val acceptsIsoOptionalTime: Boolean = alternatives.exists(isIso) - /** Every alternative is one this object can emulate exactly -- the ONLY case in which a literal - * may be rejected rather than forwarded verbatim. + /** Every alternative is a STRICT ISO optional-time or an epoch name -- the ONLY case in which a + * literal may be rejected rather than forwarded verbatim. */ val fullyUnderstood: Boolean = - alternatives.nonEmpty && alternatives.forall(a => isIso(a) || isEpoch(a)) + alternatives.nonEmpty && alternatives.forall(a => isStrictIso(a) || isEpoch(a)) /** The alternatives that are custom `DateTimeFormatter` patterns (not built-in names). */ val customPatterns: Seq[String] = @@ -155,9 +176,7 @@ object TemporalLiterals { } } - /** `now`-anchored or `||`-suffixed date math is resolved by Elasticsearch itself. The `||` form - * is accepted leniently (its date part is parsed by the field's format, whatever that is). - */ + /** `now`-anchored or `||`-suffixed date math is resolved by Elasticsearch itself. */ def isDateMath(literal: String): Boolean = NowDateMath.pattern.matcher(literal).matches() || literal.contains("||") @@ -172,22 +191,51 @@ object TemporalLiterals { field: String, format: FieldFormat ): Either[String, Option[String]] = { - if (NumericLiteral.pattern.matcher(literal).matches() || isDateMath(literal)) Right(None) - else if (format.acceptsAsCustom(literal)) Right(None) - else if (!format.acceptsIsoOptionalTime) Right(None) + if ( + NumericLiteral.pattern.matcher(literal).matches() || + NowDateMath.pattern.matcher(literal).matches() + ) Right(None) + else { + val math = literal.indexOf("||") + if (math >= 0) { + // Date math anchored on a date: normalise the HEAD by the same rule, keep the math, and + // never reject -- the math tail is Elasticsearch's to judge. + normalizeLiteral(literal.substring(0, math), field, format) match { + case Right(Some(head)) => Right(Some(head + literal.substring(math))) + case _ => Right(None) + } + } else if (format.acceptsAsCustom(literal)) Right(None) + else if (!format.acceptsIsoOptionalTime) Right(None) + else + literal match { + case CalendarLiteral(date, separator, time, zone) => + calendar(literal, date, Option(separator), Option(time), Option(zone), field, format) + case _ if DateLikeStart.findPrefixOf(literal).isDefined => + // Starts like an ISO date but carries a shape this recogniser does not model: let + // Elasticsearch decide, never reject. + Right(None) + case _ => rejectOrForward(literal, field, format) + } + } + } + + private def calendar( + literal: String, + date: String, + separator: Option[String], + time: Option[String], + zone: Option[String], + field: String, + format: FieldFormat + ): Either[String, Option[String]] = + if (!validDate(date) || !time.forall(validTime)) rejectOrForward(literal, field, format) else - literal match { - case CalendarLiteral(date, null, _, _) => - if (validDate(date)) Right(None) else rejectOrForward(literal, field, format) - case CalendarLiteral(date, separator, time, zone) => - if (validDate(date) && validTime(time) && validZone(zone)) { - if (separator == " ") Right(Some(date + "T" + time + zone.toUpperCase)) - else Right(None) - } else rejectOrForward(literal, field, format) - case _ if OtherIsoLiteral.pattern.matcher(literal).matches() => Right(None) - case _ => rejectOrForward(literal, field, format) + (separator, time) match { + case (Some(" "), Some(t)) => + val z = zone.map(z => if (z == "z") "Z" else z).getOrElse("") + Right(Some(date + "T" + t + z)) + case _ => Right(None) } - } private def rejectOrForward( literal: String, @@ -204,9 +252,6 @@ object TemporalLiterals { Try(LocalTime.parse(parseableTime)).isSuccess } - private def validZone(zone: String): Boolean = - zone.isEmpty || Try(ZoneOffset.of(zone.toUpperCase)).isSuccess - /** The literal as echoed in a rejection: control characters and line separators collapsed to a * space (the message reaches a log record, a `SQLException` and a terminal), the length bounded * head + tail so a hostile or accidental multi-kilobyte literal cannot flood either. diff --git a/sql/src/test/scala/app/softnetwork/elastic/sql/query/TemporalLiteralsSpec.scala b/sql/src/test/scala/app/softnetwork/elastic/sql/query/TemporalLiteralsSpec.scala index eb04cce56..c01bd2841 100644 --- a/sql/src/test/scala/app/softnetwork/elastic/sql/query/TemporalLiteralsSpec.scala +++ b/sql/src/test/scala/app/softnetwork/elastic/sql/query/TemporalLiteralsSpec.scala @@ -1,3 +1,19 @@ +/* + * Copyright 2025 SOFTNETWORK + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + package app.softnetwork.elastic.sql.query import app.softnetwork.elastic.schema.Index @@ -15,6 +31,12 @@ import scala.collection.immutable.ListMap * columns. The bridge emission is asserted in `TemporalLiteralQuerySpec` (bridge module) and the * core seam in `TemporalLiteralSearchSpec` (core module); this spec pins the RULES. */ +/** What the oracle expects of a literal: forwarded, rejected, or rewritten to `to`. */ +sealed trait Expected +case object Verbatim extends Expected +case object Reject extends Expected +final case class Rewrite(to: String) extends Expected + class TemporalLiteralsSpec extends AnyFlatSpec with Matchers { private val default = FieldFormat(TemporalLiterals.DefaultDateFormat) @@ -62,6 +84,12 @@ class TemporalLiteralsSpec extends AnyFlatSpec with Matchers { nanos.fullyUnderstood shouldBe true } + it should "accept the ISO form under the non-strict date_optional_time but never reject under it" in { + val lenient = FieldFormat("date_optional_time") + lenient.acceptsIsoOptionalTime shouldBe true + lenient.fullyUnderstood shouldBe false + } + it should "strip the Elasticsearch 7 java-time '8' prefix from a custom pattern" in { FieldFormat("8yyyy-MM-dd").acceptsAsCustom("2026-06-04") shouldBe true } @@ -77,76 +105,157 @@ class TemporalLiteralsSpec extends AnyFlatSpec with Matchers { ) shouldBe custom } - // ---- normalizeLiteral ------------------------------------------------------------------------ - - "normalizeLiteral" should "rewrite the SQL space form to the T form under the default format" in { - norm("2026-06-04 00:00:00.000000") shouldBe Right(Some("2026-06-04T00:00:00.000000")) - norm("2026-06-04 00:00:00") shouldBe Right(Some("2026-06-04T00:00:00")) - norm("2026-06-04 00:00") shouldBe Right(Some("2026-06-04T00:00")) - norm("2026-06-04 10") shouldBe Right(Some("2026-06-04T10")) - norm("2026-06-04 00:00:00.123+02:00") shouldBe Right(Some("2026-06-04T00:00:00.123+02:00")) - norm("2026-06-04 00:00:00+0200") shouldBe Right(Some("2026-06-04T00:00:00+0200")) - norm("2026-06-04 00:00:00Z") shouldBe Right(Some("2026-06-04T00:00:00Z")) - norm("2026-06-04 00:00:00z") shouldBe Right(Some("2026-06-04T00:00:00Z")) // zone upper-cased - norm("2026-06-04 23:59:59,5") shouldBe Right(Some("2026-06-04T23:59:59,5")) - norm("2026-06-04 00:00:00.123456789") shouldBe Right(Some("2026-06-04T00:00:00.123456789")) + // ---- normalizeLiteral: the four-major oracle ------------------------------------------------- + // + // Every row below was checked against the REAL Elasticsearch parsers of 6.8.23 (Joda), 7.17.29, + // 8.18.3 and 9.0.3 (`DateFormatter.forPattern(spec).toDateMathParser().parse(...)`, the parser a + // range/term value goes through). Contract: a `Reject` row FAILS on all four; a `Rewrite` row + // produces a value every major accepts (a zone id after the time is 7+ only -- exactly as when + // the user types the `T` form); a `Verbatim` row is a no-op, whatever Elasticsearch then says. + // The rule may REJECT only what cannot be an ISO date at all or carries an invalid recognised + // calendar/time component -- never a shape it merely does not model. + + private def check(format: FieldFormat, oracle: Seq[(String, Expected)]): Unit = + oracle.foreach { case (literal, expected) => + withClue(s"'$literal' under '${format.spec}': ") { + (norm(literal, format), expected) match { + case (Right(None), Verbatim) => succeed + case (Right(Some(v)), Rewrite(to)) => v shouldBe to + case (Left(reason), Reject) => + reason should include(s"'$literal'") + reason should include(s"'$field'") + reason should include(format.spec) + reason should not startWith "Internal parser error" + case (actual, _) => fail(s"expected $expected, got $actual") + } + } + } + + private val strictOracle: Seq[(String, Expected)] = Seq( + // -- the SQL space form is rewritten to the T form (fraction and zone kept, `z` upper-cased) + "2026-06-04 10:30:15" -> Rewrite("2026-06-04T10:30:15"), + "2026-06-04 10:30:15.000000" -> Rewrite("2026-06-04T10:30:15.000000"), + "2026-06-04 00:00" -> Rewrite("2026-06-04T00:00"), + "2026-06-04 10" -> Rewrite("2026-06-04T10"), + "2026-06-04 10:30:15.123+02:00" -> Rewrite("2026-06-04T10:30:15.123+02:00"), + "2026-06-04 10:30:15+0100" -> Rewrite("2026-06-04T10:30:15+0100"), + "2026-06-04 10:30:15+01" -> Rewrite("2026-06-04T10:30:15+01"), + "2026-06-04 10:30:15+01:00:00" -> Rewrite("2026-06-04T10:30:15+01:00:00"), + "2026-06-04 10:30:15Z" -> Rewrite("2026-06-04T10:30:15Z"), + "2026-06-04 10:30:15z" -> Rewrite("2026-06-04T10:30:15Z"), + "2026-06-04 23:59:59,5" -> Rewrite("2026-06-04T23:59:59,5"), + "2026-06-04 10:30:15.123456789" -> Rewrite("2026-06-04T10:30:15.123456789"), + "2026-06-04 10:30:15UTC" -> Rewrite("2026-06-04T10:30:15UTC"), + "2026-06-04 10:30:15||+1d" -> Rewrite("2026-06-04T10:30:15||+1d"), + // -- ISO forms Elasticsearch accepts: verbatim + "2026-06-04" -> Verbatim, + "2026-06" -> Verbatim, + "2026" -> Verbatim, + "2026-06-04T" -> Verbatim, + "2026-06-04T10" -> Verbatim, + "2026-06-04T10:30" -> Verbatim, + "2026-06-04T10:30:15" -> Verbatim, + "2026-06-04T10:30:15.1" -> Verbatim, + "2026-06-04T10:30:15.123456789" -> Verbatim, + "2026-06-04T10:30:15,5" -> Verbatim, + "2026-06-04T10:30:15Z" -> Verbatim, + "2026-06-04T10:30:15z" -> Verbatim, // 6.8 only -- Elasticsearch decides + "2026-06-04T10:30:15+01:00" -> Verbatim, + "2026-06-04T10:30:15+0100" -> Verbatim, + "2026-06-04T10:30:15+01" -> Verbatim, + "2026-06-04T10:30:15-05:30" -> Verbatim, + "2026-06-04T10:30:15+01:00:00" -> Verbatim, // offset with seconds + "2026-06-04T10:30:15UTC" -> Verbatim, // zone id, 7+ only + "2026-06-04T10:30:15Europe/Paris" -> Verbatim, // region id, 7+ only + "-0001-06-04" -> Verbatim, // negative year + // -- starts like a date but carries a shape the recogniser does not model: verbatim + "2026-155" -> Verbatim, // ordinal date, 6.8 only + "2026-W23-4" -> Verbatim, // week date, 6.8 only + "+12026-06-04" -> Verbatim, + "12026-06-04" -> Verbatim, + "2026-6-4" -> Verbatim, + "2026-06-04T1:02:03" -> Verbatim, + "2026-06-04t10:30:15" -> Verbatim, // lower-case t, 6.8 only + "2026-06-04Z" -> Verbatim, + "2026-06-04+01:00" -> Verbatim, + "2026-06-04 10:30:15 UTC" -> Verbatim, // a space before the zone: not ours + "2026-06-04 10:30:15" -> Verbatim, // two spaces: not ours + "2026-06-04T10:30:15.1234567890" -> Verbatim, + // -- numbers and date math: verbatim + "1780531200000" -> Verbatim, + "1780531200" -> Verbatim, + "1.5" -> Verbatim, + "1780531200000.5" -> Verbatim, + "-1" -> Verbatim, + "+1" -> Verbatim, // 6.8 only + "1e3" -> Verbatim, // 6.8 only + "now" -> Verbatim, + "now-1d/d" -> Verbatim, + "now+1h" -> Verbatim, + "now+1M/M" -> Verbatim, + "now/d" -> Verbatim, + "2026-06-04||/d" -> Verbatim, + // -- rejected: fails on every major, and the message names the literal and the field + "" -> Reject, + "not-a-date" -> Reject, + "nowhere" -> Reject, + "NOW-1d" -> Reject, // date math is lower-case + "now-1D" -> Reject, // no such unit + "04/06/2026" -> Reject, + "2026-02-30" -> Reject, + "2026-06-04T24:00:00" -> Reject, + "2026-13-45T00:00:00" -> Reject, + "2026-06-04 24:00:00" -> Reject, + "2026-13-45 99:99:99" -> Reject + ) + + "normalizeLiteral" should "agree with the four Elasticsearch parsers under the default format" in { + check(default, strictOracle) } - it should "leave every ISO spelling untouched" in { - Seq( - "2026-06-04T00:00:00", - "2026-06-04T00:00:00.000000", - "2026-06-04T00:00:00Z", - "2026-06-04T10:30:15.123456789+01:00", - "2026-06-04T10", - "2026-06-04", - "2026-06", - "2026-155", - "2026-W23-4" - ).foreach(literal => withClue(literal)(norm(literal) shouldBe Right(None))) + it should "behave the same under strict_date_optional_time_nanos" in { + check(nanos, strictOracle) } - it should "leave epoch numbers and date math untouched" in { - Seq( - "1780531200000", - "-1", - "1780531200", - "1780531200000.5", - "now", - "now-1d/d", - "now+1h", - "now+1M/M", - "now/d", - "2026-06-04||/M", - "2026-06-04 00:00:00||+1d" - ).foreach(literal => withClue(literal)(norm(literal) shouldBe Right(None))) + it should "never reject under the non-strict date_optional_time, but still fix the space form" in { + check( + FieldFormat("date_optional_time"), + Seq( + "2026-06-04 10:30:15" -> Rewrite("2026-06-04T10:30:15"), + "2026-06-04 00:00:00" -> Rewrite("2026-06-04T00:00:00"), + "2026-6-4" -> Verbatim, // accepted by the lenient parser on every major + "2026-06-04T1:02:03" -> Verbatim, + "12026-06-04" -> Verbatim, + "-0001-06-04" -> Verbatim, + "2026-06-04T10:30:15" -> Verbatim, + "not-a-date" -> Verbatim, // not fully understood => Elasticsearch decides + "2026-02-30" -> Verbatim, + "" -> Verbatim + ) + ) } - it should "reject an unparseable literal under the default format, naming the literal and the field" in { - Seq( - "not-a-date", - "nowhere", // starts with `now` but is not date math - "NOW-1d", // Elasticsearch date math is lower-case - "2026-13-45 99:99:99", - "2026-06-04 24:00:00", - "2026-13-45T00:00:00", // T form with an invalid calendar value - "2026-02-30", // date-only with an invalid calendar value - "", - "04/06/2026", - "2026-06-04 00:00:00 UTC", - "2026-06-04 00:00:00" // two spaces - ).foreach { literal => - norm(literal) match { - case Left(reason) => - withClue(literal) { - reason should include(s"'$literal'") - reason should include(s"'$field'") - reason should include(TemporalLiterals.DefaultDateFormat) - reason should not startWith "Internal parser error" - } - case other => fail(s"'$literal' should be rejected, got $other") - } - } + it should "never reject under a custom or opaque format" in { + check( + custom, + Seq( + "not-a-date" -> Verbatim, + "2026-06-04 00:00:00" -> Verbatim, // parity: the custom pattern parses it + "2026-06-04T00:00:00" -> Verbatim // not ours to fix: no ISO alternative + ) + ) + check(opaque, Seq("not-a-date" -> Verbatim, "2026-06-04 00:00:00" -> Verbatim)) + } + + it should "prefer parity with a custom alternative, and still fix the space form where ISO is accepted" in { + check( + mixed, + Seq( + "2026-06-04 00:00:00" -> Verbatim, // the custom pattern parses it + "2026-06-04 00:00" -> Rewrite("2026-06-04T00:00"), // custom needs seconds + "garbage" -> Verbatim // not fully understood => never rejected + ) + ) } it should "bound and sanitise the literal echoed in the rejection" in { @@ -169,26 +278,6 @@ class TemporalLiteralsSpec extends AnyFlatSpec with Matchers { TemporalLiterals.excerpt("short") shouldBe "short" } - it should "never reject under a custom or opaque format" in { - norm("not-a-date", custom) shouldBe Right(None) - norm("2026-06-04 00:00:00", custom) shouldBe Right(None) // parity: the custom pattern parses it - norm("2026-06-04T00:00:00", custom) shouldBe Right(None) // not ours to fix: no ISO alternative - norm("not-a-date", opaque) shouldBe Right(None) - norm("2026-06-04 00:00:00", opaque) shouldBe Right(None) - } - - it should "prefer parity with a custom alternative, and still fix the space form where ISO is accepted" in { - norm("2026-06-04 00:00:00", mixed) shouldBe Right(None) // the custom pattern parses it - norm("2026-06-04 00:00", mixed) shouldBe Right(Some("2026-06-04T00:00")) // custom needs seconds - norm("garbage", mixed) shouldBe Right(None) // not fully understood => never rejected - } - - it should "rewrite the space form under the date_nanos default format too" in { - norm("2026-06-04 00:00:00.123456789", nanos) shouldBe Right( - Some("2026-06-04T00:00:00.123456789") - ) - } - // ---- statement level --------------------------------------------------------------------------- private val schema: SchemaTable = SchemaTable( diff --git a/testkit/src/main/scala/app/softnetwork/elastic/client/TemporalLiteralSpec.scala b/testkit/src/main/scala/app/softnetwork/elastic/client/TemporalLiteralSpec.scala index 58c6334a0..e74f3046e 100644 --- a/testkit/src/main/scala/app/softnetwork/elastic/client/TemporalLiteralSpec.scala +++ b/testkit/src/main/scala/app/softnetwork/elastic/client/TemporalLiteralSpec.scala @@ -61,6 +61,9 @@ trait TemporalLiteralSpec extends AnyFlatSpecLike with ElasticDockerTestKit with private val customIndex = "temporal_literal_custom" + /** Mutated by the DML cases (UPDATE then DELETE), so it gets its own copy of the rows. */ + private val dmlIndex = "temporal_literal_dml" + /** Eight timestamps (UTC), ids `e1`..`e8`; `>= 2026-06-04T00:00:00` selects `e4`..`e8`. */ private val timestamps: Seq[String] = Seq( "2026-06-01T00:00:00", @@ -103,6 +106,8 @@ trait TemporalLiteralSpec extends AnyFlatSpecLike with ElasticDockerTestKit with client.setMapping(defaultIndex, defaultMapping).get shouldBe true client.createIndex(customIndex, settings = settings).get shouldBe true client.setMapping(customIndex, customMapping).get shouldBe true + client.createIndex(dmlIndex, settings = settings).get shouldBe true + client.setMapping(dmlIndex, defaultMapping).get shouldBe true val defaultDocs = timestamps.zipWithIndex.map { case (ts, i) => // `label` holds the SQL spelling of the same instant: the keyword negative control @@ -129,11 +134,13 @@ trait TemporalLiteralSpec extends AnyFlatSpecLike with ElasticDockerTestKit with load(defaultIndex, defaultDocs) load(customIndex, customDocs) + load(dmlIndex, defaultDocs) } override def afterAll(): Unit = { client.deleteIndex(defaultIndex) client.deleteIndex(customIndex) + client.deleteIndex(dmlIndex) super.afterAll() } @@ -220,6 +227,24 @@ trait TemporalLiteralSpec extends AnyFlatSpecLike with ElasticDockerTestKit with fromJune4 } + // ---- DML: the same WHERE, the same rows (review R4-4) -------------------------------------------- + + private def runDml(sql: String): Unit = + Await.result(client.run(sql), 60.seconds) match { + case ElasticSuccess(_) => client.refresh(dmlIndex) + case ElasticFailure(error) => fail(s"DML failed: ${error.message}\n$sql") + } + + "UPDATE with a space-form date literal" should "update the rows the equivalent SELECT matches" in { + runDml(s"UPDATE $dmlIndex SET amount = 100 WHERE event_ts >= '2026-06-04 00:00:00'") + searchIds(s"SELECT id FROM $dmlIndex WHERE amount = 100") shouldBe fromJune4 + } + + "DELETE with a space-form date literal" should "delete the rows the equivalent SELECT matches" in { + runDml(s"DELETE FROM $dmlIndex WHERE event_ts < '2026-06-04 00:00:00'") + searchIds(s"SELECT id FROM $dmlIndex") shouldBe fromJune4 + } + // ---- AC 6: unparseable literal is a named 400, not a raw shard failure ------------------------- "an unparseable literal against a default-format date field" should "fail naming the literal and the field" in { From a8b8453e5be3c0b3f420bf15a20ce95897fbf76c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Manciot?= Date: Sun, 6 Sep 2026 11:23:28 +0200 Subject: [PATCH 3/5] chore(sql): second-look nits for #276 - exact zone grammar in the rewrite, test ADT nested Second look on 6eeffebe approved; three LOW nits landed here. - R4-13: the rewrite's zone grammar is exactly the four offset shapes every Elasticsearch major accepts after a time (`+01`, `+0100`, `+01:00`, `+01:00:00`) plus `Z`; zone ids (`UTC`, `GMT+1`) and other offset shapes (`+010000`, `+0100:00`, `+01:0000`), which some or all majors reject, are never produced - their inputs stay verbatim. Five oracle rows added; the "every rewrite is accepted on all four" claim now states exactly what the rewrite can produce. - R4-16: the test-scope Expected / Verbatim / Reject / Rewrite types are nested in the spec's companion object instead of being public types of package sql.query. - R4-15: local deferred-work record updated (DML item fixed in 6eeffebe) - not a tracked file. Story BIDC-4 Closes #276 Co-Authored-By: Claude Fable 5.1 --- .../elastic/sql/query/TemporalLiterals.scala | 26 ++++++++++++------- .../sql/query/TemporalLiteralsSpec.scala | 22 +++++++++++----- 2 files changed, 32 insertions(+), 16 deletions(-) diff --git a/sql/src/main/scala/app/softnetwork/elastic/sql/query/TemporalLiterals.scala b/sql/src/main/scala/app/softnetwork/elastic/sql/query/TemporalLiterals.scala index b4def57bc..907a2da4d 100644 --- a/sql/src/main/scala/app/softnetwork/elastic/sql/query/TemporalLiterals.scala +++ b/sql/src/main/scala/app/softnetwork/elastic/sql/query/TemporalLiterals.scala @@ -53,12 +53,13 @@ import scala.util.matching.Regex * `format: "yyyy-MM-dd HH:mm:ss"` column parses the space form TODAY and must keep working; * 1. if the mapping accepts an ISO optional-time built-in: a literal of the RECOGNISED calendar * shape -- `yyyy-MM-dd`, optionally followed by a `T` or ONE SPACE, a time `HH[:mm[:ss[.f]]]` - * and a zone (`Z`, an offset or a zone id) -- has its date and time validated with - * `java.time`; the space form is then rewritten with a `T` (fraction digits and zone kept, a - * lower-case `z` upper-cased); an ISO form is forwarded verbatim. A literal that merely - * STARTS like an ISO date (a signed or 5-digit year, `2026-6-4`, `T1:02:03`, an ordinal or - * week date, a tail this recogniser does not model) is forwarded verbatim: Elasticsearch - * decides; + * and a zone (`Z` or an offset `+01` / `+0100` / `+01:00` / `+01:00:00`) -- has its date and + * time validated with `java.time`; the space form is then rewritten with a `T` (fraction + * digits and offset kept, a lower-case `z` upper-cased); an ISO form is forwarded verbatim. A + * literal that merely STARTS like an ISO date (a signed or 5-digit year, `2026-6-4`, + * `T1:02:03`, an ordinal or week date, a zone id such as `UTC` or `GMT+1`, an offset in + * another shape, any tail this recogniser does not model) is forwarded verbatim: + * Elasticsearch decides; * 1. a literal is REJECTED -- with a message naming the literal, the field and the format -- * only when it cannot be an ISO date at all (it does not even start with a year) or carries * an INVALID recognised calendar/time component (`2026-02-30`, `T24:00:00`), and only when @@ -68,8 +69,10 @@ import scala.util.matching.Regex * * The accept set was measured against the real Elasticsearch parsers of 6.8.23, 7.17.29, 8.18.3 * and 9.0.3 (`DateFormatter.forPattern(spec).toDateMathParser()`): every REJECT above fails on all - * four, every VERBATIM is a no-op, every rewritten value is accepted on all four (a zone id after - * the time is 7+ only -- as it is when the user types the `T` form). + * four, every VERBATIM is a no-op, and every value the rewrite can produce -- a `T` form with an + * optional fraction and an optional `Z` / `+01` / `+0100` / `+01:00` / `+01:00:00` -- is accepted + * on all four. Zone shapes some majors reject (`+010000` on 7.17, `GMT+1` on 6.8/7.17, the mixed + * `+0100:00` / `+01:0000` everywhere) are never produced: their inputs are forwarded verbatim. * * The schema-absent path (no schema attached, statement over several indices, wildcard source, * schema lookup failure) is the CALLER's decision and means "forward verbatim" -- this object @@ -114,9 +117,12 @@ object TemporalLiterals { private val IsoTime = "\\d{2}(?::\\d{2}(?::\\d{2}(?:[.,]\\d{1,9})?)?)?" - /** `Z`, an offset (`+01`, `+0100`, `+01:00`, `+01:00:00`) or a zone id (`UTC`, `Europe/Paris`). + /** `Z` or an offset in one of the FOUR shapes every Elasticsearch major accepts after a time: + * `+01`, `+0100`, `+01:00`, `+01:00:00`. Anything else (`+010000`, a mixed `+0100:00`, a zone id + * such as `UTC` or `GMT+1` -- accepted only by some majors) is left to the date-like fallback, + * i.e. forwarded verbatim, never rewritten. */ - private val IsoZone = "Z|z|[+-]\\d{2}(?::?\\d{2}(?::?\\d{2})?)?|[A-Za-z][A-Za-z0-9_+\\-/]*" + private val IsoZone = "Z|z|[+-]\\d{2}(?::\\d{2}(?::\\d{2})?|\\d{2})?" /** The RECOGNISED calendar shape: a strict `yyyy-MM-dd`, optionally followed by a `T` or ONE * space, an optional time and an optional zone. Groups: date, separator, time, zone. diff --git a/sql/src/test/scala/app/softnetwork/elastic/sql/query/TemporalLiteralsSpec.scala b/sql/src/test/scala/app/softnetwork/elastic/sql/query/TemporalLiteralsSpec.scala index c01bd2841..838a8e5af 100644 --- a/sql/src/test/scala/app/softnetwork/elastic/sql/query/TemporalLiteralsSpec.scala +++ b/sql/src/test/scala/app/softnetwork/elastic/sql/query/TemporalLiteralsSpec.scala @@ -31,14 +31,19 @@ import scala.collection.immutable.ListMap * columns. The bridge emission is asserted in `TemporalLiteralQuerySpec` (bridge module) and the * core seam in `TemporalLiteralSearchSpec` (core module); this spec pins the RULES. */ -/** What the oracle expects of a literal: forwarded, rejected, or rewritten to `to`. */ -sealed trait Expected -case object Verbatim extends Expected -case object Reject extends Expected -final case class Rewrite(to: String) extends Expected +object TemporalLiteralsSpec { + + /** What the oracle expects of a literal: forwarded, rejected, or rewritten to `to`. */ + sealed trait Expected + case object Verbatim extends Expected + case object Reject extends Expected + final case class Rewrite(to: String) extends Expected +} class TemporalLiteralsSpec extends AnyFlatSpec with Matchers { + import TemporalLiteralsSpec._ + private val default = FieldFormat(TemporalLiterals.DefaultDateFormat) private val custom = FieldFormat("yyyy-MM-dd HH:mm:ss") private val mixed = FieldFormat("yyyy-MM-dd HH:mm:ss||strict_date_optional_time") @@ -145,8 +150,13 @@ class TemporalLiteralsSpec extends AnyFlatSpec with Matchers { "2026-06-04 10:30:15z" -> Rewrite("2026-06-04T10:30:15Z"), "2026-06-04 23:59:59,5" -> Rewrite("2026-06-04T23:59:59,5"), "2026-06-04 10:30:15.123456789" -> Rewrite("2026-06-04T10:30:15.123456789"), - "2026-06-04 10:30:15UTC" -> Rewrite("2026-06-04T10:30:15UTC"), "2026-06-04 10:30:15||+1d" -> Rewrite("2026-06-04T10:30:15||+1d"), + // -- zone shapes some majors reject are never produced by the rewrite: verbatim (R4-13) + "2026-06-04 10:30:15UTC" -> Verbatim, // zone id: 7+ only + "2026-06-04 10:30:15GMT+1" -> Verbatim, // 6.8 / 7.17 reject it + "2026-06-04 10:30:15+010000" -> Verbatim, // 7.17 rejects it + "2026-06-04 10:30:15+0100:00" -> Verbatim, // every major rejects it + "2026-06-04 10:30:15+01:0000" -> Verbatim, // every major rejects it // -- ISO forms Elasticsearch accepts: verbatim "2026-06-04" -> Verbatim, "2026-06" -> Verbatim, From ab251b5e69b976f48f80e1960c5b73f044dd7785 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Manciot?= Date: Sun, 6 Sep 2026 11:53:12 +0200 Subject: [PATCH 4/5] feat(core,sql): resolve a single-index alias to its concrete mapping (#276, lead ruling on R4-5) A query through an index ALIAS got no temporal-literal normalisation: es8/es9 looked the alias up as a key in the GET-index response (which Elasticsearch keys by the CONCRETE index names) and es6/es7 built an empty schema from the same body, so the mapping never resolved and the old raw `search_phase_execution_exception` surfaced on the venue the issue reports. The rule lives in ONE place, `Index.apply(name, root)`: when the response carries no entry named `name`, an alias over exactly ONE index resolves to that index's document (the schema keeps the alias as its name); `Index.indexDocuments` exposes the members. `IndicesApi.loadIndexAsSchema` reports an alias over SEVERAL indices as not found rather than as an empty schema - uniform on every client - and es8/es9 `executeGetIndex` now hand the whole concrete-keyed map through instead of looking the requested name up as a key. es6/es7 rest and es6 jest return that body already and needed no client change. Audited consequences of the `getIndex` change, all deliberate: - the temporal-literal resolution now applies through a single-index alias (SELECT and DML alike); - `SHOW TABLE` / `DESCRIBE` through such an alias resolve instead of failing; - `INSERT INTO ... SELECT` and `COPY INTO` targeting a single-index alias resolve their metadata; targeting a MULTI-index alias they now fail with a named "not found" instead of silently using one member's mapping (es6/es7 previously proceeded on an empty schema); - a resolved alias is a schema-cache success, so the negative miss cache never remembers it. An alias over several indices stays a schema-absent boundary: the literal is forwarded verbatim, never rewritten and never rejected by us. The optional "merge when every member agrees" variant is deliberately out of scope. Tests: the resolution rule over a GET-index document with one / two members (sql, Docker-free); the core seam through an overridden `executeGetIndex` (single alias rewritten with zero cached misses, multi alias reported not found and verbatim); testkit `TemporalLiteralSpec` gains a single-alias case (three spellings via `search` and `run`) and a multi-alias case, green on ES 6.8 rest, 6.8 jest, 7.17, 8.18 and 9.0 (11/11 each). Story BIDC-4 Closes #276 Co-Authored-By: Claude Opus 5 (1M context) --- .../elastic/client/IndicesApi.scala | 15 +++++++- .../client/TemporalLiteralSearchSpec.scala | 28 +++++++++++++++ documentation/sql/dql_statements.md | 11 +++--- .../elastic/client/java/JavaClientApi.scala | 18 +++++++--- .../elastic/client/java/JavaClientApi.scala | 18 +++++++--- .../softnetwork/elastic/schema/package.scala | 24 +++++++++++++ .../sql/query/TemporalLiteralsSpec.scala | 32 +++++++++++++++++ .../elastic/client/TemporalLiteralSpec.scala | 34 +++++++++++++++++++ 8 files changed, 166 insertions(+), 14 deletions(-) diff --git a/core/src/main/scala/app/softnetwork/elastic/client/IndicesApi.scala b/core/src/main/scala/app/softnetwork/elastic/client/IndicesApi.scala index 0393e3634..c849c2ff8 100644 --- a/core/src/main/scala/app/softnetwork/elastic/client/IndicesApi.scala +++ b/core/src/main/scala/app/softnetwork/elastic/client/IndicesApi.scala @@ -357,7 +357,20 @@ trait IndicesApi extends ElasticClientHelpers { } private def loadIndexAsSchema(index: String, json: String): ElasticResult[Option[Index]] = { - var tempIndex = Index(index, json) + val root = mapper.readTree(json) + // Issue #276 -- `index` may be an ALIAS. `Index.apply` resolves an alias over exactly ONE + // index; an alias over several is ambiguous (which mapping?) and is reported as NOT FOUND + // rather than as an empty schema, on every client alike. + if (!root.has(index) && !root.has("mappings")) { + val members = Index.indexDocuments(root) + if (members.size > 1) { + logger.warn( + s"⚠️ '$index' is an alias over ${members.size} indices (${members.map(_._1).mkString(", ")}); its schema cannot be resolved" + ) + return ElasticSuccess(None) + } + } + var tempIndex = Index(index, root) tempIndex.defaultIngestPipelineName match { case Some(pipeline) => logger.info( diff --git a/core/src/test/scala/app/softnetwork/elastic/client/TemporalLiteralSearchSpec.scala b/core/src/test/scala/app/softnetwork/elastic/client/TemporalLiteralSearchSpec.scala index d2f6e1857..7d231798a 100644 --- a/core/src/test/scala/app/softnetwork/elastic/client/TemporalLiteralSearchSpec.scala +++ b/core/src/test/scala/app/softnetwork/elastic/client/TemporalLiteralSearchSpec.scala @@ -200,6 +200,34 @@ class TemporalLiteralSearchSpec extends AnyFlatSpec with Matchers with BeforeAnd flooding.temporalLiteralSchemaMissCount should be <= 1024 } + it should "resolve the schema through an alias over one index, and treat an alias over several as unknown" in { + val events = + """{"aliases":{"events_alias":{},"multi_alias":{}},"mappings":{"properties":{ + | "id":{"type":"keyword"},"event_ts":{"type":"date"}}}, + |"settings":{"index":{"number_of_shards":"1","number_of_replicas":"0"}}}""".stripMargin + val archive = events.replace("\"events_alias\":{},", "") + val client = new RecordingClient { + // what `GET /` answers: the concrete index documents keyed by THEIR names + override private[client] def executeGetIndex(index: String): ElasticResult[Option[String]] = + index match { + case "events_alias" => ElasticResult.success(Some(s"""{"events":$events}""")) + case "multi_alias" => + ElasticResult.success(Some(s"""{"events":$events,"archive":$archive}""")) + case _ => ElasticResult.success(None) + } + } + client.search( + SelectStatement(s"SELECT id FROM events_alias WHERE event_ts >= '$spaceForm' LIMIT 5") + ) + client.lastQuery.getOrElse(fail("no query was rendered")).query should include(isoForm) + client.temporalLiteralSchemaMissCount shouldBe 0 // a resolved alias is never a miss + client.getIndex("multi_alias") shouldBe ElasticSuccess(None) // ambiguous: not found + client.search( + SelectStatement(s"SELECT id FROM multi_alias WHERE event_ts >= '$spaceForm' LIMIT 5") + ) + client.lastQuery.getOrElse(fail("no query was rendered")).query should include(spaceForm) + } + it should "not look the schema up at all when the WHERE carries no candidate literal" in { val client = seeded() client.search(SelectStatement("SELECT id FROM events WHERE amount > 10 LIMIT 5")) diff --git a/documentation/sql/dql_statements.md b/documentation/sql/dql_statements.md index 8e51908a7..1d0d1dd38 100644 --- a/documentation/sql/dql_statements.md +++ b/documentation/sql/dql_statements.md @@ -316,11 +316,12 @@ WHERE event_ts >= '2026-06-04T00:00:00' -- what Elasticsearch's default `date_nanos` columns, columns qualified with a `JOIN` alias (the FROM table's own columns are resolved) and `HAVING` conditions are never touched. - The resolution needs the index mapping, loaded through the schema cache (one lookup per index - every 5 minutes, and only for statements whose `WHERE` compares a string literal to a column). It - does not apply when the statement reads several indices or a wildcard, or when the mapping cannot - be loaded -- in particular **through an index alias**, whose mapping the client cannot resolve - today: there the literal is forwarded verbatim as in previous releases, and a failed mapping - lookup is remembered for 5 minutes so it is not retried on every statement. + every 5 minutes, and only for statements whose `WHERE` compares a string literal to a column). An + **index alias over exactly one index** resolves to that index's mapping (`SHOW TABLE` / + `DESCRIBE` through such an alias resolve the same way); an alias over **several** indices is + ambiguous and is treated as unresolvable. When the statement reads several indices or a wildcard, + or the mapping cannot be loaded, the literal is forwarded verbatim as in previous releases, and a + failed mapping lookup is remembered for 5 minutes so it is not retried on every statement. --- diff --git a/es8/java/src/main/scala/app/softnetwork/elastic/client/java/JavaClientApi.scala b/es8/java/src/main/scala/app/softnetwork/elastic/client/java/JavaClientApi.scala index 7930cfbc2..7a5ee78dc 100644 --- a/es8/java/src/main/scala/app/softnetwork/elastic/client/java/JavaClientApi.scala +++ b/es8/java/src/main/scala/app/softnetwork/elastic/client/java/JavaClientApi.scala @@ -269,10 +269,20 @@ trait JavaClientIndicesApi extends IndicesApi with JavaClientHelpers { new GetIndexRequest.Builder().index(index).build() ) )(response => { - val valueOpt = response.result.asScala.get(index) - valueOpt match { - case Some(value) => Some(convertToJson(value)) - case None => None + val indices = response.result.asScala + indices.get(index) match { + case Some(value) => Some(convertToJson(value)) + case None if indices.nonEmpty => + // Issue #276 -- `index` is an ALIAS: the response is keyed by the CONCRETE index names. + // Hand the whole map to core, whose `Index.apply` resolves an alias over one index and + // reports an alias over several as not found. + val root = mapper.createObjectNode() + indices.foreach { case (name, value) => + root.set[JsonNode](name, mapper.readTree(convertToJson(value))) + () + } + Some(root.toString) + case None => None } }) } diff --git a/es9/java/src/main/scala/app/softnetwork/elastic/client/java/JavaClientApi.scala b/es9/java/src/main/scala/app/softnetwork/elastic/client/java/JavaClientApi.scala index 6efc8dbc9..1892d1fab 100644 --- a/es9/java/src/main/scala/app/softnetwork/elastic/client/java/JavaClientApi.scala +++ b/es9/java/src/main/scala/app/softnetwork/elastic/client/java/JavaClientApi.scala @@ -267,10 +267,20 @@ trait JavaClientIndicesApi extends IndicesApi with JavaClientHelpers { new GetIndexRequest.Builder().index(index).build() ) )(response => { - val valueOpt = response.indices().asScala.get(index) - valueOpt match { - case Some(value) => Some(convertToJson(value)) - case None => None + val indices = response.indices().asScala + indices.get(index) match { + case Some(value) => Some(convertToJson(value)) + case None if indices.nonEmpty => + // Issue #276 -- `index` is an ALIAS: the response is keyed by the CONCRETE index names. + // Hand the whole map to core, whose `Index.apply` resolves an alias over one index and + // reports an alias over several as not found. + val root = mapper.createObjectNode() + indices.foreach { case (name, value) => + root.set[JsonNode](name, mapper.readTree(convertToJson(value))) + () + } + Some(root.toString) + case None => None } }) } diff --git a/sql/src/main/scala/app/softnetwork/elastic/schema/package.scala b/sql/src/main/scala/app/softnetwork/elastic/schema/package.scala index 6e53d5a9a..aa3359438 100644 --- a/sql/src/main/scala/app/softnetwork/elastic/schema/package.scala +++ b/sql/src/main/scala/app/softnetwork/elastic/schema/package.scala @@ -563,11 +563,35 @@ package object schema { apply(name, root) } + /** The index documents a GET-index response holds: `{"": {"mappings": ..., ...}}` for a + * concrete index, and -- for `GET /` -- one such entry PER INDEX behind the alias, each + * keyed by the index's own name (never by the alias). + */ + def indexDocuments(root: JsonNode): Seq[(String, JsonNode)] = + if (root != null && root.isObject) + root + .properties() + .asScala + .toSeq + .collect { + case entry if entry.getValue.isObject && entry.getValue.has("mappings") => + entry.getKey -> entry.getValue + } + else Seq.empty + def apply(name: String, root: JsonNode): Index = { if (root.has(name)) { val indexNode = root.path(name) return apply(name, indexNode) } + // Issue #276 -- `name` is an ALIAS: `GET /` answers with the concrete index/indices + // keyed by THEIR names. An alias over exactly one index resolves to that index's document + // (the schema keeps the alias as its name); an alias over several is ambiguous and falls + // through to the mapping-less shape below -- callers detect it with [[indexDocuments]]. + indexDocuments(root) match { + case Seq((_, single)) => return apply(name, single) + case _ => + } val mappings = root.path("mappings") val settings = root.path("settings") val aliasesNode = root.path("aliases") diff --git a/sql/src/test/scala/app/softnetwork/elastic/sql/query/TemporalLiteralsSpec.scala b/sql/src/test/scala/app/softnetwork/elastic/sql/query/TemporalLiteralsSpec.scala index 838a8e5af..043113fb8 100644 --- a/sql/src/test/scala/app/softnetwork/elastic/sql/query/TemporalLiteralsSpec.scala +++ b/sql/src/test/scala/app/softnetwork/elastic/sql/query/TemporalLiteralsSpec.scala @@ -450,6 +450,38 @@ class TemporalLiteralsSpec extends AnyFlatSpec with Matchers { untouched("SELECT id FROM events WHERE label = '2026-06-04 00:00:00'", table) } + it should "resolve the schema through an index alias over exactly one index, not over several" in { + // `GET /` answers with the concrete index documents keyed by THEIR names (lead ruling on + // review R4-5). One document: the alias resolves to it (the schema keeps the alias name); two + // documents: ambiguous -- no mapping, so nothing is a candidate and the caller reports not found. + val events = + """{"aliases":{"events_alias":{},"multi_alias":{}},"mappings":{"properties":{ + | "id":{"type":"keyword"},"event_ts":{"type":"date"},"label":{"type":"keyword"}}}, + |"settings":{"index":{"number_of_shards":"1","number_of_replicas":"0"}}}""".stripMargin + val archive = + """{"aliases":{"multi_alias":{}},"mappings":{"properties":{ + | "id":{"type":"keyword"},"event_ts":{"type":"date","format":"yyyy-MM-dd HH:mm:ss"}}}, + |"settings":{"index":{"number_of_shards":"1","number_of_replicas":"0"}}}""".stripMargin + val single = Index("events_alias", s"""{"events":$events}""") + single.name shouldBe "events_alias" + single.schema.find("event_ts").map(_.dataType) shouldBe Some(SQLTypes.Date) + whereSql( + resolved("SELECT id FROM events_alias WHERE event_ts >= '2026-06-04 00:00:00'", single.schema) + ) shouldBe "WHERE event_ts >= '2026-06-04T00:00:00'" + + val multiRoot: com.fasterxml.jackson.databind.JsonNode = + new com.fasterxml.jackson.databind.ObjectMapper() + .readTree(s"""{"events":$events,"archive":$archive}""") + Index.indexDocuments(multiRoot).map(_._1) shouldBe Seq("events", "archive") + val multi = Index("multi_alias", multiRoot) + multi.schema.columns shouldBe empty + untouched("SELECT id FROM multi_alias WHERE event_ts >= '2026-06-04 00:00:00'", multi.schema) + + // a concrete index keyed by its own name is untouched by the rule + Index("events", s"""{"events":$events}""").schema.find("event_ts").map(_.dataType) shouldBe + Some(SQLTypes.Date) + } + it should "walk into an UNNEST nested criteria" in { val json = """{"events":{"aliases":{},"mappings":{"properties":{ diff --git a/testkit/src/main/scala/app/softnetwork/elastic/client/TemporalLiteralSpec.scala b/testkit/src/main/scala/app/softnetwork/elastic/client/TemporalLiteralSpec.scala index e74f3046e..a7ef1783d 100644 --- a/testkit/src/main/scala/app/softnetwork/elastic/client/TemporalLiteralSpec.scala +++ b/testkit/src/main/scala/app/softnetwork/elastic/client/TemporalLiteralSpec.scala @@ -227,6 +227,40 @@ trait TemporalLiteralSpec extends AnyFlatSpecLike with ElasticDockerTestKit with fromJune4 } + // ---- Aliases (lead ruling on review R4-5) --------------------------------------------------------- + + private val singleAlias = "temporal_literal_alias" + + private val multiAlias = "temporal_literal_multi_alias" + + "a query through an alias over one index" should "match the same rows for every spelling" in { + client.addAlias(defaultIndex, singleAlias).get shouldBe true + Seq("2026-06-04 00:00:00.000000", "2026-06-04 00:00:00", "2026-06-04T00:00:00").foreach { + literal => + withClue(literal) { + searchIds(s"SELECT id FROM $singleAlias WHERE event_ts >= '$literal'") shouldBe fromJune4 + } + } + gatewayIds(s"SELECT id FROM $singleAlias WHERE event_ts >= '2026-06-04 00:00:00'") shouldBe + fromJune4 + } + + "a query through an alias over several indices" should "forward the literal verbatim (not our rejection)" in { + client.addAlias(defaultIndex, multiAlias).get shouldBe true + client.addAlias(customIndex, multiAlias).get shouldBe true + // the alias itself is queryable (a non-temporal predicate: the two members' `date` formats + // differ, so a date literal would be judged by each member's own mapping) + searchIds(s"SELECT id FROM $multiAlias WHERE amount = 8") shouldBe Set("e8") + // the space form is neither rewritten nor rejected by us -- the ambiguous mapping is not + // resolved, so Elasticsearch's own answer comes back (a raw parse failure on the default index) + client.search( + SelectStatement(s"SELECT id FROM $multiAlias WHERE event_ts >= '2026-06-04 00:00:00'") + ) match { + case ElasticFailure(error) => error.message should not include "Cannot parse '" + case ElasticSuccess(_) => // a lenient cluster may answer rows; the point is no named 400 + } + } + // ---- DML: the same WHERE, the same rows (review R4-4) -------------------------------------------- private def runDml(sql: String): Unit = From d69b77d69116589aee86c1831a31386a780b7fb5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Manciot?= Date: Sun, 6 Sep 2026 12:16:16 +0200 Subject: [PATCH 5/5] fix(core,sql): delta-review fixes for the #276 alias resolution Delta review of ab251b5e approved the design; two of its LOW findings are silent-failure shaped, so they are fixed here with the rest. - R4-18: the ambiguity test counted the documents that CARRY a `mappings` key, so a two-index alias whose other member has no mapping resolved silently to the first - the very silent-wrong-answer mode this story removes. `Index.indexDocuments` now returns every top-level index ENTRY and keeps the mappings/settings/aliases check only as the shape test that tells one index document from a map of them. Pinned: a two-member alias with a mapping-less member is reported NOT FOUND, never resolved. - R4-21: `invalidateSchema()` left a cached ALIAS schema in place, so after an ALTER TABLE a query through the alias kept a stale mapping - and a stale date `format` - for up to the 5-minute TTL. `Index` now carries the concrete name it resolved from, `IndicesApi` keeps an alias -> target map beside the schema cache, and invalidating (or updating) an index drops every alias entry pointing at it. Pinned: cache the alias, invalidate the concrete index, the next alias query re-fetches. - R4-19: the core seam spec now asserts that an ambiguous alias is BOUNDED - its 404 is remembered and a second statement performs no further lookup. - R4-20: the testkit multi-alias case asserts the literal is VERBATIM in the emitted query (unrewritten and unrejected) instead of tolerating any success. The alias now spans two custom-format indices so the query reaches Elasticsearch and its body can be asserted. - R4-17: a resolved alias is no longer listed among its own ALIASES, so `SHOW CREATE TABLE ` does not render a self-referential entry. Story BIDC-4 Closes #276 Co-Authored-By: Claude Opus 5 (1M context) --- .../elastic/client/IndicesApi.scala | 31 +++++++++++++- .../client/TemporalLiteralSearchSpec.scala | 35 ++++++++++++++++ .../softnetwork/elastic/schema/package.scala | 42 +++++++++++++------ .../sql/query/TemporalLiteralsSpec.scala | 15 +++++++ .../elastic/client/TemporalLiteralSpec.scala | 42 +++++++++++++++---- 5 files changed, 143 insertions(+), 22 deletions(-) diff --git a/core/src/main/scala/app/softnetwork/elastic/client/IndicesApi.scala b/core/src/main/scala/app/softnetwork/elastic/client/IndicesApi.scala index c849c2ff8..e50d57c42 100644 --- a/core/src/main/scala/app/softnetwork/elastic/client/IndicesApi.scala +++ b/core/src/main/scala/app/softnetwork/elastic/client/IndicesApi.scala @@ -60,6 +60,12 @@ trait IndicesApi extends ElasticClientHelpers { private val schemaCache = new ConcurrentHashMap[String, (Schema, Long)]() + /** Alias -> the CONCRETE index its cached schema was resolved from (issue #276). Without it an + * `ALTER TABLE ` would leave a stale mapping -- and therefore a stale date `format` -- + * reachable through the alias for the rest of the TTL (review R4-21). + */ + private val schemaAliasTargets = new ConcurrentHashMap[String, String]() + // ======================================================================== // PUBLIC METHODS // ======================================================================== @@ -231,8 +237,22 @@ trait IndicesApi extends ElasticClientHelpers { } } + /** Drop every cached ALIAS schema resolved from `index` (#276 / review R4-21). */ + private def invalidateAliasesOf(index: String): Unit = + schemaAliasTargets + .entrySet() + .asScala + .collect { case e if e.getValue == index => e.getKey } + .toList + .foreach { alias => + schemaCache.remove(alias) + schemaAliasTargets.remove(alias) + logger.debug(s"📦 Schema cache invalidated for alias '$alias' (target '$index')") + } + def updateSchema(index: String, schema: Schema): Unit = { schemaCache.put(index, (schema, System.currentTimeMillis())) + invalidateAliasesOf(index) // #238 — ALTER TABLE may have reindexed into a different shard count invalidateShardCounts(Some(index)) logger.debug(s"📦 Schema cache updated for '$index'") @@ -240,12 +260,15 @@ trait IndicesApi extends ElasticClientHelpers { def invalidateSchema(index: String): Unit = { schemaCache.remove(index) + val _ = schemaAliasTargets.remove(index) + invalidateAliasesOf(index) invalidateShardCounts(Some(index)) // #238 — the sliced-paging shard counts follow the schema logger.info(s"🗑️ Schema cache invalidated for '$index'") } def invalidateAllSchemas(): Unit = { schemaCache.clear() + schemaAliasTargets.clear() invalidateShardCounts() logger.info("🗑️ All schema caches invalidated") } @@ -253,6 +276,12 @@ trait IndicesApi extends ElasticClientHelpers { private def fetchSchemaFromES(index: String): ElasticResult[Schema] = { getIndex(index) match { case ElasticSuccess(Some(idx)) => + // #276 -- remember which concrete index an ALIAS resolved to, so invalidating that index + // also drops the alias entry. + idx.resolvedFrom.filter(_ != index) match { + case Some(concrete) => val _ = schemaAliasTargets.put(index, concrete) + case None => val _ = schemaAliasTargets.remove(index) + } ElasticSuccess(idx.schema) case ElasticSuccess(None) => logger.warn(s"Index '$index' not found for schema loading") @@ -361,7 +390,7 @@ trait IndicesApi extends ElasticClientHelpers { // Issue #276 -- `index` may be an ALIAS. `Index.apply` resolves an alias over exactly ONE // index; an alias over several is ambiguous (which mapping?) and is reported as NOT FOUND // rather than as an empty schema, on every client alike. - if (!root.has(index) && !root.has("mappings")) { + if (!root.has(index)) { val members = Index.indexDocuments(root) if (members.size > 1) { logger.warn( diff --git a/core/src/test/scala/app/softnetwork/elastic/client/TemporalLiteralSearchSpec.scala b/core/src/test/scala/app/softnetwork/elastic/client/TemporalLiteralSearchSpec.scala index 7d231798a..7c300719c 100644 --- a/core/src/test/scala/app/softnetwork/elastic/client/TemporalLiteralSearchSpec.scala +++ b/core/src/test/scala/app/softnetwork/elastic/client/TemporalLiteralSearchSpec.scala @@ -222,10 +222,45 @@ class TemporalLiteralSearchSpec extends AnyFlatSpec with Matchers with BeforeAnd client.lastQuery.getOrElse(fail("no query was rendered")).query should include(isoForm) client.temporalLiteralSchemaMissCount shouldBe 0 // a resolved alias is never a miss client.getIndex("multi_alias") shouldBe ElasticSuccess(None) // ambiguous: not found + val before = client.schemaLookups client.search( SelectStatement(s"SELECT id FROM multi_alias WHERE event_ts >= '$spaceForm' LIMIT 5") ) client.lastQuery.getOrElse(fail("no query was rendered")).query should include(spaceForm) + client.schemaLookups shouldBe (before + 1) + client.temporalLiteralSchemaMissCount shouldBe 1 + // R4-19: the ambiguous alias is BOUNDED -- a second statement costs no further lookup + client.search( + SelectStatement(s"SELECT id FROM multi_alias WHERE event_ts < '$spaceForm' LIMIT 5") + ) + client.lastQuery.getOrElse(fail("no query was rendered")).query should include(spaceForm) + client.schemaLookups shouldBe (before + 1) + } + + it should "drop a cached alias schema when the concrete index it resolved from is invalidated" in { + // R4-21: without this an ALTER TABLE on the concrete index would leave a stale mapping (hence + // a stale date `format`) reachable through the alias for the rest of the TTL. + val events = + """{"aliases":{"events_alias":{}},"mappings":{"properties":{ + | "id":{"type":"keyword"},"event_ts":{"type":"date"}}}, + |"settings":{"index":{"number_of_shards":"1","number_of_replicas":"0"}}}""".stripMargin + var fetches = 0 + val client = new RecordingClient { + override private[client] def executeGetIndex(index: String): ElasticResult[Option[String]] = + if (index == "events_alias") { + fetches += 1 + ElasticResult.success(Some(s"""{"events":$events}""")) + } else ElasticResult.success(None) + } + val statement = + SelectStatement(s"SELECT id FROM events_alias WHERE event_ts >= '$spaceForm' LIMIT 5") + client.search(statement) + client.search(statement) + fetches shouldBe 1 // cached + client.invalidateSchema("events") // the CONCRETE index, not the alias + client.search(statement) + fetches shouldBe 2 // the alias entry went with it + client.lastQuery.getOrElse(fail("no query was rendered")).query should include(isoForm) } it should "not look the schema up at all when the WHERE carries no candidate literal" in { diff --git a/sql/src/main/scala/app/softnetwork/elastic/schema/package.scala b/sql/src/main/scala/app/softnetwork/elastic/schema/package.scala index aa3359438..eef1c2e18 100644 --- a/sql/src/main/scala/app/softnetwork/elastic/schema/package.scala +++ b/sql/src/main/scala/app/softnetwork/elastic/schema/package.scala @@ -452,7 +452,12 @@ package object schema { settings: JsonNode, aliases: Map[String, JsonNode] = Map.empty, defaultPipeline: Option[JsonNode] = None, - finalPipeline: Option[JsonNode] = None + finalPipeline: Option[JsonNode] = None, + /** Set when [[name]] is an ALIAS this index was resolved from: the concrete index name (issue + * #276). The caller uses it to invalidate the alias entry when the concrete index's schema + * changes. + */ + resolvedFrom: Option[String] = None ) { lazy val defaultIngestPipelineName: Option[String] = esSettings.options.get("index") match { @@ -563,21 +568,30 @@ package object schema { apply(name, root) } - /** The index documents a GET-index response holds: `{"": {"mappings": ..., ...}}` for a - * concrete index, and -- for `GET /` -- one such entry PER INDEX behind the alias, each - * keyed by the index's own name (never by the alias). + /** Top-level keys of an index-document MAP: what `GET /` answers, one entry per index + * behind the alias, each keyed by the index's own name (never by the alias). + * + * Returns EVERY object-valued entry -- not only the ones carrying `mappings` -- so an alias + * over several indices is detected as ambiguous even when a member has no mapping at all + * (counting mapping-bearing documents would let such an alias resolve silently to the other + * member: review R4-18). + * + * Empty when `root` is itself ONE index document (`{"mappings": …, "settings": …}`, what a + * typed client returns for a concrete index). The discriminator is a top-level `mappings` / + * `settings` / `aliases` key; an alias over an index literally NAMED one of those three would + * be misread, which no real deployment does. */ def indexDocuments(root: JsonNode): Seq[(String, JsonNode)] = - if (root != null && root.isObject) + if ( + root == null || !root.isObject || + root.has("mappings") || root.has("settings") || root.has("aliases") + ) Seq.empty + else root .properties() .asScala .toSeq - .collect { - case entry if entry.getValue.isObject && entry.getValue.has("mappings") => - entry.getKey -> entry.getValue - } - else Seq.empty + .collect { case entry if entry.getValue.isObject => entry.getKey -> entry.getValue } def apply(name: String, root: JsonNode): Index = { if (root.has(name)) { @@ -589,8 +603,12 @@ package object schema { // (the schema keeps the alias as its name); an alias over several is ambiguous and falls // through to the mapping-less shape below -- callers detect it with [[indexDocuments]]. indexDocuments(root) match { - case Seq((_, single)) => return apply(name, single) - case _ => + case Seq((concrete, single)) => + val resolved = apply(name, single) + // `name` is an alias OF `concrete`, not of itself: dropping the self-entry keeps + // `SHOW CREATE TABLE ` from rendering the alias among its own ALIASES (R4-17). + return resolved.copy(aliases = resolved.aliases - name, resolvedFrom = Some(concrete)) + case _ => } val mappings = root.path("mappings") val settings = root.path("settings") diff --git a/sql/src/test/scala/app/softnetwork/elastic/sql/query/TemporalLiteralsSpec.scala b/sql/src/test/scala/app/softnetwork/elastic/sql/query/TemporalLiteralsSpec.scala index 043113fb8..14f8467d9 100644 --- a/sql/src/test/scala/app/softnetwork/elastic/sql/query/TemporalLiteralsSpec.scala +++ b/sql/src/test/scala/app/softnetwork/elastic/sql/query/TemporalLiteralsSpec.scala @@ -480,6 +480,21 @@ class TemporalLiteralsSpec extends AnyFlatSpec with Matchers { // a concrete index keyed by its own name is untouched by the rule Index("events", s"""{"events":$events}""").schema.find("event_ts").map(_.dataType) shouldBe Some(SQLTypes.Date) + + // R4-18: a member WITHOUT mappings still counts -- otherwise a two-index alias would resolve + // silently to the other member (the silent-wrong-answer mode this story removes) + val mappingless = + """{"aliases":{"partial_alias":{}},"settings":{"index":{"number_of_shards":"1"}}}""" + val partialRoot: com.fasterxml.jackson.databind.JsonNode = + new com.fasterxml.jackson.databind.ObjectMapper() + .readTree(s"""{"events":$events,"no_mappings":$mappingless}""") + Index.indexDocuments(partialRoot).map(_._1) shouldBe Seq("events", "no_mappings") + Index("partial_alias", partialRoot).schema.columns shouldBe empty + + // R4-17: a resolved alias is not an alias of itself + single.schema.aliases.keySet should not contain "events_alias" + single.resolvedFrom shouldBe Some("events") + Index("events", s"""{"events":$events}""").resolvedFrom shouldBe None } it should "walk into an UNNEST nested criteria" in { diff --git a/testkit/src/main/scala/app/softnetwork/elastic/client/TemporalLiteralSpec.scala b/testkit/src/main/scala/app/softnetwork/elastic/client/TemporalLiteralSpec.scala index a7ef1783d..a3c60c4b1 100644 --- a/testkit/src/main/scala/app/softnetwork/elastic/client/TemporalLiteralSpec.scala +++ b/testkit/src/main/scala/app/softnetwork/elastic/client/TemporalLiteralSpec.scala @@ -64,6 +64,12 @@ trait TemporalLiteralSpec extends AnyFlatSpecLike with ElasticDockerTestKit with /** Mutated by the DML cases (UPDATE then DELETE), so it gets its own copy of the rows. */ private val dmlIndex = "temporal_literal_dml" + /** Second custom-format index: the multi-index alias spans two members whose `date` format + * AGREES, so the ambiguous-alias query reaches Elasticsearch and its emitted body can be + * asserted verbatim (review R4-20). + */ + private val customIndex2 = "temporal_literal_custom_b" + /** Eight timestamps (UTC), ids `e1`..`e8`; `>= 2026-06-04T00:00:00` selects `e4`..`e8`. */ private val timestamps: Seq[String] = Seq( "2026-06-01T00:00:00", @@ -108,6 +114,8 @@ trait TemporalLiteralSpec extends AnyFlatSpecLike with ElasticDockerTestKit with client.setMapping(customIndex, customMapping).get shouldBe true client.createIndex(dmlIndex, settings = settings).get shouldBe true client.setMapping(dmlIndex, defaultMapping).get shouldBe true + client.createIndex(customIndex2, settings = settings).get shouldBe true + client.setMapping(customIndex2, customMapping).get shouldBe true val defaultDocs = timestamps.zipWithIndex.map { case (ts, i) => // `label` holds the SQL spelling of the same instant: the keyword negative control @@ -135,18 +143,26 @@ trait TemporalLiteralSpec extends AnyFlatSpecLike with ElasticDockerTestKit with load(defaultIndex, defaultDocs) load(customIndex, customDocs) load(dmlIndex, defaultDocs) + load(customIndex2, customDocs) } override def afterAll(): Unit = { client.deleteIndex(defaultIndex) client.deleteIndex(customIndex) client.deleteIndex(dmlIndex) + client.deleteIndex(customIndex2) super.afterAll() } private def ids(rows: Seq[ListMap[String, Any]]): Set[String] = rows.flatMap(_.get("id").map(_.toString)).toSet + private def searchResponse(sql: String): ElasticResponse = + client.search(SelectStatement(sql)) match { + case ElasticSuccess(response) => response + case ElasticFailure(error) => fail(s"Query failed: ${error.message}\n$sql") + } + /** Client venue: `SearchApi.search`. */ private def searchIds(sql: String): Set[String] = client.search(SelectStatement(sql)) match { @@ -245,19 +261,27 @@ trait TemporalLiteralSpec extends AnyFlatSpecLike with ElasticDockerTestKit with fromJune4 } - "a query through an alias over several indices" should "forward the literal verbatim (not our rejection)" in { - client.addAlias(defaultIndex, multiAlias).get shouldBe true + "a query through an alias over several indices" should "forward the literal verbatim" in { + // Both members carry `format: "yyyy-MM-dd HH:mm:ss"`, so Elasticsearch itself accepts the space + // form and the emitted body can be asserted: an ambiguous alias must reach ES UNCHANGED -- + // neither rewritten to the `T` form nor rejected by us (review R4-20). client.addAlias(customIndex, multiAlias).get shouldBe true - // the alias itself is queryable (a non-temporal predicate: the two members' `date` formats - // differ, so a date literal would be judged by each member's own mapping) - searchIds(s"SELECT id FROM $multiAlias WHERE amount = 8") shouldBe Set("e8") - // the space form is neither rewritten nor rejected by us -- the ambiguous mapping is not - // resolved, so Elasticsearch's own answer comes back (a raw parse failure on the default index) + client.addAlias(customIndex2, multiAlias).get shouldBe true + searchIds(s"SELECT id FROM $multiAlias WHERE amount = 8") shouldBe Set("e8") // the alias works + + val response = searchResponse( + s"SELECT id FROM $multiAlias WHERE event_ts >= '2026-06-04 00:00:00'" + ) + response.query should include("2026-06-04 00:00:00") + response.query should not include "2026-06-04T00:00:00" + ids(response.results) shouldBe fromJune4 + + // and an unparseable literal is Elasticsearch's to judge there, never our named 400 client.search( - SelectStatement(s"SELECT id FROM $multiAlias WHERE event_ts >= '2026-06-04 00:00:00'") + SelectStatement(s"SELECT id FROM $multiAlias WHERE event_ts >= 'not-a-date'") ) match { case ElasticFailure(error) => error.message should not include "Cannot parse '" - case ElasticSuccess(_) => // a lenient cluster may answer rows; the point is no named 400 + case ElasticSuccess(_) => // a lenient member may match nothing; the point is no named 400 } }