From 6c7b5e3332136147e2ef2ba3bfd7b38554d70bf7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Manciot?= Date: Sun, 6 Sep 2026 06:14:31 +0200 Subject: [PATCH 01/11] wip(bridge,client): extended_stats over a transform -- marker + SearchBodySerializer seam, per-major serializers (#222) Work in progress on story BIDC-3; tests, docs and the final message follow in a later commit. Story BIDC-3 Co-Authored-By: Claude Fable 5.1 --- .../sql/bridge/ElasticAggregation.scala | 25 ++++-- .../bridge/ElasticMultiSearchRequest.scala | 6 ++ .../sql/bridge/ElasticSearchRequest.scala | 15 +++- .../ScriptedExtendedStatsAggregation.scala | 83 +++++++++++++++++++ .../sql/bridge/SearchBodySerializer.scala | 73 ++++++++++++++++ .../elastic/sql/bridge/package.scala | 66 ++++++++------- .../elastic/client/GatewayApi.scala | 23 +++++ .../sql/bridge/ElasticAggregation.scala | 25 ++++-- .../bridge/ElasticMultiSearchRequest.scala | 6 ++ .../sql/bridge/ElasticSearchRequest.scala | 16 +++- .../ScriptedExtendedStatsAggregation.scala | 75 +++++++++++++++++ .../sql/bridge/SearchBodySerializer.scala | 69 +++++++++++++++ .../elastic/sql/bridge/package.scala | 66 ++++++++------- .../elastic/client/jest/JestSearchApi.scala | 10 ++- .../jest/JestSearchBodySerializer.scala | 51 ++++++++++++ .../client/rest/RestHighLevelClientApi.scala | 9 ++ ...tHighLevelClientSearchBodySerializer.scala | 51 ++++++++++++ .../client/rest/RestHighLevelClientApi.scala | 9 ++ ...tHighLevelClientSearchBodySerializer.scala | 51 ++++++++++++ .../elastic/client/java/JavaClientApi.scala | 8 ++ .../java/JavaClientSearchBodySerializer.scala | 76 +++++++++++++++++ .../elastic/client/java/JavaClientApi.scala | 8 ++ .../java/JavaClientSearchBodySerializer.scala | 76 +++++++++++++++++ 23 files changed, 819 insertions(+), 78 deletions(-) create mode 100644 bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/ScriptedExtendedStatsAggregation.scala create mode 100644 bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/SearchBodySerializer.scala create mode 100644 es6/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/ScriptedExtendedStatsAggregation.scala create mode 100644 es6/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/SearchBodySerializer.scala create mode 100644 es6/jest/src/main/scala/app/softnetwork/elastic/client/jest/JestSearchBodySerializer.scala create mode 100644 es6/rest/src/main/scala/app/softnetwork/elastic/client/rest/RestHighLevelClientSearchBodySerializer.scala create mode 100644 es7/rest/src/main/scala/app/softnetwork/elastic/client/rest/RestHighLevelClientSearchBodySerializer.scala create mode 100644 es8/java/src/main/scala/app/softnetwork/elastic/client/java/JavaClientSearchBodySerializer.scala create mode 100644 es9/java/src/main/scala/app/softnetwork/elastic/client/java/JavaClientSearchBodySerializer.scala diff --git a/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/ElasticAggregation.scala b/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/ElasticAggregation.scala index f01cfa5e1..7955f0c3c 100644 --- a/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/ElasticAggregation.scala +++ b/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/ElasticAggregation.scala @@ -89,11 +89,18 @@ case class ElasticAggregation( // CHECK if it is a "global" metric (cardinality, etc.) or a bucket metric (avg, sum, etc.) val isGlobalMetric: Boolean = agg match { - case _: CardinalityAggregation => true - case _: StatsAggregation => true - case _: ExtendedStatsAggregation => true - case _ => false + case _: CardinalityAggregation => true + case _: StatsAggregation => true + case _: ExtendedStatsAggregation => true + case _: ScriptedExtendedStatsAggregation => true + case _ => false } + + /** True when this aggregation is `STDDEV` / `VARIANCE` (any of the `extended_stats` family) over + * a TRANSFORMED expression -- the shape a client module must either render with its script or + * refuse (issue #222). The second serialisation door (`sqlQueryToAggregations`) reads it. + */ + def hasTransformExtendedStats: Boolean = ScriptedExtendedStatsAggregation.existsIn(Seq(agg)) } object ElasticAggregation { @@ -196,7 +203,10 @@ object ElasticAggregation { case STDDEV | STDDEV_SAMP | STDDEV_POP | VARIANCE | VAR_SAMP | VAR_POP => aggWithFieldOrScript( extendedStatsAgg, - (name, s) => extendedStatsAgg(name, sourceField).script(s) + // Issue #222 -- a transform-bearing extended_stats is bound to the bridge's own marker: + // elastic4s's ExtendedStatsAggregationBuilder drops `script`, so the library type + // would serialise as the statistic of the raw field. See ScriptedExtendedStatsAggregation. + (name, s) => ScriptedExtendedStatsAggregation(extendedStatsAgg(name, sourceField).script(s)) ) case th: WindowFunction => th.window match { @@ -222,7 +232,10 @@ object ElasticAggregation { case STDDEV | STDDEV_SAMP | STDDEV_POP | VARIANCE | VAR_SAMP | VAR_POP => aggWithFieldOrScript( extendedStatsAgg, - (name, s) => extendedStatsAgg(name, sourceField).script(s) + // Issue #222 -- a transform-bearing extended_stats is bound to the bridge's own marker: + // elastic4s's ExtendedStatsAggregationBuilder drops `script`, so the library type + // would serialise as the statistic of the raw field. See ScriptedExtendedStatsAggregation. + (name, s) => ScriptedExtendedStatsAggregation(extendedStatsAgg(name, sourceField).script(s)) ) case PERCENTILE_CONT | PERCENTILE_DISC => // Both map to ES `percentiles` (TDigest). One call → one percent; diff --git a/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/ElasticMultiSearchRequest.scala b/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/ElasticMultiSearchRequest.scala index c6e74b035..0388fc6f9 100644 --- a/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/ElasticMultiSearchRequest.scala +++ b/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/ElasticMultiSearchRequest.scala @@ -22,5 +22,11 @@ case class ElasticMultiSearchRequest( requests: Seq[ElasticSearchRequest], multiSearch: MultiSearchRequest ) { + // Not routed through SearchBodySerializer (issue #222): no production path serialises a + // multi-search here -- core builds `_msearch` bodies from each request's own + // `singleSearchToJsonQuery` (`ElasticQueries.multiQuery`). A transform-bearing extended_stats + // inside this body still fails LOUDLY on every elastic4s line (its default aggregation handler + // throws `NotImplementedError` on the ScriptedExtendedStatsAggregation marker); it can never + // leave as the statistic of the wrong field. def query: String = MultiSearchBuilderFn(multiSearch).replace("\"version\":true,", "") /*FIXME*/ } diff --git a/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/ElasticSearchRequest.scala b/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/ElasticSearchRequest.scala index cfe03ba2b..3337d604e 100644 --- a/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/ElasticSearchRequest.scala +++ b/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/ElasticSearchRequest.scala @@ -17,7 +17,7 @@ package app.softnetwork.elastic.sql.bridge import app.softnetwork.elastic.sql.query.{Bucket, Criteria, Except, Field, FieldSort} -import com.sksamuel.elastic4s.requests.searches.{SearchBodyBuilderFn, SearchRequest} +import com.sksamuel.elastic4s.requests.searches.SearchRequest case class ElasticSearchRequest( sql: String, @@ -30,7 +30,10 @@ case class ElasticSearchRequest( search: SearchRequest, buckets: Seq[Bucket] = Seq.empty, having: Option[Criteria] = None, - sorts: Seq[FieldSort] = Seq.empty + sorts: Seq[FieldSort] = Seq.empty, + // The body serializer the client module injected through the SingleSearch conversion (issue + // #222); Default = the one-argument elastic4s builder, refusing a transform-bearing extended_stats. + serializer: SearchBodySerializer = SearchBodySerializer.Default ) { def minScore(score: Option[Double]): ElasticSearchRequest = { score match { @@ -39,6 +42,12 @@ case class ElasticSearchRequest( } } + /** True when this request carries `STDDEV` / `VARIANCE` (any of the `extended_stats` family) over + * a TRANSFORMED expression -- plain (`STDDEV(YEAR(x))`) or windowed (`... OVER (PARTITION BY + * ...)`). The shape a client module must either render with its script or refuse (issue #222). + */ + def hasTransformExtendedStats: Boolean = SearchBodySerializer.hasTransformExtendedStats(search) + def query: String = - SearchBodyBuilderFn(search).string.replace("\"version\":true,", "") /*FIXME*/ + serializer.serialize(search).replace("\"version\":true,", "") /*FIXME*/ } diff --git a/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/ScriptedExtendedStatsAggregation.scala b/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/ScriptedExtendedStatsAggregation.scala new file mode 100644 index 000000000..8477d9db7 --- /dev/null +++ b/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/ScriptedExtendedStatsAggregation.scala @@ -0,0 +1,83 @@ +/* + * 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.bridge + +import com.sksamuel.elastic4s.requests.searches.aggs.{ + AbstractAggregation, + Aggregation, + ExtendedStatsAggregation +} + +/** An `extended_stats` aggregation over a TRANSFORMED expression -- `STDDEV(YEAR(createdAt))`, + * `VARIANCE(ABS(salary))` and the rest of the family (issue #222). + * + * elastic4s's own `ExtendedStatsAggregationBuilder` never emits `agg.script` (the only metric + * builder of ten with the omission; fixed upstream once in elastic4s#2700 onto a dead branch, + * re-submitted as elastic4s#4100), so an `ExtendedStatsAggregation` carrying a script silently + * serialises as the statistic of the RAW field -- or as `extended_stats: {}` when there is no raw + * field to fall back on. The bridge therefore binds a transform-bearing extended_stats to this + * marker instead of to the library type it wraps. A marker is a type elastic4s does not know, so: + * + * - `AggregationBuilderFn`'s typed arms never claim it, and the `customAggregation` handler of + * the 8.x/9.x two-argument `SearchBodyBuilderFn.apply` IS consulted for it (the typed + * `case agg: ExtendedStatsAggregation` arm runs BEFORE that handler, which is why the handler + * cannot key on the library type). The ES 8 / ES 9 client modules render it with its script. + * - the one-argument builders (elastic4s 6.x / 7.x, and the 8.x/9.x default handler) throw a + * `NotImplementedError` on it -- the request can never leave as silently-wrong JSON. + * [[SearchBodySerializer.Default]] refuses it earlier, with a named message; the ES 6 / ES 7 + * client modules refuse it with an `ElasticError` naming their major. + * + * `inner` is exactly the aggregation the default builder would have received (name, field, script, + * sigma, missing, sub-aggregations, metadata), so a rendering handler stays in parity with it. + */ +final case class ScriptedExtendedStatsAggregation(inner: ExtendedStatsAggregation) + extends Aggregation { + + require( + inner.script.isDefined, + "ScriptedExtendedStatsAggregation wraps an extended_stats that carries a script" + ) + + type T = ScriptedExtendedStatsAggregation + + override def name: String = inner.name + + override def metadata: Map[String, AnyRef] = inner.metadata + + override def subaggs: Seq[AbstractAggregation] = inner.subaggs + + override def subAggregations(aggs: Iterable[AbstractAggregation]): T = + copy(inner = inner.subAggregations(aggs)) + + override def metadata(map: Map[String, AnyRef]): T = copy(inner = inner.metadata(map)) +} + +object ScriptedExtendedStatsAggregation { + + /** True when `aggs`, or any aggregation nested below them, is a + * [[ScriptedExtendedStatsAggregation]] -- the shape discriminator behind + * `hasTransformExtendedStats` (issue #222). Both binds are covered by construction: the plain + * `STDDEV(f(x))` metric and the windowed `STDDEV(f(x)) OVER (PARTITION BY ...)` metric are the + * same marker, one at the root and one under a partition bucket. + */ + def existsIn(aggs: Iterable[AbstractAggregation]): Boolean = + aggs.exists { + case _: ScriptedExtendedStatsAggregation => true + case a: Aggregation => existsIn(a.subaggs) + case _ => false + } +} diff --git a/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/SearchBodySerializer.scala b/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/SearchBodySerializer.scala new file mode 100644 index 000000000..f541e4814 --- /dev/null +++ b/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/SearchBodySerializer.scala @@ -0,0 +1,73 @@ +/* + * 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.bridge + +import com.sksamuel.elastic4s.requests.searches.{SearchBodyBuilderFn, SearchRequest} + +/** The ONE place a bridge `SearchRequest` becomes a JSON body (issue #222). + * + * The bridge is a shared template: `copyBridge` copies these sources byte-for-byte into the ES 7, + * ES 8 and ES 9 modules, which compile them against elastic4s 7.17.x (one-argument + * `SearchBodyBuilderFn`) and 8.x / 9.x (two-argument, with a `customAggregation` handler). Anything + * a single major can do therefore lives in that major's CLIENT module and is injected here: the + * client module puts an implicit `SearchBodySerializer` in scope of the `SingleSearch` conversions + * (`requestToElasticSearchRequest`, `sqlQueryToAggregations`), and every serialisation door + * consumes it. Without one, [[SearchBodySerializer.Default]] applies. + */ +trait SearchBodySerializer { + + /** The JSON body of `search`, exactly as the transport sends it. */ + def serialize(search: SearchRequest): String +} + +object SearchBodySerializer { + + /** True when the request carries an `extended_stats` over a transformed expression -- any + * [[ScriptedExtendedStatsAggregation]] anywhere in its aggregation tree (plain or windowed bind). + */ + def hasTransformExtendedStats(search: SearchRequest): Boolean = + ScriptedExtendedStatsAggregation.existsIn(search.aggs) + + /** The version-agnostic refusal the Default serializer raises (issue #222). */ + val TransformExtendedStatsUnsupported: String = + "STDDEV/VARIANCE over a transformed expression cannot be serialised by the default " + + "Elasticsearch body builder: the underlying elastic4s builder drops the aggregation script " + + "(elastic4s#4100), so the statistic would silently be computed over the raw field. " + + "Aggregate over a raw field, or use Elasticsearch 8+." + + /** The refusal an ES-major-aware client module raises for the same shape, naming its major. */ + def transformExtendedStatsUnsupportedOn(major: Int): String = + s"STDDEV/VARIANCE over a transformed expression is not supported on Elasticsearch $major: " + + "the underlying elastic4s builder drops the aggregation script (elastic4s#4100), so the " + + "statistic would silently be computed over the raw field. Aggregate over a raw field, or use " + + "Elasticsearch 8+." + + /** Today's behaviour -- the one-argument `SearchBodyBuilderFn` every elastic4s line offers -- + * guarded against the one shape it cannot render honestly. A transform-bearing extended_stats + * has no script-emitting builder on this path, so the request is REFUSED, loudly and by name, + * instead of leaving as the statistic of the wrong field. (Left to elastic4s, the marker would + * still fail -- `AggregationBuilderFn`'s `case ni => throw new NotImplementedError(...)` -- but + * with a message that says nothing about the statistic.) + */ + object Default extends SearchBodySerializer { + override def serialize(search: SearchRequest): String = { + if (hasTransformExtendedStats(search)) + throw new UnsupportedOperationException(TransformExtendedStatsUnsupported) + SearchBodyBuilderFn(search).string + } + } +} diff --git a/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/package.scala b/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/package.scala index 63fc7d0d4..b720d45c2 100644 --- a/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/package.scala +++ b/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/package.scala @@ -438,9 +438,14 @@ package object bridge { } } + // `serializer` is the seam of issue #222: an ES-major-aware client module puts its own + // SearchBodySerializer in implicit scope of this conversion; with none in scope the default + // argument applies (the one-argument elastic4s builder, refusing a transform-bearing + // extended_stats). Same defaulted-implicit pattern as `contextType`. implicit def requestToElasticSearchRequest(request: SingleSearch)(implicit timestamp: Long, - contextType: PainlessContextType = PainlessContextType.Query + contextType: PainlessContextType = PainlessContextType.Query, + serializer: SearchBodySerializer = SearchBodySerializer.Default ): ElasticSearchRequest = ElasticSearchRequest( request.sql, @@ -453,7 +458,8 @@ package object bridge { request, request.buckets, request.having.flatMap(_.criteria), - request.orderBy.map(_.sorts).getOrElse(Seq.empty) + request.orderBy.map(_.sorts).getOrElse(Seq.empty), + serializer = serializer ).minScore(request.score) /** Merge percentile ElasticAggregations that share a value column / `cont` flag / partition into @@ -1087,6 +1093,8 @@ package object bridge { implicit def queryToJson( query: Query ): JsonNode = { + // Query-only body (no aggregations): audited exempt from the SearchBodySerializer seam + // (issue #222) -- there is no extended_stats here for a serializer to render or refuse. JacksonBuilder.toNode( SearchBodyBuilderFn( ElasticApi.search("") query { @@ -1124,11 +1132,14 @@ package object bridge { ElasticBridge(filter) } + // The second serialisation door (issue #222): each aggregation's own single-aggregation body is + // rendered through the same injected SearchBodySerializer as ElasticSearchRequest.query. implicit def sqlQueryToAggregations( query: SelectStatement )(implicit timestamp: Long, - contextType: PainlessContextType = PainlessContextType.Query + contextType: PainlessContextType = PainlessContextType.Query, + serializer: SearchBodySerializer = SearchBodySerializer.Default ): Seq[ElasticAggregation] = { import query._ statement @@ -1143,35 +1154,32 @@ package object bridge { .flatMap(_.criteria.map(ElasticCriteria(_).asQuery())) .getOrElse(matchAllQuery()) + val body: SearchRequest = + aggregation.aggType match { + case COUNT if aggregation.sourceField.equalsIgnoreCase("_id") => + ElasticApi.search("") query { + queryFiltered + } + case _ => + ElasticApi.search("") query { + queryFiltered + } aggregations { + val filtered = + filteredAgg match { + case Some(filtered) => filtered.subAggregations(aggregation.agg) + case _ => aggregation.agg + } + aggregation.nestedAgg match { + case Some(nested) => nested.subAggregations(filtered) + case _ => filtered + } + } size 0 + } + aggregation.copy( sources = l.sources, query = Some( - (aggregation.aggType match { - case COUNT if aggregation.sourceField.equalsIgnoreCase("_id") => - SearchBodyBuilderFn( - ElasticApi.search("") query { - queryFiltered - } - ) - case _ => - SearchBodyBuilderFn( - ElasticApi.search("") query { - queryFiltered - } - aggregations { - val filtered = - filteredAgg match { - case Some(filtered) => filtered.subAggregations(aggregation.agg) - case _ => aggregation.agg - } - aggregation.nestedAgg match { - case Some(nested) => nested.subAggregations(filtered) - case _ => filtered - } - } - size 0 - ) - }).string.replace("\"version\":true,", "") /*FIXME*/ + serializer.serialize(body).replace("\"version\":true,", "") /*FIXME*/ ) ) }) diff --git a/core/src/main/scala/app/softnetwork/elastic/client/GatewayApi.scala b/core/src/main/scala/app/softnetwork/elastic/client/GatewayApi.scala index e4c8d0adf..63e09618f 100644 --- a/core/src/main/scala/app/softnetwork/elastic/client/GatewayApi.scala +++ b/core/src/main/scala/app/softnetwork/elastic/client/GatewayApi.scala @@ -116,6 +116,29 @@ class SearchExecutor(api: ScrollApi with SearchApi, logger: Logger) implicit val context: ConversionContext = NativeContext + // The SQL -> Elasticsearch translation runs SYNCHRONOUSLY inside `searchAsync` / `scroll`, + // before any Future exists. A client module may REFUSE a statement there by throwing a + // status-bearing `ElasticError` (issue #222: STDDEV / VARIANCE over a transformed expression on + // ES 6 / ES 7, where the library cannot emit the aggregation script); this boundary turns that + // deliberate refusal into the `ElasticFailure` every other DQL error is, so BI tools see an + // honest 400 instead of a raw exception. Anything else escaping translation keeps its current + // (thrown) route -- a totality boundary for the whole translation layer is a separate change. + try dispatch(statement) + catch { + case refusal: ElasticError => + logger.error(s"❌ ${refusal.message}") + Future.successful(ElasticFailure(refusal.copy(operation = Some("dql")))) + } + } + + private def dispatch( + statement: SearchStatement + )(implicit + system: ActorSystem, + ec: ExecutionContext, + context: ConversionContext + ): Future[ElasticResult[QueryResult]] = { + statement match { // ============================ diff --git a/es6/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/ElasticAggregation.scala b/es6/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/ElasticAggregation.scala index 41ccf34e1..aa2f61b30 100644 --- a/es6/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/ElasticAggregation.scala +++ b/es6/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/ElasticAggregation.scala @@ -89,11 +89,18 @@ case class ElasticAggregation( // CHECK if it is a "global" metric (cardinality, etc.) or a bucket metric (avg, sum, etc.) val isGlobalMetric: Boolean = agg match { - case _: CardinalityAggregation => true - case _: StatsAggregation => true - case _: ExtendedStatsAggregation => true - case _ => false + case _: CardinalityAggregation => true + case _: StatsAggregation => true + case _: ExtendedStatsAggregation => true + case _: ScriptedExtendedStatsAggregation => true + case _ => false } + + /** True when this aggregation is `STDDEV` / `VARIANCE` (any of the `extended_stats` family) over + * a TRANSFORMED expression -- the shape a client module must either render with its script or + * refuse (issue #222). The second serialisation door (`sqlQueryToAggregations`) reads it. + */ + def hasTransformExtendedStats: Boolean = ScriptedExtendedStatsAggregation.existsIn(Seq(agg)) } object ElasticAggregation { @@ -197,7 +204,10 @@ object ElasticAggregation { case STDDEV | STDDEV_SAMP | STDDEV_POP | VARIANCE | VAR_SAMP | VAR_POP => aggWithFieldOrScript( extendedStatsAgg, - (name, s) => extendedStatsAgg(name, sourceField).script(s) + // Issue #222 -- a transform-bearing extended_stats is bound to the bridge's own marker: + // elastic4s's ExtendedStatsAggregationBuilder drops `script`, so the library type + // would serialise as the statistic of the raw field. See ScriptedExtendedStatsAggregation. + (name, s) => ScriptedExtendedStatsAggregation(extendedStatsAgg(name, sourceField).script(s)) ) case th: WindowFunction => th.window match { @@ -223,7 +233,10 @@ object ElasticAggregation { case STDDEV | STDDEV_SAMP | STDDEV_POP | VARIANCE | VAR_SAMP | VAR_POP => aggWithFieldOrScript( extendedStatsAgg, - (name, s) => extendedStatsAgg(name, sourceField).script(s) + // Issue #222 -- a transform-bearing extended_stats is bound to the bridge's own marker: + // elastic4s's ExtendedStatsAggregationBuilder drops `script`, so the library type + // would serialise as the statistic of the raw field. See ScriptedExtendedStatsAggregation. + (name, s) => ScriptedExtendedStatsAggregation(extendedStatsAgg(name, sourceField).script(s)) ) case PERCENTILE_CONT | PERCENTILE_DISC => // Both map to ES `percentiles` (TDigest). One call → one percent; diff --git a/es6/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/ElasticMultiSearchRequest.scala b/es6/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/ElasticMultiSearchRequest.scala index 0a904a6d7..13762f6c2 100644 --- a/es6/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/ElasticMultiSearchRequest.scala +++ b/es6/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/ElasticMultiSearchRequest.scala @@ -23,5 +23,11 @@ case class ElasticMultiSearchRequest( requests: Seq[ElasticSearchRequest], multiSearch: MultiSearchRequest ) { + // Not routed through SearchBodySerializer (issue #222): no production path serialises a + // multi-search here -- core builds `_msearch` bodies from each request's own + // `singleSearchToJsonQuery` (`ElasticQueries.multiQuery`). A transform-bearing extended_stats + // inside this body still fails LOUDLY (elastic4s's aggregation builder throws + // `NotImplementedError` on the ScriptedExtendedStatsAggregation marker); it can never leave as + // the statistic of the wrong field. def query: String = MultiSearchBuilderFn(multiSearch).replace("\"version\":true,", "") /*FIXME*/ } diff --git a/es6/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/ElasticSearchRequest.scala b/es6/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/ElasticSearchRequest.scala index ff2463e84..6782773f9 100644 --- a/es6/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/ElasticSearchRequest.scala +++ b/es6/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/ElasticSearchRequest.scala @@ -16,9 +16,8 @@ package app.softnetwork.elastic.sql.bridge -import app.softnetwork.elastic.sql.query.{Bucket, Criteria, Except, Field, FieldSort, Limit} +import app.softnetwork.elastic.sql.query.{Bucket, Criteria, Except, Field, FieldSort} import com.sksamuel.elastic4s.searches.SearchRequest -import com.sksamuel.elastic4s.http.search.SearchBodyBuilderFn case class ElasticSearchRequest( sql: String, @@ -31,7 +30,10 @@ case class ElasticSearchRequest( search: SearchRequest, buckets: Seq[Bucket] = Seq.empty, having: Option[Criteria] = None, - sorts: Seq[FieldSort] = Seq.empty + sorts: Seq[FieldSort] = Seq.empty, + // The body serializer the client module injected through the SingleSearch conversion (issue + // #222); Default = the one-argument elastic4s builder, refusing a transform-bearing extended_stats. + serializer: SearchBodySerializer = SearchBodySerializer.Default ) { def minScore(score: Option[Double]): ElasticSearchRequest = { score match { @@ -40,6 +42,12 @@ case class ElasticSearchRequest( } } + /** True when this request carries `STDDEV` / `VARIANCE` (any of the `extended_stats` family) over + * a TRANSFORMED expression -- plain (`STDDEV(YEAR(x))`) or windowed (`... OVER (PARTITION BY + * ...)`). The shape a client module must either render with its script or refuse (issue #222). + */ + def hasTransformExtendedStats: Boolean = SearchBodySerializer.hasTransformExtendedStats(search) + def query: String = - SearchBodyBuilderFn(search).string.replace("\"version\":true,", "") /*FIXME*/ + serializer.serialize(search).replace("\"version\":true,", "") /*FIXME*/ } diff --git a/es6/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/ScriptedExtendedStatsAggregation.scala b/es6/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/ScriptedExtendedStatsAggregation.scala new file mode 100644 index 000000000..d6a89da54 --- /dev/null +++ b/es6/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/ScriptedExtendedStatsAggregation.scala @@ -0,0 +1,75 @@ +/* + * 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.bridge + +import com.sksamuel.elastic4s.searches.aggs.{ + AbstractAggregation, + Aggregation, + ExtendedStatsAggregation +} + +/** An `extended_stats` aggregation over a TRANSFORMED expression -- `STDDEV(YEAR(createdAt))`, + * `VARIANCE(ABS(salary))` and the rest of the family (issue #222). Hand-maintained ES 6 twin of the + * bridge template's marker (elastic4s 6.7.8 package names; same contract). + * + * elastic4s's own `ExtendedStatsAggregationBuilder` never emits `agg.script`, so an + * `ExtendedStatsAggregation` carrying a script silently serialises as the statistic of the RAW + * field -- or as `extended_stats: {}` when there is no raw field to fall back on. The bridge + * therefore binds a transform-bearing extended_stats to this marker instead of to the library type + * it wraps. The 6.x line has no script-emitting builder and no customisation seam, so the marker + * is never rendered here: [[SearchBodySerializer.Default]] refuses it with a named message, and the + * ES 6 client modules (REST and Jest) refuse it with an `ElasticError` naming their major. Left to + * elastic4s, it would still fail loudly (`AggregationBuilderFn`'s `NotImplementedError`) -- it can + * never leave as silently-wrong JSON. + */ +final case class ScriptedExtendedStatsAggregation(inner: ExtendedStatsAggregation) + extends Aggregation { + + require( + inner.script.isDefined, + "ScriptedExtendedStatsAggregation wraps an extended_stats that carries a script" + ) + + type T = ScriptedExtendedStatsAggregation + + override def name: String = inner.name + + override def metadata: Map[String, AnyRef] = inner.metadata + + override def subaggs: Seq[AbstractAggregation] = inner.subaggs + + override def subAggregations(aggs: Iterable[AbstractAggregation]): T = + copy(inner = inner.subAggregations(aggs)) + + override def metadata(map: Map[String, AnyRef]): T = copy(inner = inner.metadata(map)) +} + +object ScriptedExtendedStatsAggregation { + + /** True when `aggs`, or any aggregation nested below them, is a + * [[ScriptedExtendedStatsAggregation]] -- the shape discriminator behind + * `hasTransformExtendedStats` (issue #222). Both binds are covered by construction: the plain + * `STDDEV(f(x))` metric and the windowed `STDDEV(f(x)) OVER (PARTITION BY ...)` metric are the + * same marker, one at the root and one under a partition bucket. + */ + def existsIn(aggs: Iterable[AbstractAggregation]): Boolean = + aggs.exists { + case _: ScriptedExtendedStatsAggregation => true + case a: Aggregation => existsIn(a.subaggs) + case _ => false + } +} diff --git a/es6/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/SearchBodySerializer.scala b/es6/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/SearchBodySerializer.scala new file mode 100644 index 000000000..1d999e086 --- /dev/null +++ b/es6/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/SearchBodySerializer.scala @@ -0,0 +1,69 @@ +/* + * 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.bridge + +import com.sksamuel.elastic4s.http.search.SearchBodyBuilderFn +import com.sksamuel.elastic4s.searches.SearchRequest + +/** The ONE place a bridge `SearchRequest` becomes a JSON body (issue #222). Hand-maintained ES 6 + * twin of the bridge template's seam (elastic4s 6.7.8 package names; same contract). + * + * The ES 6 client modules (REST and Jest) put an implicit `SearchBodySerializer` in scope of the + * `SingleSearch` conversions (`requestToElasticSearchRequest`, `sqlQueryToAggregations`), and every + * serialisation door consumes it. Without one, [[SearchBodySerializer.Default]] applies. + */ +trait SearchBodySerializer { + + /** The JSON body of `search`, exactly as the transport sends it. */ + def serialize(search: SearchRequest): String +} + +object SearchBodySerializer { + + /** True when the request carries an `extended_stats` over a transformed expression -- any + * [[ScriptedExtendedStatsAggregation]] anywhere in its aggregation tree (plain or windowed bind). + */ + def hasTransformExtendedStats(search: SearchRequest): Boolean = + ScriptedExtendedStatsAggregation.existsIn(search.aggs) + + /** The version-agnostic refusal the Default serializer raises (issue #222). */ + val TransformExtendedStatsUnsupported: String = + "STDDEV/VARIANCE over a transformed expression cannot be serialised by the default " + + "Elasticsearch body builder: the underlying elastic4s builder drops the aggregation script " + + "(elastic4s#4100), so the statistic would silently be computed over the raw field. " + + "Aggregate over a raw field, or use Elasticsearch 8+." + + /** The refusal an ES-major-aware client module raises for the same shape, naming its major. */ + def transformExtendedStatsUnsupportedOn(major: Int): String = + s"STDDEV/VARIANCE over a transformed expression is not supported on Elasticsearch $major: " + + "the underlying elastic4s builder drops the aggregation script (elastic4s#4100), so the " + + "statistic would silently be computed over the raw field. Aggregate over a raw field, or use " + + "Elasticsearch 8+." + + /** Today's behaviour -- the one-argument `SearchBodyBuilderFn` -- guarded against the one shape + * it cannot render honestly. A transform-bearing extended_stats has no script-emitting builder + * on this line, so the request is REFUSED, loudly and by name, instead of leaving as the + * statistic of the wrong field. + */ + object Default extends SearchBodySerializer { + override def serialize(search: SearchRequest): String = { + if (hasTransformExtendedStats(search)) + throw new UnsupportedOperationException(TransformExtendedStatsUnsupported) + SearchBodyBuilderFn(search).string() + } + } +} diff --git a/es6/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/package.scala b/es6/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/package.scala index 38e3e4a9c..050100a88 100644 --- a/es6/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/package.scala +++ b/es6/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/package.scala @@ -434,9 +434,14 @@ package object bridge { } } + // `serializer` is the seam of issue #222: an ES-major-aware client module puts its own + // SearchBodySerializer in implicit scope of this conversion; with none in scope the default + // argument applies (the one-argument elastic4s builder, refusing a transform-bearing + // extended_stats). Same defaulted-implicit pattern as `contextType`. implicit def requestToElasticSearchRequest(request: SingleSearch)(implicit timestamp: Long, - contextType: PainlessContextType = PainlessContextType.Query + contextType: PainlessContextType = PainlessContextType.Query, + serializer: SearchBodySerializer = SearchBodySerializer.Default ): ElasticSearchRequest = ElasticSearchRequest( request.sql, @@ -449,7 +454,8 @@ package object bridge { request, request.buckets, request.having.flatMap(_.criteria), - request.orderBy.map(_.sorts).getOrElse(Seq.empty) + request.orderBy.map(_.sorts).getOrElse(Seq.empty), + serializer = serializer ).minScore(request.score) /** Merge percentile ElasticAggregations that share a value column / `cont` flag / partition into @@ -1084,6 +1090,8 @@ package object bridge { implicit def queryToString( query: Query ): String = { + // Query-only body (no aggregations): audited exempt from the SearchBodySerializer seam + // (issue #222) -- there is no extended_stats here for a serializer to render or refuse. SearchBodyBuilderFn( ElasticApi.search("") query { query @@ -1125,12 +1133,15 @@ package object bridge { ElasticBridge(filter) } + // The second serialisation door (issue #222): each aggregation's own single-aggregation body is + // rendered through the same injected SearchBodySerializer as ElasticSearchRequest.query. @deprecated implicit def sqlQueryToAggregations( query: SelectStatement )(implicit timestamp: Long, - contextType: PainlessContextType = PainlessContextType.Query + contextType: PainlessContextType = PainlessContextType.Query, + serializer: SearchBodySerializer = SearchBodySerializer.Default ): Seq[ElasticAggregation] = { import query._ statement @@ -1145,35 +1156,32 @@ package object bridge { .flatMap(_.criteria.map(ElasticCriteria(_).asQuery())) .getOrElse(matchAllQuery()) + val body: SearchRequest = + aggregation.aggType match { + case COUNT if aggregation.sourceField.equalsIgnoreCase("_id") => + ElasticApi.search("") query { + queryFiltered + } + case _ => + ElasticApi.search("") query { + queryFiltered + } aggregations { + val filtered = + filteredAgg match { + case Some(filtered) => filtered.subAggregations(aggregation.agg) + case _ => aggregation.agg + } + aggregation.nestedAgg match { + case Some(nested) => nested.subAggregations(filtered) + case _ => filtered + } + } size 0 + } + aggregation.copy( sources = l.sources, query = Some( - (aggregation.aggType match { - case COUNT if aggregation.sourceField.equalsIgnoreCase("_id") => - SearchBodyBuilderFn( - ElasticApi.search("") query { - queryFiltered - } - ) - case _ => - SearchBodyBuilderFn( - ElasticApi.search("") query { - queryFiltered - } - aggregations { - val filtered = - filteredAgg match { - case Some(filtered) => filtered.subAggregations(aggregation.agg) - case _ => aggregation.agg - } - aggregation.nestedAgg match { - case Some(nested) => nested.subAggregations(filtered) - case _ => filtered - } - } - size 0 - ) - }).string().replace("\"version\":true,", "") /*FIXME*/ + serializer.serialize(body).replace("\"version\":true,", "") /*FIXME*/ ) ) }) diff --git a/es6/jest/src/main/scala/app/softnetwork/elastic/client/jest/JestSearchApi.scala b/es6/jest/src/main/scala/app/softnetwork/elastic/client/jest/JestSearchApi.scala index 5a34f2826..5fdb8ecc4 100644 --- a/es6/jest/src/main/scala/app/softnetwork/elastic/client/jest/JestSearchApi.scala +++ b/es6/jest/src/main/scala/app/softnetwork/elastic/client/jest/JestSearchApi.scala @@ -26,7 +26,7 @@ import app.softnetwork.elastic.client.{ import com.fasterxml.jackson.databind.JsonNode import app.softnetwork.elastic.client.result.ElasticResult import app.softnetwork.elastic.sql.PainlessContextType -import app.softnetwork.elastic.sql.bridge.ElasticSearchRequest +import app.softnetwork.elastic.sql.bridge.{ElasticSearchRequest, SearchBodySerializer} import app.softnetwork.elastic.sql.query.SingleSearch import io.searchbox.core.{MultiSearch, Search, SearchResult} import org.json4s.Formats @@ -63,6 +63,14 @@ trait JestSearchApi extends SearchApi with JestClientHelpers { } } + /** Issue #222 -- the search-body serializer every `SingleSearch` conversion made from this trait + * picks up (implicit scope of the bridge's `requestToElasticSearchRequest` / + * `sqlQueryToAggregations`): elastic4s 6.x cannot render an `extended_stats` over a transformed + * expression with its script, so the request is REFUSED with a named `ElasticError` before any + * JSON exists -- never executed against the raw field. See [[JestSearchBodySerializer]]. + */ + implicit def searchBodySerializer: SearchBodySerializer = JestSearchBodySerializer + private[client] implicit def singleSearchToJsonQuery( sqlSearch: SingleSearch )(implicit diff --git a/es6/jest/src/main/scala/app/softnetwork/elastic/client/jest/JestSearchBodySerializer.scala b/es6/jest/src/main/scala/app/softnetwork/elastic/client/jest/JestSearchBodySerializer.scala new file mode 100644 index 000000000..a54b83cc8 --- /dev/null +++ b/es6/jest/src/main/scala/app/softnetwork/elastic/client/jest/JestSearchBodySerializer.scala @@ -0,0 +1,51 @@ +/* + * 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.jest + +import app.softnetwork.elastic.client.result.ElasticError +import app.softnetwork.elastic.sql.bridge.SearchBodySerializer +import com.sksamuel.elastic4s.searches.SearchRequest + +/** The ES 6 (Jest) search-body serializer (issue #222): the default one-argument elastic4s builder, with a + * LOUD refusal of `STDDEV` / `VARIANCE` (any of the `extended_stats` family) over a transformed + * expression -- plain or windowed -- BEFORE any JSON exists. + * + * elastic4s 6.7.8 has neither a script-emitting `ExtendedStatsAggregationBuilder` nor the + * `customAggregation` seam the ES 8 / ES 9 modules use to render one, so this module cannot + * compute the statistic over the transform; it used to compute it silently over the RAW field. + * The refusal is an `ElasticError` with status 400 so it reaches the caller as an honest failure. + * The 6.x elastic4s line is unmaintained: there is no upstream path (spec R-1b), so unlike the + * ES 7 module this refusal is permanent. + */ +object JestSearchBodySerializer extends SearchBodySerializer { + + val ElasticsearchMajor: Int = 6 + + /** The named error a transform-bearing extended_stats is refused with on this module. */ + val TransformExtendedStatsUnsupported: String = + SearchBodySerializer.transformExtendedStatsUnsupportedOn(ElasticsearchMajor) + + override def serialize(search: SearchRequest): String = { + if (SearchBodySerializer.hasTransformExtendedStats(search)) + throw ElasticError( + message = TransformExtendedStatsUnsupported, + statusCode = Some(400), + operation = Some("search") + ) + SearchBodySerializer.Default.serialize(search) + } +} diff --git a/es6/rest/src/main/scala/app/softnetwork/elastic/client/rest/RestHighLevelClientApi.scala b/es6/rest/src/main/scala/app/softnetwork/elastic/client/rest/RestHighLevelClientApi.scala index a5ffc81fa..7031b951c 100644 --- a/es6/rest/src/main/scala/app/softnetwork/elastic/client/rest/RestHighLevelClientApi.scala +++ b/es6/rest/src/main/scala/app/softnetwork/elastic/client/rest/RestHighLevelClientApi.scala @@ -1043,6 +1043,15 @@ trait RestHighLevelClientGetApi extends GetApi with RestHighLevelClientHelpers { trait RestHighLevelClientSearchApi extends SearchApi with RestHighLevelClientHelpers { _: ElasticConversion with RestHighLevelClientCompanion => + /** Issue #222 -- the search-body serializer every `SingleSearch` conversion made from this trait + * picks up (implicit scope of the bridge's `requestToElasticSearchRequest` / + * `sqlQueryToAggregations`): elastic4s 6.x cannot render an `extended_stats` over a transformed + * expression with its script, so the request is REFUSED with a named `ElasticError` before any + * JSON exists -- never executed against the raw field. See + * [[RestHighLevelClientSearchBodySerializer]]. + */ + implicit def searchBodySerializer: SearchBodySerializer = RestHighLevelClientSearchBodySerializer + override implicit def singleSearchToJsonQuery(sqlSearch: SingleSearch)(implicit timestamp: Long, contextType: PainlessContextType = PainlessContextType.Query diff --git a/es6/rest/src/main/scala/app/softnetwork/elastic/client/rest/RestHighLevelClientSearchBodySerializer.scala b/es6/rest/src/main/scala/app/softnetwork/elastic/client/rest/RestHighLevelClientSearchBodySerializer.scala new file mode 100644 index 000000000..c82b0a334 --- /dev/null +++ b/es6/rest/src/main/scala/app/softnetwork/elastic/client/rest/RestHighLevelClientSearchBodySerializer.scala @@ -0,0 +1,51 @@ +/* + * 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.rest + +import app.softnetwork.elastic.client.result.ElasticError +import app.softnetwork.elastic.sql.bridge.SearchBodySerializer +import com.sksamuel.elastic4s.searches.SearchRequest + +/** The ES 6 (REST) search-body serializer (issue #222): the default one-argument elastic4s builder, with a + * LOUD refusal of `STDDEV` / `VARIANCE` (any of the `extended_stats` family) over a transformed + * expression -- plain or windowed -- BEFORE any JSON exists. + * + * elastic4s 6.7.8 has neither a script-emitting `ExtendedStatsAggregationBuilder` nor the + * `customAggregation` seam the ES 8 / ES 9 modules use to render one, so this module cannot + * compute the statistic over the transform; it used to compute it silently over the RAW field. + * The refusal is an `ElasticError` with status 400 so it reaches the caller as an honest failure. + * The 6.x elastic4s line is unmaintained: there is no upstream path (spec R-1b), so unlike the + * ES 7 module this refusal is permanent. + */ +object RestHighLevelClientSearchBodySerializer extends SearchBodySerializer { + + val ElasticsearchMajor: Int = 6 + + /** The named error a transform-bearing extended_stats is refused with on this module. */ + val TransformExtendedStatsUnsupported: String = + SearchBodySerializer.transformExtendedStatsUnsupportedOn(ElasticsearchMajor) + + override def serialize(search: SearchRequest): String = { + if (SearchBodySerializer.hasTransformExtendedStats(search)) + throw ElasticError( + message = TransformExtendedStatsUnsupported, + statusCode = Some(400), + operation = Some("search") + ) + SearchBodySerializer.Default.serialize(search) + } +} diff --git a/es7/rest/src/main/scala/app/softnetwork/elastic/client/rest/RestHighLevelClientApi.scala b/es7/rest/src/main/scala/app/softnetwork/elastic/client/rest/RestHighLevelClientApi.scala index b4a25a5e9..4a97abe61 100644 --- a/es7/rest/src/main/scala/app/softnetwork/elastic/client/rest/RestHighLevelClientApi.scala +++ b/es7/rest/src/main/scala/app/softnetwork/elastic/client/rest/RestHighLevelClientApi.scala @@ -1091,6 +1091,15 @@ trait RestHighLevelClientGetApi extends GetApi with RestHighLevelClientHelpers { trait RestHighLevelClientSearchApi extends SearchApi with RestHighLevelClientHelpers { _: ElasticConversion with RestHighLevelClientCompanion => + /** Issue #222 -- the search-body serializer every `SingleSearch` conversion made from this trait + * picks up (implicit scope of the bridge's `requestToElasticSearchRequest` / + * `sqlQueryToAggregations`): elastic4s 7.17.x cannot render an `extended_stats` over a + * transformed expression with its script, so the request is REFUSED with a named `ElasticError` + * before any JSON exists -- never executed against the raw field. See + * [[RestHighLevelClientSearchBodySerializer]]. + */ + implicit def searchBodySerializer: SearchBodySerializer = RestHighLevelClientSearchBodySerializer + override implicit def singleSearchToJsonQuery(singleSearch: SingleSearch)(implicit timestamp: Long, contextType: PainlessContextType = PainlessContextType.Query diff --git a/es7/rest/src/main/scala/app/softnetwork/elastic/client/rest/RestHighLevelClientSearchBodySerializer.scala b/es7/rest/src/main/scala/app/softnetwork/elastic/client/rest/RestHighLevelClientSearchBodySerializer.scala new file mode 100644 index 000000000..6b1b34c86 --- /dev/null +++ b/es7/rest/src/main/scala/app/softnetwork/elastic/client/rest/RestHighLevelClientSearchBodySerializer.scala @@ -0,0 +1,51 @@ +/* + * 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.rest + +import app.softnetwork.elastic.client.result.ElasticError +import app.softnetwork.elastic.sql.bridge.SearchBodySerializer +import com.sksamuel.elastic4s.requests.searches.SearchRequest + +/** The ES 7 search-body serializer (issue #222): the default one-argument elastic4s builder, with a + * LOUD refusal of `STDDEV` / `VARIANCE` (any of the `extended_stats` family) over a transformed + * expression -- plain or windowed -- BEFORE any JSON exists. + * + * elastic4s 7.17.x has neither a script-emitting `ExtendedStatsAggregationBuilder` nor the + * `customAggregation` seam the ES 8 / ES 9 modules use to render one, so this module cannot + * compute the statistic over the transform; it used to compute it silently over the RAW field. + * The refusal is an `ElasticError` with status 400 so it reaches the caller as an honest failure. + * Watch item (spec AD-S3-3): when the elastic4s#4100 backport lands on the 7.17 line and + * `Versions.elastic74s` moves past it, replace the refusal with a rendering handler. + */ +object RestHighLevelClientSearchBodySerializer extends SearchBodySerializer { + + val ElasticsearchMajor: Int = 7 + + /** The named error a transform-bearing extended_stats is refused with on this module. */ + val TransformExtendedStatsUnsupported: String = + SearchBodySerializer.transformExtendedStatsUnsupportedOn(ElasticsearchMajor) + + override def serialize(search: SearchRequest): String = { + if (SearchBodySerializer.hasTransformExtendedStats(search)) + throw ElasticError( + message = TransformExtendedStatsUnsupported, + statusCode = Some(400), + operation = Some("search") + ) + SearchBodySerializer.Default.serialize(search) + } +} 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..a4219d2a7 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 @@ -1016,6 +1016,14 @@ trait JavaClientGetApi extends GetApi with JavaClientHelpers { trait JavaClientSearchApi extends SearchApi with JavaClientHelpers { _: JavaClientCompanion => + /** Issue #222 -- the search-body serializer every `SingleSearch` conversion made from this trait + * picks up (implicit scope of the bridge's `requestToElasticSearchRequest` / + * `sqlQueryToAggregations`): elastic4s's two-argument builder with the handler that renders an + * `extended_stats` over a transformed expression WITH its script. See + * [[JavaClientSearchBodySerializer]]. + */ + implicit def searchBodySerializer: SearchBodySerializer = JavaClientSearchBodySerializer + override implicit def singleSearchToJsonQuery(singleSearch: SingleSearch)(implicit timestamp: Long, contextType: PainlessContextType = PainlessContextType.Query diff --git a/es8/java/src/main/scala/app/softnetwork/elastic/client/java/JavaClientSearchBodySerializer.scala b/es8/java/src/main/scala/app/softnetwork/elastic/client/java/JavaClientSearchBodySerializer.scala new file mode 100644 index 000000000..2af7b5f5c --- /dev/null +++ b/es8/java/src/main/scala/app/softnetwork/elastic/client/java/JavaClientSearchBodySerializer.scala @@ -0,0 +1,76 @@ +/* + * 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.java + +import app.softnetwork.elastic.sql.bridge.{ScriptedExtendedStatsAggregation, SearchBodySerializer} +import com.sksamuel.elastic4s.handlers.script.ScriptBuilderFn +import com.sksamuel.elastic4s.json.{XContentBuilder, XContentFactory} +import com.sksamuel.elastic4s.requests.searches.aggs.{ + AbstractAggregation, + AggMetaDataFn, + SubAggsBuilderFn +} +import com.sksamuel.elastic4s.requests.searches.{SearchBodyBuilderFn, SearchRequest} + +/** The ES 8 / ES 9 search-body serializer (issue #222): elastic4s's two-argument + * `SearchBodyBuilderFn.apply(request, customAggregation)` with a handler for the bridge's + * [[ScriptedExtendedStatsAggregation]] marker -- an `extended_stats` over a TRANSFORMED expression + * (`STDDEV(YEAR(createdAt))`, `VARIANCE(ABS(salary))`, ...). + * + * elastic4s's own `ExtendedStatsAggregationBuilder` never emits `agg.script` (the only metric + * builder of ten with the omission; fixed upstream once in elastic4s#2700 onto a dead branch, + * re-submitted as elastic4s#4100). Its typed `case agg: ExtendedStatsAggregation` arm in + * `AggregationBuilderFn` also runs BEFORE the custom handler, which is why the bridge binds the + * shape to a marker type the library does not know: the handler below is consulted for exactly + * that type and nothing else. Every other aggregation goes through the library's own builders, so + * this serializer is byte-identical to the one-argument default for any request without a marker. + * + * The handler renders what `ExtendedStatsAggregationBuilder` renders -- `field`, `sigma`, + * `missing` -- PLUS the `script` (through the same `ScriptBuilderFn` the Stats / Max / Avg builders + * use), then the sub-aggregations and metadata. Watch item (spec AD-S3-3): when elastic4s#4100 + * ships and `Versions.elastic84s` / `elastic94s` move past it, the marker can be bound back to the + * library type and this handler deleted. + * + * Kept byte-identical between the ES 8 and ES 9 modules (their elastic4s builders are identical). + */ +object JavaClientSearchBodySerializer extends SearchBodySerializer { + + private val handler: PartialFunction[AbstractAggregation, XContentBuilder] = { + case agg: ScriptedExtendedStatsAggregation => extendedStatsWithScript(agg) + } + + private def extendedStatsWithScript(agg: ScriptedExtendedStatsAggregation): XContentBuilder = { + val inner = agg.inner + val builder = XContentFactory.jsonBuilder() + builder.startObject("extended_stats") + inner.field.foreach(builder.field("field", _)) + inner.sigma.foreach(builder.field("sigma", _)) + inner.missing.foreach(builder.autofield("missing", _)) + // The one line elastic4s's ExtendedStatsAggregationBuilder lacks (elastic4s#4100). + inner.script.foreach(script => builder.rawField("script", ScriptBuilderFn(script))) + builder.endObject() + // Sub-aggregations and metadata are siblings of the aggregation type, as Elasticsearch + // expects them (`"aggs"` / `"meta"` beside `"extended_stats"`). The bridge never attaches either + // to an extended_stats, so the rendered shape stays identical to the default builder's. + SubAggsBuilderFn(inner, builder, handler) + AggMetaDataFn(inner, builder) + builder.endObject() + } + + override def serialize(search: SearchRequest): String = + SearchBodyBuilderFn(search, handler).string +} 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..c252e80ec 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 @@ -1016,6 +1016,14 @@ trait JavaClientGetApi extends GetApi with JavaClientHelpers { trait JavaClientSearchApi extends SearchApi with JavaClientHelpers { _: JavaClientCompanion => + /** Issue #222 -- the search-body serializer every `SingleSearch` conversion made from this trait + * picks up (implicit scope of the bridge's `requestToElasticSearchRequest` / + * `sqlQueryToAggregations`): elastic4s's two-argument builder with the handler that renders an + * `extended_stats` over a transformed expression WITH its script. See + * [[JavaClientSearchBodySerializer]]. + */ + implicit def searchBodySerializer: SearchBodySerializer = JavaClientSearchBodySerializer + override implicit def singleSearchToJsonQuery(singleSearch: SingleSearch)(implicit timestamp: Long, contextType: PainlessContextType = PainlessContextType.Query diff --git a/es9/java/src/main/scala/app/softnetwork/elastic/client/java/JavaClientSearchBodySerializer.scala b/es9/java/src/main/scala/app/softnetwork/elastic/client/java/JavaClientSearchBodySerializer.scala new file mode 100644 index 000000000..2af7b5f5c --- /dev/null +++ b/es9/java/src/main/scala/app/softnetwork/elastic/client/java/JavaClientSearchBodySerializer.scala @@ -0,0 +1,76 @@ +/* + * 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.java + +import app.softnetwork.elastic.sql.bridge.{ScriptedExtendedStatsAggregation, SearchBodySerializer} +import com.sksamuel.elastic4s.handlers.script.ScriptBuilderFn +import com.sksamuel.elastic4s.json.{XContentBuilder, XContentFactory} +import com.sksamuel.elastic4s.requests.searches.aggs.{ + AbstractAggregation, + AggMetaDataFn, + SubAggsBuilderFn +} +import com.sksamuel.elastic4s.requests.searches.{SearchBodyBuilderFn, SearchRequest} + +/** The ES 8 / ES 9 search-body serializer (issue #222): elastic4s's two-argument + * `SearchBodyBuilderFn.apply(request, customAggregation)` with a handler for the bridge's + * [[ScriptedExtendedStatsAggregation]] marker -- an `extended_stats` over a TRANSFORMED expression + * (`STDDEV(YEAR(createdAt))`, `VARIANCE(ABS(salary))`, ...). + * + * elastic4s's own `ExtendedStatsAggregationBuilder` never emits `agg.script` (the only metric + * builder of ten with the omission; fixed upstream once in elastic4s#2700 onto a dead branch, + * re-submitted as elastic4s#4100). Its typed `case agg: ExtendedStatsAggregation` arm in + * `AggregationBuilderFn` also runs BEFORE the custom handler, which is why the bridge binds the + * shape to a marker type the library does not know: the handler below is consulted for exactly + * that type and nothing else. Every other aggregation goes through the library's own builders, so + * this serializer is byte-identical to the one-argument default for any request without a marker. + * + * The handler renders what `ExtendedStatsAggregationBuilder` renders -- `field`, `sigma`, + * `missing` -- PLUS the `script` (through the same `ScriptBuilderFn` the Stats / Max / Avg builders + * use), then the sub-aggregations and metadata. Watch item (spec AD-S3-3): when elastic4s#4100 + * ships and `Versions.elastic84s` / `elastic94s` move past it, the marker can be bound back to the + * library type and this handler deleted. + * + * Kept byte-identical between the ES 8 and ES 9 modules (their elastic4s builders are identical). + */ +object JavaClientSearchBodySerializer extends SearchBodySerializer { + + private val handler: PartialFunction[AbstractAggregation, XContentBuilder] = { + case agg: ScriptedExtendedStatsAggregation => extendedStatsWithScript(agg) + } + + private def extendedStatsWithScript(agg: ScriptedExtendedStatsAggregation): XContentBuilder = { + val inner = agg.inner + val builder = XContentFactory.jsonBuilder() + builder.startObject("extended_stats") + inner.field.foreach(builder.field("field", _)) + inner.sigma.foreach(builder.field("sigma", _)) + inner.missing.foreach(builder.autofield("missing", _)) + // The one line elastic4s's ExtendedStatsAggregationBuilder lacks (elastic4s#4100). + inner.script.foreach(script => builder.rawField("script", ScriptBuilderFn(script))) + builder.endObject() + // Sub-aggregations and metadata are siblings of the aggregation type, as Elasticsearch + // expects them (`"aggs"` / `"meta"` beside `"extended_stats"`). The bridge never attaches either + // to an extended_stats, so the rendered shape stays identical to the default builder's. + SubAggsBuilderFn(inner, builder, handler) + AggMetaDataFn(inner, builder) + builder.endObject() + } + + override def serialize(search: SearchRequest): String = + SearchBodyBuilderFn(search, handler).string +} From 832c92cc7bbba0b2b743ecc5056029e5924fd45b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Manciot?= Date: Sun, 6 Sep 2026 06:58:22 +0200 Subject: [PATCH 02/11] wip(core,tests,docs): refusal boundary at GatewayApi.run; per-major specs; testkit #222 cases; docs (#222) Work in progress on story BIDC-3 (second WIP commit; unit runs pending). Story BIDC-3 Co-Authored-By: Claude Fable 5.1 --- .../elastic/sql/AggregationNamingSpec.scala | 79 ++++-- .../sql/ExtendedStatsEmissionSpec.scala | 241 ++++++++++++++++++ .../elastic/client/GatewayApi.scala | 49 ++-- .../client/GatewayRefusalBoundarySpec.scala | 108 ++++++++ documentation/sql/dql_statements.md | 6 + documentation/sql/functions_aggregate.md | 9 + documentation/sql/known_limitations.md | 11 + .../elastic/sql/AggregationNamingSpec.scala | 79 ++++-- .../sql/ExtendedStatsEmissionSpec.scala | 241 ++++++++++++++++++ ...JestClientExtendedStatsRejectionSpec.scala | 122 +++++++++ ...evelClientExtendedStatsRejectionSpec.scala | 122 +++++++++ ...evelClientExtendedStatsRejectionSpec.scala | 122 +++++++++ .../JavaClientExtendedStatsEmissionSpec.scala | 178 +++++++++++++ .../JavaClientExtendedStatsEmissionSpec.scala | 178 +++++++++++++ .../elastic/client/WindowFunctionSpec.scala | 150 +++++++++++ .../elastic/model/window/package.scala | 22 ++ 16 files changed, 1656 insertions(+), 61 deletions(-) create mode 100644 bridge/src/test/scala/app/softnetwork/elastic/sql/ExtendedStatsEmissionSpec.scala create mode 100644 core/src/test/scala/app/softnetwork/elastic/client/GatewayRefusalBoundarySpec.scala create mode 100644 es6/bridge/src/test/scala/app/softnetwork/elastic/sql/ExtendedStatsEmissionSpec.scala create mode 100644 es6/jest/src/test/scala/app/softnetwork/elastic/client/JestClientExtendedStatsRejectionSpec.scala create mode 100644 es6/rest/src/test/scala/app/softnetwork/elastic/client/RestHighLevelClientExtendedStatsRejectionSpec.scala create mode 100644 es7/rest/src/test/scala/app/softnetwork/elastic/client/RestHighLevelClientExtendedStatsRejectionSpec.scala create mode 100644 es8/java/src/test/scala/app/softnetwork/elastic/client/JavaClientExtendedStatsEmissionSpec.scala create mode 100644 es9/java/src/test/scala/app/softnetwork/elastic/client/JavaClientExtendedStatsEmissionSpec.scala diff --git a/bridge/src/test/scala/app/softnetwork/elastic/sql/AggregationNamingSpec.scala b/bridge/src/test/scala/app/softnetwork/elastic/sql/AggregationNamingSpec.scala index 543fec334..921d832f4 100644 --- a/bridge/src/test/scala/app/softnetwork/elastic/sql/AggregationNamingSpec.scala +++ b/bridge/src/test/scala/app/softnetwork/elastic/sql/AggregationNamingSpec.scala @@ -3,6 +3,7 @@ package app.softnetwork.elastic.sql import app.softnetwork.elastic.sql.bridge._ import app.softnetwork.elastic.sql.query._ import com.fasterxml.jackson.databind.{JsonNode, ObjectMapper} +import com.sksamuel.elastic4s.requests.searches.aggs.{AbstractAggregation, Aggregation} import org.scalatest.flatspec.AnyFlatSpec import org.scalatest.matchers.should.Matchers @@ -515,25 +516,65 @@ class AggregationNamingSpec extends AnyFlatSpec with Matchers { "no emitted Painless" should "dereference or compare a possibly-null value unguarded (AC 4b)" in { shapes.foreach { sql => withClue(s"[$sql] ") { - scriptsOf(mapper.readTree(queryOf(sql))).foreach { script => - withClue(s"script [$script] ") { - // (a) A `( ... ? null : ... )` group must not be dereferenced: `(cond ? null : v).m()` - // invokes `m` on null whenever `cond` holds -- the doubled `.get(ChronoField.YEAR)`. - nullTernaryGroupEnds(script).foreach { end => - if (end + 1 < script.length) script.charAt(end + 1) should not be '.' - } - // (b) A `def` bound to such a group is nullable: any later `name.` dereference must be - // preceded by a `name == null` / `name != null` test. - nullableDefs(script).foreach { name => - val afterDef = script.substring(script.indexOf(s"def $name") + 4 + name.length) - if (afterDef.contains(s"$name.")) - assert( - afterDef.contains(s"$name == null") || afterDef.contains(s"$name != null"), - s"'$name.' is dereferenced without a null test" - ) - } - } - } + scriptsOf(mapper.readTree(queryOf(sql))).foreach(assertNullSafe) + } + } + } + + /** The null-safety rule (lead directive 2026-09-06) on ONE Painless script. */ + private def assertNullSafe(script: String): Unit = + withClue(s"script [$script] ") { + // (a) A `( ... ? null : ... )` group must not be dereferenced: `(cond ? null : v).m()` + // invokes `m` on null whenever `cond` holds -- the doubled `.get(ChronoField.YEAR)`. + nullTernaryGroupEnds(script).foreach { end => + if (end + 1 < script.length) script.charAt(end + 1) should not be '.' + } + // (b) A `def` bound to such a group is nullable: any later `name.` dereference must be + // preceded by a `name == null` / `name != null` test. + nullableDefs(script).foreach { name => + val afterDef = script.substring(script.indexOf(s"def $name") + 4 + name.length) + if (afterDef.contains(s"$name.")) + assert( + afterDef.contains(s"$name == null") || afterDef.contains(s"$name != null"), + s"'$name.' is dereferenced without a null test" + ) + } + } + + // --------------------------------------------------------------------------------------------- + // Issue #222 (story BIDC-3) -- extended_stats over a transform. The metric scripts these shapes + // make reach Elasticsearch 8 / 9 for the first time are held to the same rule. The Default + // serializer refuses to render them (it would drop the script), so they are read off the built + // request's aggregation tree -- the ScriptedExtendedStatsAggregation marker -- not off JSON. + // --------------------------------------------------------------------------------------------- + + private val transformExtendedStatsShapes: Seq[String] = Seq( + "SELECT id, STDDEV(YEAR(createdAt)) AS s FROM t GROUP BY id", + "SELECT id, STDDEV(ABS(salary)) AS s FROM t GROUP BY id", + "SELECT id, VARIANCE(ABS(salary)) AS v FROM t GROUP BY id", + "SELECT id, STDDEV_POP(DATE_TRUNC(createdAt, MINUTE)) AS s FROM t GROUP BY id", + "SELECT id, VAR_SAMP(YEAR(createdAt)) AS a, VAR_POP(YEAR(createdAt)) AS b FROM t GROUP BY id", + "SELECT id, STDDEV(YEAR(createdAt)) AS s FROM t GROUP BY id HAVING COUNT(x) > 1", + "SELECT id, name, STDDEV(YEAR(createdAt)) OVER (PARTITION BY id) AS s FROM t", + "SELECT id, name, VARIANCE(ABS(salary)) OVER (PARTITION BY id) AS v FROM t", + "SELECT id, name, STDDEV_SAMP(YEAR(createdAt)) OVER (PARTITION BY id) AS s FROM t" + ) + + private def markerScriptsOf(aggs: Iterable[AbstractAggregation]): Seq[String] = + aggs.toSeq.flatMap { + case m: ScriptedExtendedStatsAggregation => m.inner.script.map(_.script).toSeq + case a: Aggregation => markerScriptsOf(a.subaggs) + case _ => Seq.empty + } + + "an extended_stats over a transform" should "carry a null-safe metric script, plain and windowed (issue #222)" in { + transformExtendedStatsShapes.foreach { sql => + withClue(s"[$sql] ") { + val request: ElasticSearchRequest = SelectStatement(sql) + request.hasTransformExtendedStats shouldBe true + val scripts = markerScriptsOf(request.search.aggs) + scripts should not be empty + scripts.foreach(assertNullSafe) } } } diff --git a/bridge/src/test/scala/app/softnetwork/elastic/sql/ExtendedStatsEmissionSpec.scala b/bridge/src/test/scala/app/softnetwork/elastic/sql/ExtendedStatsEmissionSpec.scala new file mode 100644 index 000000000..ba14bfacf --- /dev/null +++ b/bridge/src/test/scala/app/softnetwork/elastic/sql/ExtendedStatsEmissionSpec.scala @@ -0,0 +1,241 @@ +package app.softnetwork.elastic.sql + +import app.softnetwork.elastic.sql.bridge._ +import app.softnetwork.elastic.sql.query._ +import com.sksamuel.elastic4s.requests.searches.SearchRequest +import com.sksamuel.elastic4s.requests.searches.aggs.{AbstractAggregation, Aggregation} +import org.scalatest.flatspec.AnyFlatSpec +import org.scalatest.matchers.should.Matchers + +import java.time.ZonedDateTime + +/** Issue #222 (story BIDC-3): `STDDEV` / `VARIANCE` (the `extended_stats` family) over a + * TRANSFORMED expression -- the shared bridge template's half. + * + * The template cannot render the shape (that needs the ES 8 / ES 9 client modules' serializer), + * so what it owns is asserted here: the shape DISCRIMINATOR, both binds (plain and windowed) and + * both doors; the marker carrying field + script; the Default serializer's LOUD, named refusal + * instead of the silent raw-field statistic the library used to emit; and the raw-field + * `extended_stats` emission, pinned byte-for-byte (AC 4 -- these fixtures did not exist before and + * they must not move). + * + * Captured on the base commit (T2, both bridge copies, both doors), before the fix: + * `STDDEV(YEAR(createdAt))` -> `"extended_stats":{"field":"createdAt"}` (no script: the standard + * deviation of the raw timestamps), `STDDEV(ABS(salary))` -> `"extended_stats":{}` (neither field + * nor script), while `MAX(YEAR(createdAt))` emitted `"field":"createdAt","script":{...}`. + */ +class ExtendedStatsEmissionSpec extends AnyFlatSpec with Matchers { + + import scala.language.implicitConversions + + implicit def timestamp: Long = ZonedDateTime.parse("2025-12-31T00:00:00Z").toInstant.toEpochMilli + + implicit def sqlQueryToRequest(sqlQuery: SelectStatement): ElasticSearchRequest = + sqlQuery.statement match { + case Some(value: SingleSearch) => value.copy(score = sqlQuery.score) + case other => throw new IllegalArgumentException(s"Not a single search: $other") + } + + private def requestOf(sql: String): ElasticSearchRequest = SelectStatement(sql) + + private def aggregationsOf(sql: String): Seq[ElasticAggregation] = SelectStatement(sql) + + private val family = Seq("STDDEV", "STDDEV_SAMP", "STDDEV_POP", "VARIANCE", "VAR_SAMP", "VAR_POP") + + private def plain(fn: String, operand: String) = + s"SELECT id, $fn($operand) AS s FROM t GROUP BY id" + + private def windowed(fn: String, operand: String) = + s"SELECT id, name, $fn($operand) OVER (PARTITION BY id) AS s FROM t" + + /** Every marker in the aggregation tree of `search`. */ + private def markersOf(aggs: Iterable[AbstractAggregation]): Seq[ScriptedExtendedStatsAggregation] = + aggs.toSeq.flatMap { + case m: ScriptedExtendedStatsAggregation => Seq(m) + case a: Aggregation => markersOf(a.subaggs) + case _ => Seq.empty + } + + private def markersOf(search: SearchRequest): Seq[ScriptedExtendedStatsAggregation] = + markersOf(search.aggs) + + private val yearScript = + "def param1 = (doc['createdAt'].size() == 0 ? null : " + + "doc['createdAt'].value.toInstant().atZone(ZoneId.of('Z')).get(ChronoField.YEAR)); param1" + + // --------------------------------------------------------------------------------------------- + // The discriminator (AC 5): true for the plain bind, true for the windowed bind, false for a raw + // field -- on the search-body door AND on the sqlQueryToAggregations door. + // --------------------------------------------------------------------------------------------- + + "hasTransformExtendedStats" should "be true for every family member over a transform (plain bind)" in { + family.foreach { fn => + withClue(s"[$fn] ") { + requestOf(plain(fn, "YEAR(createdAt)")).hasTransformExtendedStats shouldBe true + requestOf(plain(fn, "ABS(salary)")).hasTransformExtendedStats shouldBe true + } + } + } + + it should "be true for every family member over a transform (windowed bind)" in { + family.foreach { fn => + withClue(s"[$fn] ") { + requestOf(windowed(fn, "YEAR(createdAt)")).hasTransformExtendedStats shouldBe true + requestOf(windowed(fn, "ABS(salary)")).hasTransformExtendedStats shouldBe true + } + } + } + + it should "be false for a raw field, plain and windowed" in { + family.foreach { fn => + withClue(s"[$fn] ") { + requestOf(plain(fn, "salary")).hasTransformExtendedStats shouldBe false + requestOf(windowed(fn, "salary")).hasTransformExtendedStats shouldBe false + } + } + } + + it should "be false for a transform inside ANOTHER aggregate (MAX emits its own script)" in { + requestOf("SELECT id, MAX(YEAR(createdAt)) AS m FROM t GROUP BY id").hasTransformExtendedStats shouldBe false + } + + it should "flag the aggregation itself on the sqlQueryToAggregations door" in { + // The Default serializer refuses the body of a transform-bearing aggregation, so the + // discriminator is read on the aggregation built with a permissive serializer -- passed + // explicitly: an implicit declared in a test body is not in scope of a class-level helper. + def permissiveAggregationsOf(sql: String): Seq[ElasticAggregation] = + sqlQueryToAggregations(SelectStatement(sql))( + timestamp, + PainlessContextType.Query, + TestSerializers.RawDefault + ) + val transformed = permissiveAggregationsOf(plain("STDDEV", "YEAR(createdAt)")) + transformed should have size 1 + transformed.head.hasTransformExtendedStats shouldBe true + transformed.head.isGlobalMetric shouldBe true + transformed.head.query shouldBe Some("") + + val raw = permissiveAggregationsOf(plain("STDDEV", "salary")) + raw should have size 1 + raw.head.hasTransformExtendedStats shouldBe false + raw.head.isGlobalMetric shouldBe true + } + + // --------------------------------------------------------------------------------------------- + // The marker carries what a rendering serializer needs -- and nothing the raw field emits. + // --------------------------------------------------------------------------------------------- + + "the marker" should "carry the field and the transform script for a field-derived transform" in { + val markers = markersOf(requestOf(plain("STDDEV", "YEAR(createdAt)")).search) + markers should have size 1 + val inner = markers.head.inner + inner.name shouldBe "s" + inner.field shouldBe Some("createdAt") + inner.script.map(_.script) shouldBe Some(yearScript) + } + + it should "carry no field for a transform with no raw field behind it (ABS(salary))" in { + // Before the fix this shape emitted `extended_stats: {}` -- which Elasticsearch rejects. + val markers = markersOf(requestOf(plain("VARIANCE", "ABS(salary)")).search) + markers should have size 1 + markers.head.inner.field shouldBe None + markers.head.inner.script.map(_.script) shouldBe Some( + "def param1 = (doc['salary'].size() == 0 ? null : doc['salary'].value); " + + "(param1 == null) ? null : Double.valueOf(Math.abs(param1))" + ) + } + + it should "sit under the partition bucket on the windowed bind" in { + val search = requestOf(windowed("STDDEV", "YEAR(createdAt)")).search + // Root level carries the partition terms aggregation, not the marker... + search.aggs.collect { case m: ScriptedExtendedStatsAggregation => m } shouldBe empty + // ...and the marker is exactly one level below it. + val markers = markersOf(search) + markers should have size 1 + markers.head.inner.field shouldBe Some("createdAt") + markers.head.inner.script.map(_.script) shouldBe Some(yearScript) + } + + it should "refuse to wrap an extended_stats without a script" in { + import com.sksamuel.elastic4s.ElasticApi.extendedStatsAgg + an[IllegalArgumentException] should be thrownBy + ScriptedExtendedStatsAggregation(extendedStatsAgg("s", "salary")) + } + + // --------------------------------------------------------------------------------------------- + // The Default serializer REFUSES the shape, loudly and by name -- both doors, both binds. + // --------------------------------------------------------------------------------------------- + + "the Default serializer" should "refuse a transform-bearing extended_stats on the search-body door" in { + Seq(plain("STDDEV", "YEAR(createdAt)"), windowed("VARIANCE", "ABS(salary)")).foreach { sql => + withClue(s"[$sql] ") { + val refusal = intercept[UnsupportedOperationException](requestOf(sql).query) + refusal.getMessage shouldBe SearchBodySerializer.TransformExtendedStatsUnsupported + refusal.getMessage should include("elastic4s#4100") + } + } + } + + it should "refuse it on the sqlQueryToAggregations door too" in { + Seq(plain("STDDEV", "YEAR(createdAt)"), plain("VAR_POP", "ABS(salary)")).foreach { sql => + withClue(s"[$sql] ") { + val refusal = intercept[UnsupportedOperationException](aggregationsOf(sql)) + refusal.getMessage shouldBe SearchBodySerializer.TransformExtendedStatsUnsupported + } + } + } + + it should "name the major in the client-module wording" in { + SearchBodySerializer.transformExtendedStatsUnsupportedOn(7) should include("Elasticsearch 7") + SearchBodySerializer.transformExtendedStatsUnsupportedOn(6) should include("Elasticsearch 6") + SearchBodySerializer.transformExtendedStatsUnsupportedOn(6) should include("elastic4s#4100") + } + + // --------------------------------------------------------------------------------------------- + // Raw-field extended_stats: byte-identical to the base commit on every major (AC 4). + // --------------------------------------------------------------------------------------------- + + "a raw-field extended_stats" should "emit exactly what it emitted before (plain bind)" in { + requestOf(plain("STDDEV", "salary")).query shouldBe + """{"query":{"match_all":{}},"size":0,"_source":false,"aggs":{"id":{""" + + """"terms":{"field":"id","size":65536,"min_doc_count":1},""" + + """"aggs":{"s":{"extended_stats":{"field":"salary"}}}}}}""" + } + + it should "emit exactly what it emitted before (windowed bind)" in { + requestOf(windowed("STDDEV", "salary")).query shouldBe + """{"query":{"match_all":{}},"size":0,"_source":false,"aggs":{"id":{""" + + """"terms":{"field":"id","size":65536,"min_doc_count":1},""" + + """"aggs":{"s":{"extended_stats":{"field":"salary"}}}}}}""" + } + + it should "emit exactly what it emitted before (sqlQueryToAggregations door)" in { + val aggs = aggregationsOf(plain("VAR_POP", "salary")) + aggs should have size 1 + aggs.head.query shouldBe Some( + """{"query":{"match_all":{}},"size":0,"aggs":{"s":{"extended_stats":{"field":"salary"}}}}""" + ) + } + + it should "leave every OTHER scripted metric untouched (MAX(YEAR(x)) still emits its script)" in { + requestOf("SELECT id, MAX(YEAR(createdAt)) AS m FROM t GROUP BY id").query shouldBe + """{"query":{"match_all":{}},"size":0,"_source":false,"aggs":{"id":{""" + + """"terms":{"field":"id","size":65536,"min_doc_count":1},""" + + """"aggs":{"m":{"max":{"field":"createdAt","script":{"lang":"painless",""" + + s""""source":"$yearScript"}}}}}}}""" + } +} + +/** Test-side serializers for the shared template tree. */ +object TestSerializers { + + /** The library's one-argument builder WITHOUT the Default's refusal -- reaches elastic4s's own + * behaviour on a marker (a `NotImplementedError`), so a test can build the aggregations of a + * transform-bearing statement on the sqlQueryToAggregations door and inspect them. + */ + object RawDefault extends SearchBodySerializer { + override def serialize(search: SearchRequest): String = + if (SearchBodySerializer.hasTransformExtendedStats(search)) "" + else SearchBodySerializer.Default.serialize(search) + } +} diff --git a/core/src/main/scala/app/softnetwork/elastic/client/GatewayApi.scala b/core/src/main/scala/app/softnetwork/elastic/client/GatewayApi.scala index 63e09618f..96440dff1 100644 --- a/core/src/main/scala/app/softnetwork/elastic/client/GatewayApi.scala +++ b/core/src/main/scala/app/softnetwork/elastic/client/GatewayApi.scala @@ -116,29 +116,6 @@ class SearchExecutor(api: ScrollApi with SearchApi, logger: Logger) implicit val context: ConversionContext = NativeContext - // The SQL -> Elasticsearch translation runs SYNCHRONOUSLY inside `searchAsync` / `scroll`, - // before any Future exists. A client module may REFUSE a statement there by throwing a - // status-bearing `ElasticError` (issue #222: STDDEV / VARIANCE over a transformed expression on - // ES 6 / ES 7, where the library cannot emit the aggregation script); this boundary turns that - // deliberate refusal into the `ElasticFailure` every other DQL error is, so BI tools see an - // honest 400 instead of a raw exception. Anything else escaping translation keeps its current - // (thrown) route -- a totality boundary for the whole translation layer is a separate change. - try dispatch(statement) - catch { - case refusal: ElasticError => - logger.error(s"❌ ${refusal.message}") - Future.successful(ElasticFailure(refusal.copy(operation = Some("dql")))) - } - } - - private def dispatch( - statement: SearchStatement - )(implicit - system: ActorSystem, - ec: ExecutionContext, - context: ConversionContext - ): Future[ElasticResult[QueryResult]] = { - statement match { // ============================ @@ -1954,6 +1931,32 @@ trait GatewayApi extends IndicesApi with ElasticClientHelpers { )(implicit system: ActorSystem): Future[ElasticResult[QueryResult]] = { implicit val ec: ExecutionContext = system.dispatcher + // The SQL -> Elasticsearch translation runs SYNCHRONOUSLY inside `searchAsync` / `scroll`, + // before any Future exists, on every route below -- the extension route included + // (`CoreDqlExtension`'s quota-capped scroll calls `client.scroll` directly and never enters an + // executor). A client module may REFUSE a statement there by throwing a status-bearing + // `ElasticError` (issue #222: STDDEV / VARIANCE over a transformed expression on ES 6 / ES 7, + // where the library cannot emit the aggregation script). This ONE boundary, at the front door + // every route converges on, turns that deliberate refusal into the `ElasticFailure` every other + // error is, so the REPL, JDBC and Arrow see an honest 400 instead of a raw exception. Anything + // else escaping translation keeps its current (thrown) route -- a `NonFatal` totality boundary + // for the whole translation layer is #250's shape and a separate change. + try dispatch(statement) + catch { + case refusal: ElasticError => + logger.error(s"❌ ${refusal.message}") + val operation = statement match { + case _: DqlStatement => Some("dql") // the relabel every DQL executor failure carries + case _ => refusal.operation + } + Future.successful(ElasticFailure(refusal.copy(operation = operation))) + } + } + + private def dispatch( + statement: Statement + )(implicit system: ActorSystem, ec: ExecutionContext): Future[ElasticResult[QueryResult]] = { + // ✅ TRY EXTENSIONS FIRST extensionRegistry.findHandler(statement) match { case Some(extension) => diff --git a/core/src/test/scala/app/softnetwork/elastic/client/GatewayRefusalBoundarySpec.scala b/core/src/test/scala/app/softnetwork/elastic/client/GatewayRefusalBoundarySpec.scala new file mode 100644 index 000000000..2831cd51e --- /dev/null +++ b/core/src/test/scala/app/softnetwork/elastic/client/GatewayRefusalBoundarySpec.scala @@ -0,0 +1,108 @@ +package app.softnetwork.elastic.client + +import akka.actor.ActorSystem +import app.softnetwork.elastic.client.result._ +import app.softnetwork.elastic.sql.PainlessContextType +import app.softnetwork.elastic.sql.query.SingleSearch +import org.scalatest.BeforeAndAfterAll +import org.scalatest.concurrent.ScalaFutures +import org.scalatest.flatspec.AnyFlatSpec +import org.scalatest.matchers.should.Matchers +import org.slf4j.{Logger, LoggerFactory} + +import scala.concurrent.duration._ + +/** Issue #222 (story BIDC-3) -- the ONE core boundary the per-major refusal relies on. + * + * The SQL -> Elasticsearch translation runs synchronously inside `searchAsync` / `scroll`, before + * any Future exists. A client module refuses a statement there by throwing a status-bearing + * `ElasticError` (the ES 6 / ES 7 modules do so for STDDEV / VARIANCE over a transformed + * expression). `GatewayApi.run` must surface that refusal as the `ElasticFailure` every other error + * is -- on EVERY route a DQL statement can take: the `SearchExecutor` (aggregation-shaped, or an + * explicit LIMIT) AND the `CoreDqlExtension`'s quota-capped scroll (an un-LIMITed row query, plain + * or windowed), which calls `client.scroll` directly and never enters the executor. That second + * route is why the boundary lives in `run`, not in `SearchExecutor`: a first cut there let the + * refusal escape as a raw exception on exactly the path BI tools take for a plain projection. + * + * Docker-free: a `NopeClientApi` whose `singleSearchToJsonQuery` refuses like a client module. + */ +class GatewayRefusalBoundarySpec + extends AnyFlatSpec + with Matchers + with ScalaFutures + with BeforeAndAfterAll { + + implicit private val system: ActorSystem = ActorSystem("gateway-refusal-boundary") + override implicit val patienceConfig: PatienceConfig = + PatienceConfig(timeout = scaled(5.seconds)) + + override def afterAll(): Unit = { + system.terminate() + super.afterAll() + } + + private val refusalMessage = + "STDDEV/VARIANCE over a transformed expression is not supported on Elasticsearch 7 (test double)" + + private class RefusingClient extends NopeClientApi { + override protected def logger: Logger = LoggerFactory.getLogger(getClass) + + override private[client] implicit def singleSearchToJsonQuery( + sqlSearch: SingleSearch + )(implicit timestamp: Long, contextType: PainlessContextType): String = + throw ElasticError( + message = refusalMessage, + statusCode = Some(400), + operation = Some("search") + ) + } + + private val client = new RefusingClient + + private def assertRefused(result: ElasticResult[QueryResult]): Unit = + result match { + case ElasticFailure(error) => + error.message shouldBe refusalMessage + error.statusCode shouldBe Some(400) + error.operation shouldBe Some("dql") + case other => fail(s"a refused translation must be an ElasticFailure, got $other") + } + + "GatewayApi.run" should "surface a client-module refusal as an ElasticFailure on the SearchExecutor route (aggregation shape)" in { + assertRefused( + client.run("SELECT id, STDDEV(YEAR(createdAt)) AS s FROM t GROUP BY id").futureValue + ) + } + + it should "surface it on the SearchExecutor route (explicit LIMIT, one-shot row query)" in { + assertRefused(client.run("SELECT id, name FROM t LIMIT 5").futureValue) + } + + it should "surface it on the CoreDqlExtension quota-capped scroll route (un-LIMITed row query)" in { + // Community quota is finite, so `capOrReject` rule (2) calls `client.scroll` directly -- the + // route that bypasses every executor. This is the case that found the misplaced boundary. + assertRefused(client.run("SELECT id, name FROM t").futureValue) + } + + it should "surface it on the windowed row route (un-LIMITed STDDEV OVER PARTITION BY)" in { + assertRefused( + client + .run("SELECT id, name, STDDEV(YEAR(createdAt)) OVER (PARTITION BY id) AS s FROM t") + .futureValue + ) + } + + it should "not manufacture a refusal for a client that does not refuse" in { + // NopeClientApi answers a search with no response body, which core reports as an ordinary + // execution failure -- proving the boundary only relabels a refusal the client raised. + val plain = new NopeClientApi { + override protected def logger: Logger = LoggerFactory.getLogger(getClass) + } + plain.run("SELECT id, name FROM t LIMIT 5").futureValue match { + case ElasticFailure(error) => + error.message should not include refusalMessage + error.statusCode should not be Some(400) + case other => fail(s"unexpected $other") + } + } +} diff --git a/documentation/sql/dql_statements.md b/documentation/sql/dql_statements.md index 3dface591..171a27989 100644 --- a/documentation/sql/dql_statements.md +++ b/documentation/sql/dql_statements.md @@ -475,6 +475,12 @@ All six map to a single Elasticsearch `extended_stats` aggregation per call; the `std_deviation`, `variance` for the population variants) is projected from the response. Sample variants require **Elasticsearch 7.7+**; population variants work on Elasticsearch 6+. +Over a **transformed** operand (`STDDEV(YEAR(hire_date))`, `VARIANCE(ABS(salary))`, plain or +windowed) the statistic is computed over the transform on **Elasticsearch 8+**; on Elasticsearch 6 +and 7 the query is **refused** with a `400` naming the release, because the client library cannot +emit the aggregation script there and used to return the statistic of the raw field silently. See +[STDDEV / VARIANCE family](functions_aggregate.md#function-stddev--variance-family). + ### Percentiles — `PERCENTILE_CONT` / `PERCENTILE_DISC` - `PERCENTILE_CONT(p) WITHIN GROUP (ORDER BY column)` — ANSI ordered-set aggregate (optionally with a top-level `GROUP BY`) diff --git a/documentation/sql/functions_aggregate.md b/documentation/sql/functions_aggregate.md index 8a74355a7..9785ae674 100644 --- a/documentation/sql/functions_aggregate.md +++ b/documentation/sql/functions_aggregate.md @@ -1245,6 +1245,15 @@ STDDEV(expr) OVER (PARTITION BY partition_expr, ...) - `NULL` values are ignored. - The un-suffixed `std_deviation` / `variance` keys are the **population** values (present on Elasticsearch 6+); the `_sampling` keys are the **sample** values (introduced in Elasticsearch 7.7). Consequently the sample variants — including the default `STDDEV` / `VARIANCE` — require Elasticsearch 7.7+. On older clusters the column is returned as `null` and a warning is logged. - Each call emits its own `extended_stats` aggregation; two stat calls over the same column emit two aggregations. +- **Transformed operands are per-major** (`STDDEV(YEAR(created_at))`, `VARIANCE(ABS(salary))`, `STDDEV_POP(DATE_TRUNC(ts, MONTH))`, plain or windowed). The Elasticsearch client library the driver builds on drops the aggregation script of an `extended_stats` on every line (elastic4s#4100), so: + + | Elasticsearch | `STDDEV(f(x))` / `VARIANCE(f(x))` | + |---------------|-----------------------------------| + | 8.x, 9.x | Computed over the transform — the driver emits the script itself. | + | 7.x | **Refused** with a `400` naming the release (`STDDEV/VARIANCE over a transformed expression is not supported on Elasticsearch 7 …`). Lifted when the upstream fix reaches the 7.17 line. | + | 6.x | **Refused** the same way, permanently (unmaintained library line). | + + Before this rule, every release silently returned the statistic of the **raw** field (or an empty `extended_stats` Elasticsearch rejected). A raw-field operand (`STDDEV(salary)`) is unaffected on every release. **Examples:** ```sql diff --git a/documentation/sql/known_limitations.md b/documentation/sql/known_limitations.md index 5f5bb574c..bbb2e2378 100644 --- a/documentation/sql/known_limitations.md +++ b/documentation/sql/known_limitations.md @@ -107,6 +107,17 @@ Quoted column names and aliases work in both spellings — see is deliberate: when the dot was allowed to float, `ORDER BY b. DESC` silently parsed as a column named `b.DESC` sorted *ascending*. +## `STDDEV` / `VARIANCE` over a transformed expression — Elasticsearch 6 and 7 refuse it + +`STDDEV(YEAR(hire_date))`, `VARIANCE(ABS(salary))` and the rest of the `extended_stats` family over +a transformed operand (plain or `OVER (PARTITION BY …)`) compute correctly on **Elasticsearch 8 and +9**. On **Elasticsearch 6 and 7** the query is refused with a `400` — *"STDDEV/VARIANCE over a +transformed expression is not supported on Elasticsearch 7 …"* — because the client library the +driver builds on drops the aggregation script on those lines (elastic4s#4100); until this rule, the +query silently returned the statistic of the **raw** field. Aggregate over a raw field there, or use +Elasticsearch 8+. The 7.x refusal is lifted once the upstream fix reaches the 7.17 line; the 6.x one +is permanent. See [STDDEV / VARIANCE family](functions_aggregate.md#function-stddev--variance-family). + ## Coming in the upcoming release (Quarter 1 2027) - **Heterogeneous federation**: JOIN or correlate Elasticsearch with PostgreSQL, MySQL, ClickHouse, Snowflake, and more — plus cross-cluster subqueries (e.g. correlate one cluster's data against another's). diff --git a/es6/bridge/src/test/scala/app/softnetwork/elastic/sql/AggregationNamingSpec.scala b/es6/bridge/src/test/scala/app/softnetwork/elastic/sql/AggregationNamingSpec.scala index 543fec334..63acdbc5e 100644 --- a/es6/bridge/src/test/scala/app/softnetwork/elastic/sql/AggregationNamingSpec.scala +++ b/es6/bridge/src/test/scala/app/softnetwork/elastic/sql/AggregationNamingSpec.scala @@ -3,6 +3,7 @@ package app.softnetwork.elastic.sql import app.softnetwork.elastic.sql.bridge._ import app.softnetwork.elastic.sql.query._ import com.fasterxml.jackson.databind.{JsonNode, ObjectMapper} +import com.sksamuel.elastic4s.searches.aggs.{AbstractAggregation, Aggregation} import org.scalatest.flatspec.AnyFlatSpec import org.scalatest.matchers.should.Matchers @@ -515,25 +516,65 @@ class AggregationNamingSpec extends AnyFlatSpec with Matchers { "no emitted Painless" should "dereference or compare a possibly-null value unguarded (AC 4b)" in { shapes.foreach { sql => withClue(s"[$sql] ") { - scriptsOf(mapper.readTree(queryOf(sql))).foreach { script => - withClue(s"script [$script] ") { - // (a) A `( ... ? null : ... )` group must not be dereferenced: `(cond ? null : v).m()` - // invokes `m` on null whenever `cond` holds -- the doubled `.get(ChronoField.YEAR)`. - nullTernaryGroupEnds(script).foreach { end => - if (end + 1 < script.length) script.charAt(end + 1) should not be '.' - } - // (b) A `def` bound to such a group is nullable: any later `name.` dereference must be - // preceded by a `name == null` / `name != null` test. - nullableDefs(script).foreach { name => - val afterDef = script.substring(script.indexOf(s"def $name") + 4 + name.length) - if (afterDef.contains(s"$name.")) - assert( - afterDef.contains(s"$name == null") || afterDef.contains(s"$name != null"), - s"'$name.' is dereferenced without a null test" - ) - } - } - } + scriptsOf(mapper.readTree(queryOf(sql))).foreach(assertNullSafe) + } + } + } + + /** The null-safety rule (lead directive 2026-09-06) on ONE Painless script. */ + private def assertNullSafe(script: String): Unit = + withClue(s"script [$script] ") { + // (a) A `( ... ? null : ... )` group must not be dereferenced: `(cond ? null : v).m()` + // invokes `m` on null whenever `cond` holds -- the doubled `.get(ChronoField.YEAR)`. + nullTernaryGroupEnds(script).foreach { end => + if (end + 1 < script.length) script.charAt(end + 1) should not be '.' + } + // (b) A `def` bound to such a group is nullable: any later `name.` dereference must be + // preceded by a `name == null` / `name != null` test. + nullableDefs(script).foreach { name => + val afterDef = script.substring(script.indexOf(s"def $name") + 4 + name.length) + if (afterDef.contains(s"$name.")) + assert( + afterDef.contains(s"$name == null") || afterDef.contains(s"$name != null"), + s"'$name.' is dereferenced without a null test" + ) + } + } + + // --------------------------------------------------------------------------------------------- + // Issue #222 (story BIDC-3) -- extended_stats over a transform. The metric scripts these shapes + // make reach Elasticsearch 8 / 9 for the first time are held to the same rule. The Default + // serializer refuses to render them (it would drop the script), so they are read off the built + // request's aggregation tree -- the ScriptedExtendedStatsAggregation marker -- not off JSON. + // --------------------------------------------------------------------------------------------- + + private val transformExtendedStatsShapes: Seq[String] = Seq( + "SELECT id, STDDEV(YEAR(createdAt)) AS s FROM t GROUP BY id", + "SELECT id, STDDEV(ABS(salary)) AS s FROM t GROUP BY id", + "SELECT id, VARIANCE(ABS(salary)) AS v FROM t GROUP BY id", + "SELECT id, STDDEV_POP(DATE_TRUNC(createdAt, MINUTE)) AS s FROM t GROUP BY id", + "SELECT id, VAR_SAMP(YEAR(createdAt)) AS a, VAR_POP(YEAR(createdAt)) AS b FROM t GROUP BY id", + "SELECT id, STDDEV(YEAR(createdAt)) AS s FROM t GROUP BY id HAVING COUNT(x) > 1", + "SELECT id, name, STDDEV(YEAR(createdAt)) OVER (PARTITION BY id) AS s FROM t", + "SELECT id, name, VARIANCE(ABS(salary)) OVER (PARTITION BY id) AS v FROM t", + "SELECT id, name, STDDEV_SAMP(YEAR(createdAt)) OVER (PARTITION BY id) AS s FROM t" + ) + + private def markerScriptsOf(aggs: Iterable[AbstractAggregation]): Seq[String] = + aggs.toSeq.flatMap { + case m: ScriptedExtendedStatsAggregation => m.inner.script.map(_.script).toSeq + case a: Aggregation => markerScriptsOf(a.subaggs) + case _ => Seq.empty + } + + "an extended_stats over a transform" should "carry a null-safe metric script, plain and windowed (issue #222)" in { + transformExtendedStatsShapes.foreach { sql => + withClue(s"[$sql] ") { + val request: ElasticSearchRequest = SelectStatement(sql) + request.hasTransformExtendedStats shouldBe true + val scripts = markerScriptsOf(request.search.aggs) + scripts should not be empty + scripts.foreach(assertNullSafe) } } } diff --git a/es6/bridge/src/test/scala/app/softnetwork/elastic/sql/ExtendedStatsEmissionSpec.scala b/es6/bridge/src/test/scala/app/softnetwork/elastic/sql/ExtendedStatsEmissionSpec.scala new file mode 100644 index 000000000..4b0b3bf3a --- /dev/null +++ b/es6/bridge/src/test/scala/app/softnetwork/elastic/sql/ExtendedStatsEmissionSpec.scala @@ -0,0 +1,241 @@ +package app.softnetwork.elastic.sql + +import app.softnetwork.elastic.sql.bridge._ +import app.softnetwork.elastic.sql.query._ +import com.sksamuel.elastic4s.searches.SearchRequest +import com.sksamuel.elastic4s.searches.aggs.{AbstractAggregation, Aggregation} +import org.scalatest.flatspec.AnyFlatSpec +import org.scalatest.matchers.should.Matchers + +import java.time.ZonedDateTime + +/** Issue #222 (story BIDC-3): `STDDEV` / `VARIANCE` (the `extended_stats` family) over a + * TRANSFORMED expression -- the shared bridge template's half. + * + * The template cannot render the shape (that needs the ES 8 / ES 9 client modules' serializer), + * so what it owns is asserted here: the shape DISCRIMINATOR, both binds (plain and windowed) and + * both doors; the marker carrying field + script; the Default serializer's LOUD, named refusal + * instead of the silent raw-field statistic the library used to emit; and the raw-field + * `extended_stats` emission, pinned byte-for-byte (AC 4 -- these fixtures did not exist before and + * they must not move). + * + * Captured on the base commit (T2, both bridge copies, both doors), before the fix: + * `STDDEV(YEAR(createdAt))` -> `"extended_stats":{"field":"createdAt"}` (no script: the standard + * deviation of the raw timestamps), `STDDEV(ABS(salary))` -> `"extended_stats":{}` (neither field + * nor script), while `MAX(YEAR(createdAt))` emitted `"field":"createdAt","script":{...}`. + */ +class ExtendedStatsEmissionSpec extends AnyFlatSpec with Matchers { + + import scala.language.implicitConversions + + implicit def timestamp: Long = ZonedDateTime.parse("2025-12-31T00:00:00Z").toInstant.toEpochMilli + + implicit def sqlQueryToRequest(sqlQuery: SelectStatement): ElasticSearchRequest = + sqlQuery.statement match { + case Some(value: SingleSearch) => value.copy(score = sqlQuery.score) + case other => throw new IllegalArgumentException(s"Not a single search: $other") + } + + private def requestOf(sql: String): ElasticSearchRequest = SelectStatement(sql) + + private def aggregationsOf(sql: String): Seq[ElasticAggregation] = SelectStatement(sql) + + private val family = Seq("STDDEV", "STDDEV_SAMP", "STDDEV_POP", "VARIANCE", "VAR_SAMP", "VAR_POP") + + private def plain(fn: String, operand: String) = + s"SELECT id, $fn($operand) AS s FROM t GROUP BY id" + + private def windowed(fn: String, operand: String) = + s"SELECT id, name, $fn($operand) OVER (PARTITION BY id) AS s FROM t" + + /** Every marker in the aggregation tree of `search`. */ + private def markersOf(aggs: Iterable[AbstractAggregation]): Seq[ScriptedExtendedStatsAggregation] = + aggs.toSeq.flatMap { + case m: ScriptedExtendedStatsAggregation => Seq(m) + case a: Aggregation => markersOf(a.subaggs) + case _ => Seq.empty + } + + private def markersOf(search: SearchRequest): Seq[ScriptedExtendedStatsAggregation] = + markersOf(search.aggs) + + private val yearScript = + "def param1 = (doc['createdAt'].size() == 0 ? null : " + + "doc['createdAt'].value.toInstant().atZone(ZoneId.of('Z')).get(ChronoField.YEAR)); param1" + + // --------------------------------------------------------------------------------------------- + // The discriminator (AC 5): true for the plain bind, true for the windowed bind, false for a raw + // field -- on the search-body door AND on the sqlQueryToAggregations door. + // --------------------------------------------------------------------------------------------- + + "hasTransformExtendedStats" should "be true for every family member over a transform (plain bind)" in { + family.foreach { fn => + withClue(s"[$fn] ") { + requestOf(plain(fn, "YEAR(createdAt)")).hasTransformExtendedStats shouldBe true + requestOf(plain(fn, "ABS(salary)")).hasTransformExtendedStats shouldBe true + } + } + } + + it should "be true for every family member over a transform (windowed bind)" in { + family.foreach { fn => + withClue(s"[$fn] ") { + requestOf(windowed(fn, "YEAR(createdAt)")).hasTransformExtendedStats shouldBe true + requestOf(windowed(fn, "ABS(salary)")).hasTransformExtendedStats shouldBe true + } + } + } + + it should "be false for a raw field, plain and windowed" in { + family.foreach { fn => + withClue(s"[$fn] ") { + requestOf(plain(fn, "salary")).hasTransformExtendedStats shouldBe false + requestOf(windowed(fn, "salary")).hasTransformExtendedStats shouldBe false + } + } + } + + it should "be false for a transform inside ANOTHER aggregate (MAX emits its own script)" in { + requestOf("SELECT id, MAX(YEAR(createdAt)) AS m FROM t GROUP BY id").hasTransformExtendedStats shouldBe false + } + + it should "flag the aggregation itself on the sqlQueryToAggregations door" in { + // The Default serializer refuses the body of a transform-bearing aggregation, so the + // discriminator is read on the aggregation built with a permissive serializer -- passed + // explicitly: an implicit declared in a test body is not in scope of a class-level helper. + def permissiveAggregationsOf(sql: String): Seq[ElasticAggregation] = + sqlQueryToAggregations(SelectStatement(sql))( + timestamp, + PainlessContextType.Query, + TestSerializers.RawDefault + ) + val transformed = permissiveAggregationsOf(plain("STDDEV", "YEAR(createdAt)")) + transformed should have size 1 + transformed.head.hasTransformExtendedStats shouldBe true + transformed.head.isGlobalMetric shouldBe true + transformed.head.query shouldBe Some("") + + val raw = permissiveAggregationsOf(plain("STDDEV", "salary")) + raw should have size 1 + raw.head.hasTransformExtendedStats shouldBe false + raw.head.isGlobalMetric shouldBe true + } + + // --------------------------------------------------------------------------------------------- + // The marker carries what a rendering serializer needs -- and nothing the raw field emits. + // --------------------------------------------------------------------------------------------- + + "the marker" should "carry the field and the transform script for a field-derived transform" in { + val markers = markersOf(requestOf(plain("STDDEV", "YEAR(createdAt)")).search) + markers should have size 1 + val inner = markers.head.inner + inner.name shouldBe "s" + inner.field shouldBe Some("createdAt") + inner.script.map(_.script) shouldBe Some(yearScript) + } + + it should "carry no field for a transform with no raw field behind it (ABS(salary))" in { + // Before the fix this shape emitted `extended_stats: {}` -- which Elasticsearch rejects. + val markers = markersOf(requestOf(plain("VARIANCE", "ABS(salary)")).search) + markers should have size 1 + markers.head.inner.field shouldBe None + markers.head.inner.script.map(_.script) shouldBe Some( + "def param1 = (doc['salary'].size() == 0 ? null : doc['salary'].value); " + + "(param1 == null) ? null : Double.valueOf(Math.abs(param1))" + ) + } + + it should "sit under the partition bucket on the windowed bind" in { + val search = requestOf(windowed("STDDEV", "YEAR(createdAt)")).search + // Root level carries the partition terms aggregation, not the marker... + search.aggs.collect { case m: ScriptedExtendedStatsAggregation => m } shouldBe empty + // ...and the marker is exactly one level below it. + val markers = markersOf(search) + markers should have size 1 + markers.head.inner.field shouldBe Some("createdAt") + markers.head.inner.script.map(_.script) shouldBe Some(yearScript) + } + + it should "refuse to wrap an extended_stats without a script" in { + import com.sksamuel.elastic4s.ElasticApi.extendedStatsAgg + an[IllegalArgumentException] should be thrownBy + ScriptedExtendedStatsAggregation(extendedStatsAgg("s", "salary")) + } + + // --------------------------------------------------------------------------------------------- + // The Default serializer REFUSES the shape, loudly and by name -- both doors, both binds. + // --------------------------------------------------------------------------------------------- + + "the Default serializer" should "refuse a transform-bearing extended_stats on the search-body door" in { + Seq(plain("STDDEV", "YEAR(createdAt)"), windowed("VARIANCE", "ABS(salary)")).foreach { sql => + withClue(s"[$sql] ") { + val refusal = intercept[UnsupportedOperationException](requestOf(sql).query) + refusal.getMessage shouldBe SearchBodySerializer.TransformExtendedStatsUnsupported + refusal.getMessage should include("elastic4s#4100") + } + } + } + + it should "refuse it on the sqlQueryToAggregations door too" in { + Seq(plain("STDDEV", "YEAR(createdAt)"), plain("VAR_POP", "ABS(salary)")).foreach { sql => + withClue(s"[$sql] ") { + val refusal = intercept[UnsupportedOperationException](aggregationsOf(sql)) + refusal.getMessage shouldBe SearchBodySerializer.TransformExtendedStatsUnsupported + } + } + } + + it should "name the major in the client-module wording" in { + SearchBodySerializer.transformExtendedStatsUnsupportedOn(7) should include("Elasticsearch 7") + SearchBodySerializer.transformExtendedStatsUnsupportedOn(6) should include("Elasticsearch 6") + SearchBodySerializer.transformExtendedStatsUnsupportedOn(6) should include("elastic4s#4100") + } + + // --------------------------------------------------------------------------------------------- + // Raw-field extended_stats: byte-identical to the base commit on every major (AC 4). + // --------------------------------------------------------------------------------------------- + + "a raw-field extended_stats" should "emit exactly what it emitted before (plain bind)" in { + requestOf(plain("STDDEV", "salary")).query shouldBe + """{"query":{"match_all":{}},"size":0,"_source":false,"aggs":{"id":{""" + + """"terms":{"field":"id","size":65536,"min_doc_count":1},""" + + """"aggs":{"s":{"extended_stats":{"field":"salary"}}}}}}""" + } + + it should "emit exactly what it emitted before (windowed bind)" in { + requestOf(windowed("STDDEV", "salary")).query shouldBe + """{"query":{"match_all":{}},"size":0,"_source":false,"aggs":{"id":{""" + + """"terms":{"field":"id","size":65536,"min_doc_count":1},""" + + """"aggs":{"s":{"extended_stats":{"field":"salary"}}}}}}""" + } + + it should "emit exactly what it emitted before (sqlQueryToAggregations door)" in { + val aggs = aggregationsOf(plain("VAR_POP", "salary")) + aggs should have size 1 + aggs.head.query shouldBe Some( + """{"query":{"match_all":{}},"size":0,"aggs":{"s":{"extended_stats":{"field":"salary"}}}}""" + ) + } + + it should "leave every OTHER scripted metric untouched (MAX(YEAR(x)) still emits its script)" in { + requestOf("SELECT id, MAX(YEAR(createdAt)) AS m FROM t GROUP BY id").query shouldBe + """{"query":{"match_all":{}},"size":0,"_source":false,"aggs":{"id":{""" + + """"terms":{"field":"id","size":65536,"min_doc_count":1},""" + + """"aggs":{"m":{"max":{"field":"createdAt","script":{"lang":"painless",""" + + s""""source":"$yearScript"}}}}}}}""" + } +} + +/** Test-side serializers for the shared template tree. */ +object TestSerializers { + + /** The library's one-argument builder WITHOUT the Default's refusal -- reaches elastic4s's own + * behaviour on a marker (a `NotImplementedError`), so a test can build the aggregations of a + * transform-bearing statement on the sqlQueryToAggregations door and inspect them. + */ + object RawDefault extends SearchBodySerializer { + override def serialize(search: SearchRequest): String = + if (SearchBodySerializer.hasTransformExtendedStats(search)) "" + else SearchBodySerializer.Default.serialize(search) + } +} diff --git a/es6/jest/src/test/scala/app/softnetwork/elastic/client/JestClientExtendedStatsRejectionSpec.scala b/es6/jest/src/test/scala/app/softnetwork/elastic/client/JestClientExtendedStatsRejectionSpec.scala new file mode 100644 index 000000000..a8ce28b22 --- /dev/null +++ b/es6/jest/src/test/scala/app/softnetwork/elastic/client/JestClientExtendedStatsRejectionSpec.scala @@ -0,0 +1,122 @@ +/* + * 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 app.softnetwork.elastic.client.jest.{ + JestClientApi, + JestSearchBodySerializer +} +import app.softnetwork.elastic.client.result.ElasticError +import app.softnetwork.elastic.sql.bridge._ +import app.softnetwork.elastic.sql.query.{SelectStatement, SingleSearch} +import com.typesafe.config.{Config, ConfigFactory} +import org.scalatest.matchers.should.Matchers +import org.scalatest.wordspec.AnyWordSpec + +/** Issue #222 (story BIDC-3) -- the ES 6 Jest-client half: `STDDEV` / `VARIANCE` (the whole + * `extended_stats` family) over a TRANSFORMED expression is REFUSED before any JSON exists, with a + * named `ElasticError` (status 400), for BOTH binds and on BOTH serialisation doors -- never + * executed against the raw field, which is what elastic4s 6.x's script-dropping builder used to + * do silently. A raw-field extended_stats is untouched. + * + * No Docker: `ElasticClientCompanion` builds the underlying client lazily and `apply()` is never + * called, so nothing here touches the network. + */ +class JestClientExtendedStatsRejectionSpec extends AnyWordSpec with Matchers { + + private val client: JestClientApi = new JestClientApi { + override lazy val config: Config = ConfigFactory.load() + } + + implicit private val timestamp: Long = 1767139200000L // 2025-12-31T00:00:00Z + + private val expectedMajor = 6 + + private def single(sql: String): SingleSearch = + SelectStatement(sql).statement match { + case Some(s: SingleSearch) => s + case other => fail(s"Not a single search: $other") + } + + private val family = Seq("STDDEV", "STDDEV_SAMP", "STDDEV_POP", "VARIANCE", "VAR_SAMP", "VAR_POP") + + private def plain(fn: String, operand: String) = + s"SELECT id, $fn($operand) AS s FROM t GROUP BY id" + + private def windowed(fn: String, operand: String) = + s"SELECT id, name, $fn($operand) OVER (PARTITION BY id) AS s FROM t" + + private def assertRefused(refusal: ElasticError): Unit = { + refusal.message shouldBe JestSearchBodySerializer.TransformExtendedStatsUnsupported + refusal.message should include(s"Elasticsearch $expectedMajor") + refusal.message should include("elastic4s#4100") + refusal.statusCode shouldBe Some(400) + refusal.operation shouldBe Some("search") + } + + s"the ES $expectedMajor client" should { + + "refuse a transform-bearing extended_stats before emission, for every family member and both binds" in { + family.foreach { fn => + Seq( + plain(fn, "YEAR(createdAt)"), + plain(fn, "ABS(salary)"), + windowed(fn, "YEAR(createdAt)"), + windowed(fn, "ABS(salary)") + ).foreach { sql => + withClue(s"[$sql] ") { + assertRefused(intercept[ElasticError](client.singleSearchToJsonQuery(single(sql)))) + } + } + } + } + + "refuse it on the sqlQueryToAggregations door too" in { + implicit val serializer: SearchBodySerializer = client.searchBodySerializer + val refusal = intercept[ElasticError] { + val aggs: Seq[ElasticAggregation] = SelectStatement(plain("STDDEV", "YEAR(createdAt)")) + aggs + } + assertRefused(refusal) + } + + "leave a raw-field extended_stats exactly as the default serializer emits it" in { + Seq(plain("STDDEV", "salary"), windowed("VAR_POP", "salary")).foreach { sql => + withClue(s"[$sql] ") { + val request: ElasticSearchRequest = requestToElasticSearchRequest(single(sql)) + request.hasTransformExtendedStats shouldBe false + client.singleSearchToJsonQuery(single(sql)) shouldBe + SearchBodySerializer.Default.serialize(request.search).replace("\"version\":true,", "") + client.singleSearchToJsonQuery(single(sql)) should include( + """"extended_stats":{"field":"salary"}""" + ) + } + } + } + + "leave every OTHER scripted metric untouched (MAX(YEAR(x)) keeps its script)" in { + client.singleSearchToJsonQuery( + single("SELECT id, MAX(YEAR(createdAt)) AS m FROM t GROUP BY id") + ) should include(""""max":{"field":"createdAt","script":{"lang":"painless"""") + } + + "inject the refusing serializer" in { + client.searchBodySerializer shouldBe JestSearchBodySerializer + JestSearchBodySerializer.ElasticsearchMajor shouldBe expectedMajor + } + } +} diff --git a/es6/rest/src/test/scala/app/softnetwork/elastic/client/RestHighLevelClientExtendedStatsRejectionSpec.scala b/es6/rest/src/test/scala/app/softnetwork/elastic/client/RestHighLevelClientExtendedStatsRejectionSpec.scala new file mode 100644 index 000000000..fdc4e5953 --- /dev/null +++ b/es6/rest/src/test/scala/app/softnetwork/elastic/client/RestHighLevelClientExtendedStatsRejectionSpec.scala @@ -0,0 +1,122 @@ +/* + * 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 app.softnetwork.elastic.client.rest.{ + RestHighLevelClientApi, + RestHighLevelClientSearchBodySerializer +} +import app.softnetwork.elastic.client.result.ElasticError +import app.softnetwork.elastic.sql.bridge._ +import app.softnetwork.elastic.sql.query.{SelectStatement, SingleSearch} +import com.typesafe.config.{Config, ConfigFactory} +import org.scalatest.matchers.should.Matchers +import org.scalatest.wordspec.AnyWordSpec + +/** Issue #222 (story BIDC-3) -- the ES 6 REST-client half: `STDDEV` / `VARIANCE` (the whole + * `extended_stats` family) over a TRANSFORMED expression is REFUSED before any JSON exists, with a + * named `ElasticError` (status 400), for BOTH binds and on BOTH serialisation doors -- never + * executed against the raw field, which is what elastic4s 6.x's script-dropping builder used to + * do silently. A raw-field extended_stats is untouched. + * + * No Docker: `ElasticClientCompanion` builds the underlying client lazily and `apply()` is never + * called, so nothing here touches the network. + */ +class RestHighLevelClientExtendedStatsRejectionSpec extends AnyWordSpec with Matchers { + + private val client: RestHighLevelClientApi = new RestHighLevelClientApi { + override def config: Config = ConfigFactory.load() + } + + implicit private val timestamp: Long = 1767139200000L // 2025-12-31T00:00:00Z + + private val expectedMajor = 6 + + private def single(sql: String): SingleSearch = + SelectStatement(sql).statement match { + case Some(s: SingleSearch) => s + case other => fail(s"Not a single search: $other") + } + + private val family = Seq("STDDEV", "STDDEV_SAMP", "STDDEV_POP", "VARIANCE", "VAR_SAMP", "VAR_POP") + + private def plain(fn: String, operand: String) = + s"SELECT id, $fn($operand) AS s FROM t GROUP BY id" + + private def windowed(fn: String, operand: String) = + s"SELECT id, name, $fn($operand) OVER (PARTITION BY id) AS s FROM t" + + private def assertRefused(refusal: ElasticError): Unit = { + refusal.message shouldBe RestHighLevelClientSearchBodySerializer.TransformExtendedStatsUnsupported + refusal.message should include(s"Elasticsearch $expectedMajor") + refusal.message should include("elastic4s#4100") + refusal.statusCode shouldBe Some(400) + refusal.operation shouldBe Some("search") + } + + s"the ES $expectedMajor client" should { + + "refuse a transform-bearing extended_stats before emission, for every family member and both binds" in { + family.foreach { fn => + Seq( + plain(fn, "YEAR(createdAt)"), + plain(fn, "ABS(salary)"), + windowed(fn, "YEAR(createdAt)"), + windowed(fn, "ABS(salary)") + ).foreach { sql => + withClue(s"[$sql] ") { + assertRefused(intercept[ElasticError](client.singleSearchToJsonQuery(single(sql)))) + } + } + } + } + + "refuse it on the sqlQueryToAggregations door too" in { + implicit val serializer: SearchBodySerializer = client.searchBodySerializer + val refusal = intercept[ElasticError] { + val aggs: Seq[ElasticAggregation] = SelectStatement(plain("STDDEV", "YEAR(createdAt)")) + aggs + } + assertRefused(refusal) + } + + "leave a raw-field extended_stats exactly as the default serializer emits it" in { + Seq(plain("STDDEV", "salary"), windowed("VAR_POP", "salary")).foreach { sql => + withClue(s"[$sql] ") { + val request: ElasticSearchRequest = requestToElasticSearchRequest(single(sql)) + request.hasTransformExtendedStats shouldBe false + client.singleSearchToJsonQuery(single(sql)) shouldBe + SearchBodySerializer.Default.serialize(request.search).replace("\"version\":true,", "") + client.singleSearchToJsonQuery(single(sql)) should include( + """"extended_stats":{"field":"salary"}""" + ) + } + } + } + + "leave every OTHER scripted metric untouched (MAX(YEAR(x)) keeps its script)" in { + client.singleSearchToJsonQuery( + single("SELECT id, MAX(YEAR(createdAt)) AS m FROM t GROUP BY id") + ) should include(""""max":{"field":"createdAt","script":{"lang":"painless"""") + } + + "inject the refusing serializer" in { + client.searchBodySerializer shouldBe RestHighLevelClientSearchBodySerializer + RestHighLevelClientSearchBodySerializer.ElasticsearchMajor shouldBe expectedMajor + } + } +} diff --git a/es7/rest/src/test/scala/app/softnetwork/elastic/client/RestHighLevelClientExtendedStatsRejectionSpec.scala b/es7/rest/src/test/scala/app/softnetwork/elastic/client/RestHighLevelClientExtendedStatsRejectionSpec.scala new file mode 100644 index 000000000..84e08d75c --- /dev/null +++ b/es7/rest/src/test/scala/app/softnetwork/elastic/client/RestHighLevelClientExtendedStatsRejectionSpec.scala @@ -0,0 +1,122 @@ +/* + * 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 app.softnetwork.elastic.client.rest.{ + RestHighLevelClientApi, + RestHighLevelClientSearchBodySerializer +} +import app.softnetwork.elastic.client.result.ElasticError +import app.softnetwork.elastic.sql.bridge._ +import app.softnetwork.elastic.sql.query.{SelectStatement, SingleSearch} +import com.typesafe.config.{Config, ConfigFactory} +import org.scalatest.matchers.should.Matchers +import org.scalatest.wordspec.AnyWordSpec + +/** Issue #222 (story BIDC-3) -- the ES 7 REST-client half: `STDDEV` / `VARIANCE` (the whole + * `extended_stats` family) over a TRANSFORMED expression is REFUSED before any JSON exists, with a + * named `ElasticError` (status 400), for BOTH binds and on BOTH serialisation doors -- never + * executed against the raw field, which is what elastic4s 7.17.x's script-dropping builder used to + * do silently. A raw-field extended_stats is untouched. + * + * No Docker: `ElasticClientCompanion` builds the underlying client lazily and `apply()` is never + * called, so nothing here touches the network. + */ +class RestHighLevelClientExtendedStatsRejectionSpec extends AnyWordSpec with Matchers { + + private val client: RestHighLevelClientApi = new RestHighLevelClientApi { + override def config: Config = ConfigFactory.load() + } + + implicit private val timestamp: Long = 1767139200000L // 2025-12-31T00:00:00Z + + private val expectedMajor = 7 + + private def single(sql: String): SingleSearch = + SelectStatement(sql).statement match { + case Some(s: SingleSearch) => s + case other => fail(s"Not a single search: $other") + } + + private val family = Seq("STDDEV", "STDDEV_SAMP", "STDDEV_POP", "VARIANCE", "VAR_SAMP", "VAR_POP") + + private def plain(fn: String, operand: String) = + s"SELECT id, $fn($operand) AS s FROM t GROUP BY id" + + private def windowed(fn: String, operand: String) = + s"SELECT id, name, $fn($operand) OVER (PARTITION BY id) AS s FROM t" + + private def assertRefused(refusal: ElasticError): Unit = { + refusal.message shouldBe RestHighLevelClientSearchBodySerializer.TransformExtendedStatsUnsupported + refusal.message should include(s"Elasticsearch $expectedMajor") + refusal.message should include("elastic4s#4100") + refusal.statusCode shouldBe Some(400) + refusal.operation shouldBe Some("search") + } + + s"the ES $expectedMajor client" should { + + "refuse a transform-bearing extended_stats before emission, for every family member and both binds" in { + family.foreach { fn => + Seq( + plain(fn, "YEAR(createdAt)"), + plain(fn, "ABS(salary)"), + windowed(fn, "YEAR(createdAt)"), + windowed(fn, "ABS(salary)") + ).foreach { sql => + withClue(s"[$sql] ") { + assertRefused(intercept[ElasticError](client.singleSearchToJsonQuery(single(sql)))) + } + } + } + } + + "refuse it on the sqlQueryToAggregations door too" in { + implicit val serializer: SearchBodySerializer = client.searchBodySerializer + val refusal = intercept[ElasticError] { + val aggs: Seq[ElasticAggregation] = SelectStatement(plain("STDDEV", "YEAR(createdAt)")) + aggs + } + assertRefused(refusal) + } + + "leave a raw-field extended_stats exactly as the default serializer emits it" in { + Seq(plain("STDDEV", "salary"), windowed("VAR_POP", "salary")).foreach { sql => + withClue(s"[$sql] ") { + val request: ElasticSearchRequest = requestToElasticSearchRequest(single(sql)) + request.hasTransformExtendedStats shouldBe false + client.singleSearchToJsonQuery(single(sql)) shouldBe + SearchBodySerializer.Default.serialize(request.search).replace("\"version\":true,", "") + client.singleSearchToJsonQuery(single(sql)) should include( + """"extended_stats":{"field":"salary"}""" + ) + } + } + } + + "leave every OTHER scripted metric untouched (MAX(YEAR(x)) keeps its script)" in { + client.singleSearchToJsonQuery( + single("SELECT id, MAX(YEAR(createdAt)) AS m FROM t GROUP BY id") + ) should include(""""max":{"field":"createdAt","script":{"lang":"painless"""") + } + + "inject the refusing serializer" in { + client.searchBodySerializer shouldBe RestHighLevelClientSearchBodySerializer + RestHighLevelClientSearchBodySerializer.ElasticsearchMajor shouldBe expectedMajor + } + } +} diff --git a/es8/java/src/test/scala/app/softnetwork/elastic/client/JavaClientExtendedStatsEmissionSpec.scala b/es8/java/src/test/scala/app/softnetwork/elastic/client/JavaClientExtendedStatsEmissionSpec.scala new file mode 100644 index 000000000..c3ef68097 --- /dev/null +++ b/es8/java/src/test/scala/app/softnetwork/elastic/client/JavaClientExtendedStatsEmissionSpec.scala @@ -0,0 +1,178 @@ +/* + * 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 app.softnetwork.elastic.client.java.{JavaClientApi, JavaClientSearchBodySerializer} +import app.softnetwork.elastic.sql.bridge._ +import app.softnetwork.elastic.sql.query.{SelectStatement, SingleSearch} +import com.fasterxml.jackson.databind.ObjectMapper +import com.typesafe.config.{Config, ConfigFactory} +import org.scalatest.matchers.should.Matchers +import org.scalatest.wordspec.AnyWordSpec + +import scala.jdk.CollectionConverters._ + +/** Issue #222 (story BIDC-3) -- the ES 8 / ES 9 Java-client half: `STDDEV` / `VARIANCE` (the whole + * `extended_stats` family) over a TRANSFORMED expression carries its script in the emitted JSON, + * on BOTH serialisation doors and for BOTH binds (plain, and windowed under a partition bucket). + * + * Before the fix the same statements emitted `"extended_stats":{"field":"createdAt"}` -- the + * statistic of the raw timestamps -- or `"extended_stats":{}` when the transform had no raw field + * behind it (T2 capture on the base commit, both bridge copies). An execution-success assertion + * cannot see either; these assert the JSON. + * + * No Docker: `ElasticClientCompanion` builds the underlying client lazily and `apply()` is never + * called, so nothing here touches the network. Kept byte-identical between the ES 8 and ES 9 + * modules (their elastic4s builders are identical). + */ +class JavaClientExtendedStatsEmissionSpec extends AnyWordSpec with Matchers { + + private val client: JavaClientApi = new JavaClientApi { + override def config: Config = ConfigFactory.load() + } + + implicit private val timestamp: Long = 1767139200000L // 2025-12-31T00:00:00Z + + private val mapper = new ObjectMapper() + + private def single(sql: String): SingleSearch = + SelectStatement(sql).statement match { + case Some(s: SingleSearch) => s + case other => fail(s"Not a single search: $other") + } + + /** Door 1 -- what the client sends: `singleSearchToJsonQuery`. */ + private def emitted(sql: String): String = client.singleSearchToJsonQuery(single(sql)) + + private val family = Seq("STDDEV", "STDDEV_SAMP", "STDDEV_POP", "VARIANCE", "VAR_SAMP", "VAR_POP") + + private def plain(fn: String, operand: String) = + s"SELECT id, $fn($operand) AS s FROM t GROUP BY id" + + private def windowed(fn: String, operand: String) = + s"SELECT id, name, $fn($operand) OVER (PARTITION BY id) AS s FROM t" + + private val yearScript = + """"script":{"lang":"painless","source":"def param1 = (doc['createdAt'].size() == 0 ? null : """ + + """doc['createdAt'].value.toInstant().atZone(ZoneId.of('Z')).get(ChronoField.YEAR)); param1"}""" + + private val absScript = + """"script":{"lang":"painless","source":"def param1 = (doc['salary'].size() == 0 ? null : """ + + """doc['salary'].value); (param1 == null) ? null : Double.valueOf(Math.abs(param1))"}""" + + private val partition = """"terms":{"field":"id","size":65536,"min_doc_count":1}""" + + "the ES 8/9 client" should { + + "emit the script of STDDEV over a field-derived transform (plain bind) -- issue #222" in { + emitted(plain("STDDEV", "YEAR(createdAt)")) shouldBe + s"""{"query":{"match_all":{}},"size":0,"_source":false,"aggs":{"id":{$partition,""" + + s""""aggs":{"s":{"extended_stats":{"field":"createdAt",$yearScript}}}}}}""" + } + + "emit a pure script when the transform has no raw field behind it (ABS(salary))" in { + // Before: `"extended_stats":{}` -- neither field nor script, rejected by Elasticsearch. + emitted(plain("VARIANCE", "ABS(salary)")) shouldBe + s"""{"query":{"match_all":{}},"size":0,"_source":false,"aggs":{"id":{$partition,""" + + s""""aggs":{"s":{"extended_stats":{$absScript}}}}}}""" + } + + "emit the script on the WINDOWED bind (under the partition bucket)" in { + emitted(windowed("STDDEV", "YEAR(createdAt)")) shouldBe + s"""{"query":{"match_all":{}},"size":0,"_source":false,"aggs":{"id":{$partition,""" + + s""""aggs":{"s":{"extended_stats":{"field":"createdAt",$yearScript}}}}}}""" + } + + "emit the script for EVERY family member, plain and windowed" in { + family.foreach { fn => + Seq(plain(fn, "YEAR(createdAt)"), windowed(fn, "YEAR(createdAt)")).foreach { sql => + withClue(s"[$sql] ") { + val json = emitted(sql) + json should include(""""extended_stats":{"field":"createdAt","script":{""") + json should include("ChronoField.YEAR") + json should not include """"extended_stats":{"field":"createdAt"}""" + } + } + Seq(plain(fn, "ABS(salary)"), windowed(fn, "ABS(salary)")).foreach { sql => + withClue(s"[$sql] ") { + val json = emitted(sql) + json should include(""""extended_stats":{"script":{""") + json should not include """"extended_stats":{}""" + } + } + } + } + + "emit the script on the sqlQueryToAggregations door too" in { + implicit val serializer: SearchBodySerializer = client.searchBodySerializer + val aggs: Seq[ElasticAggregation] = SelectStatement(plain("STDDEV_POP", "YEAR(createdAt)")) + aggs should have size 1 + aggs.head.hasTransformExtendedStats shouldBe true + aggs.head.query shouldBe Some( + s"""{"query":{"match_all":{}},"size":0,"aggs":{"s":{"extended_stats":{"field":"createdAt",$yearScript}}}}""" + ) + } + + "never carry a null-unsafe transform script (lead directive 2026-09-06)" in { + // The scripts reaching Elasticsearch for the first time through this module are the metric + // scripts the other aggregates already use: a `def paramN = (doc[..].size() == 0 ? null : ..)` + // preamble and a null-guarded result. Nothing dereferences a `? null :` group. + family.foreach { fn => + Seq(plain(fn, "YEAR(createdAt)"), windowed(fn, "ABS(salary)")).foreach { sql => + withClue(s"[$sql] ") { + val root = mapper.readTree(emitted(sql)) + val scripts = root.findValues("script").asScala.flatMap { s => + Option(s.get("source")).map(_.asText()) + } + scripts should not be empty + scripts.foreach { script => + script should not include ")." + script should not include "doc['createdAt'].value.toInstant().atZone(ZoneId.of('Z')).get(ChronoField.YEAR)." + } + } + } + } + } + } + + "JavaClientSearchBodySerializer" should { + + "be byte-identical to the default serializer for any request without a transform-bearing extended_stats" in { + Seq( + plain("STDDEV", "salary"), + windowed("VAR_POP", "salary"), + "SELECT id, MAX(YEAR(createdAt)) AS m FROM t GROUP BY id", + "SELECT id, COUNT(x) AS c FROM t GROUP BY id HAVING MAX(YEAR(createdAt)) > 2020", + "SELECT id, MAX(x) - MIN(x) AS d FROM t GROUP BY id", + """SELECT c.id, COUNT(DISTINCT e.address) AS nb FROM customers c JOIN UNNEST(c.emails) AS e + |GROUP BY c.id HAVING COUNT(DISTINCT e.address) > 1""".stripMargin, + "SELECT name, salary FROM t WHERE salary > 10 ORDER BY salary DESC LIMIT 5" + ).foreach { sql => + withClue(s"[$sql] ") { + val request: ElasticSearchRequest = requestToElasticSearchRequest(single(sql)) + request.hasTransformExtendedStats shouldBe false + JavaClientSearchBodySerializer.serialize(request.search) shouldBe + SearchBodySerializer.Default.serialize(request.search) + } + } + } + + "be the serializer the client injects" in { + client.searchBodySerializer shouldBe JavaClientSearchBodySerializer + } + } +} diff --git a/es9/java/src/test/scala/app/softnetwork/elastic/client/JavaClientExtendedStatsEmissionSpec.scala b/es9/java/src/test/scala/app/softnetwork/elastic/client/JavaClientExtendedStatsEmissionSpec.scala new file mode 100644 index 000000000..c3ef68097 --- /dev/null +++ b/es9/java/src/test/scala/app/softnetwork/elastic/client/JavaClientExtendedStatsEmissionSpec.scala @@ -0,0 +1,178 @@ +/* + * 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 app.softnetwork.elastic.client.java.{JavaClientApi, JavaClientSearchBodySerializer} +import app.softnetwork.elastic.sql.bridge._ +import app.softnetwork.elastic.sql.query.{SelectStatement, SingleSearch} +import com.fasterxml.jackson.databind.ObjectMapper +import com.typesafe.config.{Config, ConfigFactory} +import org.scalatest.matchers.should.Matchers +import org.scalatest.wordspec.AnyWordSpec + +import scala.jdk.CollectionConverters._ + +/** Issue #222 (story BIDC-3) -- the ES 8 / ES 9 Java-client half: `STDDEV` / `VARIANCE` (the whole + * `extended_stats` family) over a TRANSFORMED expression carries its script in the emitted JSON, + * on BOTH serialisation doors and for BOTH binds (plain, and windowed under a partition bucket). + * + * Before the fix the same statements emitted `"extended_stats":{"field":"createdAt"}` -- the + * statistic of the raw timestamps -- or `"extended_stats":{}` when the transform had no raw field + * behind it (T2 capture on the base commit, both bridge copies). An execution-success assertion + * cannot see either; these assert the JSON. + * + * No Docker: `ElasticClientCompanion` builds the underlying client lazily and `apply()` is never + * called, so nothing here touches the network. Kept byte-identical between the ES 8 and ES 9 + * modules (their elastic4s builders are identical). + */ +class JavaClientExtendedStatsEmissionSpec extends AnyWordSpec with Matchers { + + private val client: JavaClientApi = new JavaClientApi { + override def config: Config = ConfigFactory.load() + } + + implicit private val timestamp: Long = 1767139200000L // 2025-12-31T00:00:00Z + + private val mapper = new ObjectMapper() + + private def single(sql: String): SingleSearch = + SelectStatement(sql).statement match { + case Some(s: SingleSearch) => s + case other => fail(s"Not a single search: $other") + } + + /** Door 1 -- what the client sends: `singleSearchToJsonQuery`. */ + private def emitted(sql: String): String = client.singleSearchToJsonQuery(single(sql)) + + private val family = Seq("STDDEV", "STDDEV_SAMP", "STDDEV_POP", "VARIANCE", "VAR_SAMP", "VAR_POP") + + private def plain(fn: String, operand: String) = + s"SELECT id, $fn($operand) AS s FROM t GROUP BY id" + + private def windowed(fn: String, operand: String) = + s"SELECT id, name, $fn($operand) OVER (PARTITION BY id) AS s FROM t" + + private val yearScript = + """"script":{"lang":"painless","source":"def param1 = (doc['createdAt'].size() == 0 ? null : """ + + """doc['createdAt'].value.toInstant().atZone(ZoneId.of('Z')).get(ChronoField.YEAR)); param1"}""" + + private val absScript = + """"script":{"lang":"painless","source":"def param1 = (doc['salary'].size() == 0 ? null : """ + + """doc['salary'].value); (param1 == null) ? null : Double.valueOf(Math.abs(param1))"}""" + + private val partition = """"terms":{"field":"id","size":65536,"min_doc_count":1}""" + + "the ES 8/9 client" should { + + "emit the script of STDDEV over a field-derived transform (plain bind) -- issue #222" in { + emitted(plain("STDDEV", "YEAR(createdAt)")) shouldBe + s"""{"query":{"match_all":{}},"size":0,"_source":false,"aggs":{"id":{$partition,""" + + s""""aggs":{"s":{"extended_stats":{"field":"createdAt",$yearScript}}}}}}""" + } + + "emit a pure script when the transform has no raw field behind it (ABS(salary))" in { + // Before: `"extended_stats":{}` -- neither field nor script, rejected by Elasticsearch. + emitted(plain("VARIANCE", "ABS(salary)")) shouldBe + s"""{"query":{"match_all":{}},"size":0,"_source":false,"aggs":{"id":{$partition,""" + + s""""aggs":{"s":{"extended_stats":{$absScript}}}}}}""" + } + + "emit the script on the WINDOWED bind (under the partition bucket)" in { + emitted(windowed("STDDEV", "YEAR(createdAt)")) shouldBe + s"""{"query":{"match_all":{}},"size":0,"_source":false,"aggs":{"id":{$partition,""" + + s""""aggs":{"s":{"extended_stats":{"field":"createdAt",$yearScript}}}}}}""" + } + + "emit the script for EVERY family member, plain and windowed" in { + family.foreach { fn => + Seq(plain(fn, "YEAR(createdAt)"), windowed(fn, "YEAR(createdAt)")).foreach { sql => + withClue(s"[$sql] ") { + val json = emitted(sql) + json should include(""""extended_stats":{"field":"createdAt","script":{""") + json should include("ChronoField.YEAR") + json should not include """"extended_stats":{"field":"createdAt"}""" + } + } + Seq(plain(fn, "ABS(salary)"), windowed(fn, "ABS(salary)")).foreach { sql => + withClue(s"[$sql] ") { + val json = emitted(sql) + json should include(""""extended_stats":{"script":{""") + json should not include """"extended_stats":{}""" + } + } + } + } + + "emit the script on the sqlQueryToAggregations door too" in { + implicit val serializer: SearchBodySerializer = client.searchBodySerializer + val aggs: Seq[ElasticAggregation] = SelectStatement(plain("STDDEV_POP", "YEAR(createdAt)")) + aggs should have size 1 + aggs.head.hasTransformExtendedStats shouldBe true + aggs.head.query shouldBe Some( + s"""{"query":{"match_all":{}},"size":0,"aggs":{"s":{"extended_stats":{"field":"createdAt",$yearScript}}}}""" + ) + } + + "never carry a null-unsafe transform script (lead directive 2026-09-06)" in { + // The scripts reaching Elasticsearch for the first time through this module are the metric + // scripts the other aggregates already use: a `def paramN = (doc[..].size() == 0 ? null : ..)` + // preamble and a null-guarded result. Nothing dereferences a `? null :` group. + family.foreach { fn => + Seq(plain(fn, "YEAR(createdAt)"), windowed(fn, "ABS(salary)")).foreach { sql => + withClue(s"[$sql] ") { + val root = mapper.readTree(emitted(sql)) + val scripts = root.findValues("script").asScala.flatMap { s => + Option(s.get("source")).map(_.asText()) + } + scripts should not be empty + scripts.foreach { script => + script should not include ")." + script should not include "doc['createdAt'].value.toInstant().atZone(ZoneId.of('Z')).get(ChronoField.YEAR)." + } + } + } + } + } + } + + "JavaClientSearchBodySerializer" should { + + "be byte-identical to the default serializer for any request without a transform-bearing extended_stats" in { + Seq( + plain("STDDEV", "salary"), + windowed("VAR_POP", "salary"), + "SELECT id, MAX(YEAR(createdAt)) AS m FROM t GROUP BY id", + "SELECT id, COUNT(x) AS c FROM t GROUP BY id HAVING MAX(YEAR(createdAt)) > 2020", + "SELECT id, MAX(x) - MIN(x) AS d FROM t GROUP BY id", + """SELECT c.id, COUNT(DISTINCT e.address) AS nb FROM customers c JOIN UNNEST(c.emails) AS e + |GROUP BY c.id HAVING COUNT(DISTINCT e.address) > 1""".stripMargin, + "SELECT name, salary FROM t WHERE salary > 10 ORDER BY salary DESC LIMIT 5" + ).foreach { sql => + withClue(s"[$sql] ") { + val request: ElasticSearchRequest = requestToElasticSearchRequest(single(sql)) + request.hasTransformExtendedStats shouldBe false + JavaClientSearchBodySerializer.serialize(request.search) shouldBe + SearchBodySerializer.Default.serialize(request.search) + } + } + } + + "be the serializer the client injects" in { + client.searchBodySerializer shouldBe JavaClientSearchBodySerializer + } + } +} diff --git a/testkit/src/main/scala/app/softnetwork/elastic/client/WindowFunctionSpec.scala b/testkit/src/main/scala/app/softnetwork/elastic/client/WindowFunctionSpec.scala index eadd93e1e..014c51104 100644 --- a/testkit/src/main/scala/app/softnetwork/elastic/client/WindowFunctionSpec.scala +++ b/testkit/src/main/scala/app/softnetwork/elastic/client/WindowFunctionSpec.scala @@ -22,6 +22,7 @@ import app.softnetwork.elastic.client.scroll.ScrollConfig import app.softnetwork.elastic.client.spi.ElasticClientFactory import app.softnetwork.elastic.model.window._ import app.softnetwork.elastic.scalatest.ElasticDockerTestKit +import app.softnetwork.elastic.sql.query.SelectStatement import org.scalatest.flatspec.AnyFlatSpecLike import org.scalatest.matchers.should.Matchers import org.slf4j.{Logger, LoggerFactory} @@ -2112,4 +2113,153 @@ trait WindowFunctionSpec } } + // ======================================================================== + // ISSUE #222 (story BIDC-3) — STDDEV / VARIANCE over a TRANSFORMED expression + // + // Elasticsearch 8 / 9: the statistic is computed over the transform (the emitted extended_stats + // carries its script); the oracle is computed from the fixture itself and sits ten orders of + // magnitude away from the statistic of the raw `hire_date` millis the query used to compute + // silently. Elasticsearch 6 / 7: the query is REFUSED with a named 400 -- never executed against + // the raw field. An execution-success assertion is explicitly insufficient here (issue #222). + // ======================================================================== + + def elasticsearchMajor: Int = + client.asInstanceOf[VersionApi].version match { + case ElasticSuccess(v) => v.split("\\.").head.toInt + case ElasticFailure(error) => + fail(s"Failed to retrieve Elasticsearch version: ${error.message}") + } + + /** The hire YEARS per department, read from the fixture rows -- the oracle's input. */ + private def hireYearsByDepartment: Map[String, Seq[Int]] = + client.searchAs[Employee]( + "SELECT name, department, location, salary, hire_date, level, skills, id FROM emp LIMIT 100" + ) match { + case ElasticSuccess(rows) => + rows.groupBy(_.department).map { case (dept, emps) => + dept -> emps.map(_.hire_date.take(4).toInt) + } + case ElasticFailure(error) => fail(s"Fixture read failed: ${error.message}") + } + + private def populationVariance(xs: Seq[Int]): Double = { + val mean = xs.sum.toDouble / xs.size + xs.map(x => (x - mean) * (x - mean)).sum / xs.size + } + + private def sampleVariance(xs: Seq[Int]): Double = { + val mean = xs.sum.toDouble / xs.size + xs.map(x => (x - mean) * (x - mean)).sum / (xs.size - 1) + } + + private val transformedStatsSql = + """SELECT department, + | STDDEV(YEAR(hire_date)) AS sd_year, + | VAR_SAMP(YEAR(hire_date)) AS vs_year, + | VAR_POP(YEAR(hire_date)) AS vp_year, + | STDDEV_POP(YEAR(hire_date)) AS sdp_year + |FROM emp + |GROUP BY department""".stripMargin + + private val windowedTransformedStatsSql = + """SELECT department, name, hire_date, + | STDDEV(YEAR(hire_date)) OVER (PARTITION BY department) AS sd_year + |FROM emp + |LIMIT 100""".stripMargin + + /** ES 6 / 7: the same statement is refused on the gateway route (REPL / JDBC / Arrow -- an honest + * 400 naming the major) AND on the direct client API (the same `ElasticError`, thrown). + */ + private def assertRefusedOnThisMajor(sql: String): Unit = { + val major = elasticsearchMajor + Await.result(client.run(sql), 30.seconds) match { + case ElasticFailure(error) => + error.statusCode shouldBe Some(400) + error.message should include(s"Elasticsearch $major") + error.message should include("elastic4s#4100") + error.message should include("transformed expression") + log.info(s" ✓ ES $major refused: ${error.message}") + case other => + fail(s"a transform-bearing STDDEV must be refused on Elasticsearch $major, got $other") + } + val direct = intercept[app.softnetwork.elastic.client.result.ElasticError]( + client.searchAsUnchecked[DepartmentYearStats](SelectStatement(sql)) + ) + direct.statusCode shouldBe Some(400) + direct.message should include(s"Elasticsearch $major") + } + + "STDDEV / VARIANCE over a transformed expression" should "compute the statistic over the transform on ES 8+ and refuse loudly on ES 6/7 (issue #222)" in { + if (elasticsearchMajor >= 8) { + val years = hireYearsByDepartment + client.searchAs[DepartmentYearStats]( + """SELECT department, + | STDDEV(YEAR(hire_date)) AS sd_year, + | VAR_SAMP(YEAR(hire_date)) AS vs_year, + | VAR_POP(YEAR(hire_date)) AS vp_year, + | STDDEV_POP(YEAR(hire_date)) AS sdp_year + |FROM emp + |GROUP BY department""".stripMargin + ) match { + case ElasticSuccess(rows) => + rows.map(_.department).toSet shouldBe years.keySet + rows.foreach { r => + val xs = years(r.department) + withClue(s"${r.department} years=$xs ") { + r.vp_year.get shouldBe populationVariance(xs) +- 1e-6 + r.sdp_year.get shouldBe math.sqrt(populationVariance(xs)) +- 1e-6 + r.vs_year.get shouldBe sampleVariance(xs) +- 1e-6 + r.sd_year.get shouldBe math.sqrt(sampleVariance(xs)) +- 1e-6 + // The statistic of the RAW field is over epoch millis (~1e10): never this. + r.sd_year.get should be < 100.0 + log.info( + f"${r.department}%-12s sd=${r.sd_year.get}%8.4f vs=${r.vs_year.get}%8.4f " + + f"vp=${r.vp_year.get}%8.4f sdp=${r.sdp_year.get}%8.4f (years=$xs)" + ) + } + } + // Engineering hire years: 2019, 2018, 2020, 2017, 2021, 2016, 2015 -> mean 2018, SS 28. + val eng = rows.find(_.department == "Engineering").getOrElse(fail("no Engineering row")) + eng.vp_year.get shouldBe 4.0 +- 1e-6 + eng.sdp_year.get shouldBe 2.0 +- 1e-6 + eng.vs_year.get shouldBe 28.0 / 6 +- 1e-6 + eng.sd_year.get shouldBe math.sqrt(28.0 / 6) +- 1e-6 + + case ElasticFailure(error) => + fail(s"Query failed: ${error.message}") + } + } else { + assertRefusedOnThisMajor(transformedStatsSql) + } + } + + it should "compute the WINDOWED statistic over the transform on ES 8+ and refuse loudly on ES 6/7 (issue #222)" in { + if (elasticsearchMajor >= 8) { + val years = hireYearsByDepartment + client.searchAs[EmployeeYearStats]( + """SELECT department, name, hire_date, + | STDDEV(YEAR(hire_date)) OVER (PARTITION BY department) AS sd_year + |FROM emp + |LIMIT 100""".stripMargin + ) match { + case ElasticSuccess(rows) => + rows should have size 20 + rows.groupBy(_.department).foreach { case (dept, emps) => + val values = emps.flatMap(_.sd_year).distinct + withClue(s"$dept years=${years(dept)} ") { + values should have size 1 + values.head shouldBe math.sqrt(sampleVariance(years(dept))) +- 1e-6 + values.head should be < 100.0 + log.info(f" ✓ $dept%-12s windowed sd(YEAR(hire_date)) = ${values.head}%8.4f") + } + } + + case ElasticFailure(error) => + fail(s"Query failed: ${error.message}") + } + } else { + assertRefusedOnThisMajor(windowedTransformedStatsSql) + } + } + } diff --git a/testkit/src/main/scala/app/softnetwork/elastic/model/window/package.scala b/testkit/src/main/scala/app/softnetwork/elastic/model/window/package.scala index a9b830850..746f816e9 100644 --- a/testkit/src/main/scala/app/softnetwork/elastic/model/window/package.scala +++ b/testkit/src/main/scala/app/softnetwork/elastic/model/window/package.scala @@ -72,6 +72,28 @@ package object window { vs_salary: Option[Double] = None ) + /** Issue #222 (story BIDC-3) — the `extended_stats` family over a TRANSFORMED expression + * (`YEAR(hire_date)`), per department. Four projections of one `extended_stats` whose value is + * the year, not the raw `hire_date` millis. + */ + case class DepartmentYearStats( + department: String, + sd_year: Option[Double] = None, + vs_year: Option[Double] = None, + vp_year: Option[Double] = None, + sdp_year: Option[Double] = None + ) + + /** Issue #222 (story BIDC-3) — the windowed twin: every row carries its department's sample + * standard deviation of `YEAR(hire_date)`. + */ + case class EmployeeYearStats( + department: String, + name: String, + hire_date: String, + sd_year: Option[Double] = None + ) + /** Story 14.5 — PERCENTILE_CONT integration shape. Each field is one percentile projected from a * single ES `percentiles` aggregation per call. */ From 8c0c93564eca9a3ed6b3329a438f335b44ebbae5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Manciot?= Date: Sun, 6 Sep 2026 07:32:54 +0200 Subject: [PATCH 03/11] wip(core): pin the lazy (windowed scroll) refusal surface in GatewayRefusalBoundarySpec (#222) Story BIDC-3 Co-Authored-By: Claude Fable 5.1 --- .../client/GatewayRefusalBoundarySpec.scala | 36 +++++++++++++------ 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/core/src/test/scala/app/softnetwork/elastic/client/GatewayRefusalBoundarySpec.scala b/core/src/test/scala/app/softnetwork/elastic/client/GatewayRefusalBoundarySpec.scala index 2831cd51e..c53447b29 100644 --- a/core/src/test/scala/app/softnetwork/elastic/client/GatewayRefusalBoundarySpec.scala +++ b/core/src/test/scala/app/softnetwork/elastic/client/GatewayRefusalBoundarySpec.scala @@ -1,6 +1,7 @@ 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.SingleSearch @@ -10,6 +11,7 @@ import org.scalatest.flatspec.AnyFlatSpec import org.scalatest.matchers.should.Matchers import org.slf4j.{Logger, LoggerFactory} +import scala.concurrent.Await import scala.concurrent.duration._ /** Issue #222 (story BIDC-3) -- the ONE core boundary the per-major refusal relies on. @@ -19,10 +21,12 @@ import scala.concurrent.duration._ * `ElasticError` (the ES 6 / ES 7 modules do so for STDDEV / VARIANCE over a transformed * expression). `GatewayApi.run` must surface that refusal as the `ElasticFailure` every other error * is -- on EVERY route a DQL statement can take: the `SearchExecutor` (aggregation-shaped, or an - * explicit LIMIT) AND the `CoreDqlExtension`'s quota-capped scroll (an un-LIMITed row query, plain - * or windowed), which calls `client.scroll` directly and never enters the executor. That second - * route is why the boundary lives in `run`, not in `SearchExecutor`: a first cut there let the - * refusal escape as a raw exception on exactly the path BI tools take for a plain projection. + * explicit LIMIT) AND the `CoreDqlExtension`'s quota-capped scroll (an un-LIMITed row query), + * which calls `client.scroll` directly and never enters the executor. That second route is why + * the boundary lives in `run`, not in `SearchExecutor`: a first cut there let the refusal escape + * as a raw exception on exactly the path BI tools take for a plain projection. One route + * translates LAZILY (the windowed row query's scroll): there the refusal fails the stream when it + * is materialised -- loud, never silent -- and that surface is pinned too. * * Docker-free: a `NopeClientApi` whose `singleSearchToJsonQuery` refuses like a client module. */ @@ -84,12 +88,24 @@ class GatewayRefusalBoundarySpec assertRefused(client.run("SELECT id, name FROM t").futureValue) } - it should "surface it on the windowed row route (un-LIMITed STDDEV OVER PARTITION BY)" in { - assertRefused( - client - .run("SELECT id, name, STDDEV(YEAR(createdAt)) OVER (PARTITION BY id) AS s FROM t") - .futureValue - ) + it should "surface it on the windowed row route (un-LIMITed STDDEV OVER PARTITION BY) when the stream runs" in { + // The quota-capped scroll of a WINDOWED row query builds its source lazily + // (`scrollWithWindowEnrichment` -> `Source.futureSource`), so the translation -- and the + // refusal -- happen when the stream is materialised, not when `run` returns. The refusal is + // still the same status-bearing ElasticError, and it FAILS the stream: nothing is returned + // silently. Pinned here so the surface is known, not assumed. + client + .run("SELECT id, name, STDDEV(YEAR(createdAt)) OVER (PARTITION BY id) AS s FROM t") + .futureValue match { + case ElasticFailure(error) => + error.message shouldBe refusalMessage + error.statusCode shouldBe Some(400) + case ElasticSuccess(QueryStream(stream, _)) => + val failure = intercept[ElasticError](Await.result(stream.runWith(Sink.seq), 5.seconds)) + failure.message shouldBe refusalMessage + failure.statusCode shouldBe Some(400) + case other => fail(s"a refused translation must fail, got $other") + } } it should "not manufacture a refusal for a client that does not refuse" in { From 9077e09221305676aaa12e9c077820d3b0deaa25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Manciot?= Date: Sun, 6 Sep 2026 11:16:36 +0200 Subject: [PATCH 04/11] wip(core): translate the windowed scroll's aggregation request eagerly so a refusal is answered at run (#222) Story BIDC-3 Co-Authored-By: Claude Fable 5.1 --- .../elastic/client/ScrollApi.scala | 28 ++++++++------ .../elastic/client/SearchApi.scala | 37 +++++++++++++----- .../client/GatewayRefusalBoundarySpec.scala | 38 ++++++++----------- 3 files changed, 59 insertions(+), 44 deletions(-) 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..c42d21541 100644 --- a/core/src/main/scala/app/softnetwork/elastic/client/ScrollApi.scala +++ b/core/src/main/scala/app/softnetwork/elastic/client/ScrollApi.scala @@ -719,12 +719,23 @@ trait ScrollApi extends ElasticClientHelpers { logger.info(s"🪟 Scrolling with ${request.windowFunctions.size} window functions") - // Execute window aggregations first - val windowCacheFuture: Future[ElasticResult[WindowCache]] = - Future(executeWindowAggregations(request)) + // Translate BOTH requests on the calling thread, before any Future or Source exists (issue + // #222): a client module's refusal (a status-bearing ElasticError raised by the translation) + // must reach `GatewayApi.run` as an ElasticFailure, exactly as on the one-shot routes -- not + // sit inside a lazily-built stream that only fails once someone materialises it. + val (aggRequest, aggQuery) = windowAggregationQuery(request) // Create base query without window functions val baseQuery = createBaseQuery(request) + val baseElasticQuery = ElasticQuery( + baseQuery, + collection.immutable.Seq(baseQuery.sources: _*), + sql = Some(baseQuery.sql) + ) + + // Execute window aggregations first + val windowCacheFuture: Future[ElasticResult[WindowCache]] = + Future(executeWindowAggregations(request, aggRequest, aggQuery)) // Stream and enrich val outputFields = extractOutputFieldNames(request) @@ -740,11 +751,7 @@ trait ScrollApi extends ElasticClientHelpers { windowCacheFuture.map { case ElasticSuccess(cache) => scrollWithMetrics( - ElasticQuery( - baseQuery, - collection.immutable.Seq(baseQuery.sources: _*), - sql = Some(baseQuery.sql) - ), + baseElasticQuery, baseQuery.fieldAliases, baseQuery.sqlAggregations, // The base rows must carry `_id`: the ordinal lookup below matches each row to @@ -774,10 +781,7 @@ trait ScrollApi extends ElasticClientHelpers { // Fallback: return base results without enrichment logger.warn("⚠️ Falling back to base results without window enrichment") scrollWithMetrics( - ElasticQuery( - baseQuery, - collection.immutable.Seq(baseQuery.sources: _*) - ), + baseElasticQuery, baseQuery.fieldAliases, baseQuery.sqlAggregations, config.copy(retainDocumentId = shouldKeepDocumentId), 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..651935004 100644 --- a/core/src/main/scala/app/softnetwork/elastic/client/SearchApi.scala +++ b/core/src/main/scala/app/softnetwork/elastic/client/SearchApi.scala @@ -1379,20 +1379,39 @@ trait SearchApi extends ElasticConversion with ElasticClientHelpers { protected def executeWindowAggregations( request: SingleSearch )(implicit timestamp: Long, context: ConversionContext): ElasticResult[WindowCache] = { + val (aggRequest, elasticQuery) = windowAggregationQuery(request) + executeWindowAggregations(request, aggRequest, elasticQuery) + } - // Build aggregation request + /** The window-aggregation request of `request` and its TRANSLATION to an Elasticsearch body. + * + * Kept separate from the execution (issue #222): the translation is where a client module may + * REFUSE the statement (a status-bearing `ElasticError`, e.g. STDDEV / VARIANCE over a transformed + * expression on ES 6 / ES 7), and a refusal known at translation time must be answered at + * `GatewayApi.run` -- so a caller that executes asynchronously translates here FIRST, on the + * calling thread, and only then schedules the execution. + */ + private[client] def windowAggregationQuery( + request: SingleSearch + )(implicit timestamp: Long): (SingleSearch, ElasticQuery) = { val aggRequest = buildWindowAggregationRequest(request) - val sql = aggRequest.sql - - logger.info( - s"🔍 Executing window aggregation query:\n$sql" - ) - - // Execute aggregation using existing search infrastructure val elasticQuery = ElasticQuery( aggRequest, collection.immutable.Seq(aggRequest.sources: _*), - sql = Some(sql) + sql = Some(aggRequest.sql) + ) + (aggRequest, elasticQuery) + } + + /** Execute an already-translated window-aggregation request -- see [[windowAggregationQuery]]. */ + private[client] def executeWindowAggregations( + request: SingleSearch, + aggRequest: SingleSearch, + elasticQuery: ElasticQuery + )(implicit context: ConversionContext): ElasticResult[WindowCache] = { + + logger.info( + s"🔍 Executing window aggregation query:\n${aggRequest.sql}" ) for { diff --git a/core/src/test/scala/app/softnetwork/elastic/client/GatewayRefusalBoundarySpec.scala b/core/src/test/scala/app/softnetwork/elastic/client/GatewayRefusalBoundarySpec.scala index c53447b29..cc7923d38 100644 --- a/core/src/test/scala/app/softnetwork/elastic/client/GatewayRefusalBoundarySpec.scala +++ b/core/src/test/scala/app/softnetwork/elastic/client/GatewayRefusalBoundarySpec.scala @@ -1,7 +1,6 @@ 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.SingleSearch @@ -11,7 +10,6 @@ import org.scalatest.flatspec.AnyFlatSpec import org.scalatest.matchers.should.Matchers import org.slf4j.{Logger, LoggerFactory} -import scala.concurrent.Await import scala.concurrent.duration._ /** Issue #222 (story BIDC-3) -- the ONE core boundary the per-major refusal relies on. @@ -24,9 +22,10 @@ import scala.concurrent.duration._ * explicit LIMIT) AND the `CoreDqlExtension`'s quota-capped scroll (an un-LIMITed row query), * which calls `client.scroll` directly and never enters the executor. That second route is why * the boundary lives in `run`, not in `SearchExecutor`: a first cut there let the refusal escape - * as a raw exception on exactly the path BI tools take for a plain projection. One route - * translates LAZILY (the windowed row query's scroll): there the refusal fails the stream when it - * is materialised -- loud, never silent -- and that surface is pinned too. + * as a raw exception on exactly the path BI tools take for a plain projection. The windowed row + * query's scroll used to translate LAZILY (inside the stream's Future); its translation is now + * hoisted onto the calling thread so the contract is uniform: a refusal known at translation time + * is answered at `run`, on every route. * * Docker-free: a `NopeClientApi` whose `singleSearchToJsonQuery` refuses like a client module. */ @@ -88,24 +87,17 @@ class GatewayRefusalBoundarySpec assertRefused(client.run("SELECT id, name FROM t").futureValue) } - it should "surface it on the windowed row route (un-LIMITed STDDEV OVER PARTITION BY) when the stream runs" in { - // The quota-capped scroll of a WINDOWED row query builds its source lazily - // (`scrollWithWindowEnrichment` -> `Source.futureSource`), so the translation -- and the - // refusal -- happen when the stream is materialised, not when `run` returns. The refusal is - // still the same status-bearing ElasticError, and it FAILS the stream: nothing is returned - // silently. Pinned here so the surface is known, not assumed. - client - .run("SELECT id, name, STDDEV(YEAR(createdAt)) OVER (PARTITION BY id) AS s FROM t") - .futureValue match { - case ElasticFailure(error) => - error.message shouldBe refusalMessage - error.statusCode shouldBe Some(400) - case ElasticSuccess(QueryStream(stream, _)) => - val failure = intercept[ElasticError](Await.result(stream.runWith(Sink.seq), 5.seconds)) - failure.message shouldBe refusalMessage - failure.statusCode shouldBe Some(400) - case other => fail(s"a refused translation must fail, got $other") - } + it should "surface it on the windowed row route (un-LIMITed STDDEV OVER PARTITION BY)" in { + // The quota-capped scroll of a WINDOWED row query executes its window aggregations inside a + // Future and builds its source lazily. The TRANSLATION, where the refusal is raised, runs on + // the calling thread BEFORE either exists (ScrollApi.scrollWithWindowEnrichment), so the + // refusal is answered here, at `run` -- a first cut deferred it into the stream, where it only + // surfaced once somebody materialised the source. + assertRefused( + client + .run("SELECT id, name, STDDEV(YEAR(createdAt)) OVER (PARTITION BY id) AS s FROM t") + .futureValue + ) } it should "not manufacture a refusal for a client that does not refuse" in { From 77582ed208bd7d5650bfe4f322685617d2bc74e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Manciot?= Date: Sun, 6 Sep 2026 11:21:51 +0200 Subject: [PATCH 05/11] wip(es8,es9): the emission spec asserts the marker script is rendered verbatim; null-safety has one owner (#222) Story BIDC-3 Co-Authored-By: Claude Fable 5.1 --- .../JavaClientExtendedStatsEmissionSpec.scala | 30 ++++++++++++------- .../JavaClientExtendedStatsEmissionSpec.scala | 30 ++++++++++++------- 2 files changed, 38 insertions(+), 22 deletions(-) diff --git a/es8/java/src/test/scala/app/softnetwork/elastic/client/JavaClientExtendedStatsEmissionSpec.scala b/es8/java/src/test/scala/app/softnetwork/elastic/client/JavaClientExtendedStatsEmissionSpec.scala index c3ef68097..5da3bf018 100644 --- a/es8/java/src/test/scala/app/softnetwork/elastic/client/JavaClientExtendedStatsEmissionSpec.scala +++ b/es8/java/src/test/scala/app/softnetwork/elastic/client/JavaClientExtendedStatsEmissionSpec.scala @@ -20,6 +20,7 @@ import app.softnetwork.elastic.client.java.{JavaClientApi, JavaClientSearchBodyS import app.softnetwork.elastic.sql.bridge._ import app.softnetwork.elastic.sql.query.{SelectStatement, SingleSearch} import com.fasterxml.jackson.databind.ObjectMapper +import com.sksamuel.elastic4s.requests.searches.aggs.{AbstractAggregation, Aggregation} import com.typesafe.config.{Config, ConfigFactory} import org.scalatest.matchers.should.Matchers import org.scalatest.wordspec.AnyWordSpec @@ -76,6 +77,14 @@ class JavaClientExtendedStatsEmissionSpec extends AnyWordSpec with Matchers { private val partition = """"terms":{"field":"id","size":65536,"min_doc_count":1}""" + /** The script of every ScriptedExtendedStatsAggregation marker in the aggregation tree. */ + private def markerScriptsOf(aggs: Iterable[AbstractAggregation]): Seq[String] = + aggs.toSeq.flatMap { + case m: ScriptedExtendedStatsAggregation => m.inner.script.map(_.script).toSeq + case a: Aggregation => markerScriptsOf(a.subaggs) + case _ => Seq.empty + } + "the ES 8/9 client" should { "emit the script of STDDEV over a field-derived transform (plain bind) -- issue #222" in { @@ -127,22 +136,21 @@ class JavaClientExtendedStatsEmissionSpec extends AnyWordSpec with Matchers { ) } - "never carry a null-unsafe transform script (lead directive 2026-09-06)" in { - // The scripts reaching Elasticsearch for the first time through this module are the metric - // scripts the other aggregates already use: a `def paramN = (doc[..].size() == 0 ? null : ..)` - // preamble and a null-guarded result. Nothing dereferences a `? null :` group. + "render the marker's script VERBATIM -- the script the bridge's null-safety guard vets" in { + // The null-safety rule (lead directive 2026-09-06) has ONE owner: AggregationNamingSpec's + // marker-tree guard in the bridge template (and its es6 twin). What this module adds is the + // rendering, so what it must prove is that the rendered `script.source` is byte-for-byte the + // script carried by the ScriptedExtendedStatsAggregation marker that guard walks. family.foreach { fn => Seq(plain(fn, "YEAR(createdAt)"), windowed(fn, "ABS(salary)")).foreach { sql => withClue(s"[$sql] ") { - val root = mapper.readTree(emitted(sql)) - val scripts = root.findValues("script").asScala.flatMap { s => + val request: ElasticSearchRequest = requestToElasticSearchRequest(single(sql)) + val vetted = markerScriptsOf(request.search.aggs) + vetted should have size 1 + val rendered = mapper.readTree(emitted(sql)).findValues("script").asScala.flatMap { s => Option(s.get("source")).map(_.asText()) } - scripts should not be empty - scripts.foreach { script => - script should not include ")." - script should not include "doc['createdAt'].value.toInstant().atZone(ZoneId.of('Z')).get(ChronoField.YEAR)." - } + rendered shouldBe vetted } } } diff --git a/es9/java/src/test/scala/app/softnetwork/elastic/client/JavaClientExtendedStatsEmissionSpec.scala b/es9/java/src/test/scala/app/softnetwork/elastic/client/JavaClientExtendedStatsEmissionSpec.scala index c3ef68097..5da3bf018 100644 --- a/es9/java/src/test/scala/app/softnetwork/elastic/client/JavaClientExtendedStatsEmissionSpec.scala +++ b/es9/java/src/test/scala/app/softnetwork/elastic/client/JavaClientExtendedStatsEmissionSpec.scala @@ -20,6 +20,7 @@ import app.softnetwork.elastic.client.java.{JavaClientApi, JavaClientSearchBodyS import app.softnetwork.elastic.sql.bridge._ import app.softnetwork.elastic.sql.query.{SelectStatement, SingleSearch} import com.fasterxml.jackson.databind.ObjectMapper +import com.sksamuel.elastic4s.requests.searches.aggs.{AbstractAggregation, Aggregation} import com.typesafe.config.{Config, ConfigFactory} import org.scalatest.matchers.should.Matchers import org.scalatest.wordspec.AnyWordSpec @@ -76,6 +77,14 @@ class JavaClientExtendedStatsEmissionSpec extends AnyWordSpec with Matchers { private val partition = """"terms":{"field":"id","size":65536,"min_doc_count":1}""" + /** The script of every ScriptedExtendedStatsAggregation marker in the aggregation tree. */ + private def markerScriptsOf(aggs: Iterable[AbstractAggregation]): Seq[String] = + aggs.toSeq.flatMap { + case m: ScriptedExtendedStatsAggregation => m.inner.script.map(_.script).toSeq + case a: Aggregation => markerScriptsOf(a.subaggs) + case _ => Seq.empty + } + "the ES 8/9 client" should { "emit the script of STDDEV over a field-derived transform (plain bind) -- issue #222" in { @@ -127,22 +136,21 @@ class JavaClientExtendedStatsEmissionSpec extends AnyWordSpec with Matchers { ) } - "never carry a null-unsafe transform script (lead directive 2026-09-06)" in { - // The scripts reaching Elasticsearch for the first time through this module are the metric - // scripts the other aggregates already use: a `def paramN = (doc[..].size() == 0 ? null : ..)` - // preamble and a null-guarded result. Nothing dereferences a `? null :` group. + "render the marker's script VERBATIM -- the script the bridge's null-safety guard vets" in { + // The null-safety rule (lead directive 2026-09-06) has ONE owner: AggregationNamingSpec's + // marker-tree guard in the bridge template (and its es6 twin). What this module adds is the + // rendering, so what it must prove is that the rendered `script.source` is byte-for-byte the + // script carried by the ScriptedExtendedStatsAggregation marker that guard walks. family.foreach { fn => Seq(plain(fn, "YEAR(createdAt)"), windowed(fn, "ABS(salary)")).foreach { sql => withClue(s"[$sql] ") { - val root = mapper.readTree(emitted(sql)) - val scripts = root.findValues("script").asScala.flatMap { s => + val request: ElasticSearchRequest = requestToElasticSearchRequest(single(sql)) + val vetted = markerScriptsOf(request.search.aggs) + vetted should have size 1 + val rendered = mapper.readTree(emitted(sql)).findValues("script").asScala.flatMap { s => Option(s.get("source")).map(_.asText()) } - scripts should not be empty - scripts.foreach { script => - script should not include ")." - script should not include "doc['createdAt'].value.toInstant().atZone(ZoneId.of('Z')).get(ChronoField.YEAR)." - } + rendered shouldBe vetted } } } From 0b33270dddc8e049bf5dbee92ecd6a3c9b1995d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Manciot?= Date: Sun, 6 Sep 2026 11:30:47 +0200 Subject: [PATCH 06/11] style: scalafmtAll over the #222 sources Story BIDC-3 Co-Authored-By: Claude Fable 5.1 --- .../sql/bridge/ElasticAggregation.scala | 10 ++++++---- .../ScriptedExtendedStatsAggregation.scala | 6 +++--- .../sql/bridge/SearchBodySerializer.scala | 13 ++++++------ .../sql/ExtendedStatsEmissionSpec.scala | 20 +++++++++++-------- .../elastic/client/SearchApi.scala | 8 ++++---- .../client/GatewayRefusalBoundarySpec.scala | 14 ++++++------- .../sql/bridge/ElasticAggregation.scala | 10 ++++++---- .../ScriptedExtendedStatsAggregation.scala | 12 +++++------ .../sql/bridge/SearchBodySerializer.scala | 7 ++++--- .../sql/ExtendedStatsEmissionSpec.scala | 20 +++++++++++-------- .../jest/JestSearchBodySerializer.scala | 14 ++++++------- ...JestClientExtendedStatsRejectionSpec.scala | 9 +++------ ...tHighLevelClientSearchBodySerializer.scala | 14 ++++++------- ...evelClientExtendedStatsRejectionSpec.scala | 4 ++-- ...tHighLevelClientSearchBodySerializer.scala | 4 ++-- .../java/JavaClientSearchBodySerializer.scala | 8 ++++---- .../java/JavaClientSearchBodySerializer.scala | 8 ++++---- 17 files changed, 96 insertions(+), 85 deletions(-) diff --git a/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/ElasticAggregation.scala b/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/ElasticAggregation.scala index 7955f0c3c..e0f1e5dd6 100644 --- a/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/ElasticAggregation.scala +++ b/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/ElasticAggregation.scala @@ -206,7 +206,8 @@ object ElasticAggregation { // Issue #222 -- a transform-bearing extended_stats is bound to the bridge's own marker: // elastic4s's ExtendedStatsAggregationBuilder drops `script`, so the library type // would serialise as the statistic of the raw field. See ScriptedExtendedStatsAggregation. - (name, s) => ScriptedExtendedStatsAggregation(extendedStatsAgg(name, sourceField).script(s)) + (name, s) => + ScriptedExtendedStatsAggregation(extendedStatsAgg(name, sourceField).script(s)) ) case th: WindowFunction => th.window match { @@ -233,9 +234,10 @@ object ElasticAggregation { aggWithFieldOrScript( extendedStatsAgg, // Issue #222 -- a transform-bearing extended_stats is bound to the bridge's own marker: - // elastic4s's ExtendedStatsAggregationBuilder drops `script`, so the library type - // would serialise as the statistic of the raw field. See ScriptedExtendedStatsAggregation. - (name, s) => ScriptedExtendedStatsAggregation(extendedStatsAgg(name, sourceField).script(s)) + // elastic4s's ExtendedStatsAggregationBuilder drops `script`, so the library type + // would serialise as the statistic of the raw field. See ScriptedExtendedStatsAggregation. + (name, s) => + ScriptedExtendedStatsAggregation(extendedStatsAgg(name, sourceField).script(s)) ) case PERCENTILE_CONT | PERCENTILE_DISC => // Both map to ES `percentiles` (TDigest). One call → one percent; diff --git a/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/ScriptedExtendedStatsAggregation.scala b/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/ScriptedExtendedStatsAggregation.scala index 8477d9db7..eeaef493c 100644 --- a/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/ScriptedExtendedStatsAggregation.scala +++ b/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/ScriptedExtendedStatsAggregation.scala @@ -33,9 +33,9 @@ import com.sksamuel.elastic4s.requests.searches.aggs.{ * marker instead of to the library type it wraps. A marker is a type elastic4s does not know, so: * * - `AggregationBuilderFn`'s typed arms never claim it, and the `customAggregation` handler of - * the 8.x/9.x two-argument `SearchBodyBuilderFn.apply` IS consulted for it (the typed - * `case agg: ExtendedStatsAggregation` arm runs BEFORE that handler, which is why the handler - * cannot key on the library type). The ES 8 / ES 9 client modules render it with its script. + * the 8.x/9.x two-argument `SearchBodyBuilderFn.apply` IS consulted for it (the typed `case + * agg: ExtendedStatsAggregation` arm runs BEFORE that handler, which is why the handler cannot + * key on the library type). The ES 8 / ES 9 client modules render it with its script. * - the one-argument builders (elastic4s 6.x / 7.x, and the 8.x/9.x default handler) throw a * `NotImplementedError` on it -- the request can never leave as silently-wrong JSON. * [[SearchBodySerializer.Default]] refuses it earlier, with a named message; the ES 6 / ES 7 diff --git a/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/SearchBodySerializer.scala b/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/SearchBodySerializer.scala index f541e4814..5082ecc13 100644 --- a/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/SearchBodySerializer.scala +++ b/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/SearchBodySerializer.scala @@ -22,11 +22,11 @@ import com.sksamuel.elastic4s.requests.searches.{SearchBodyBuilderFn, SearchRequ * * The bridge is a shared template: `copyBridge` copies these sources byte-for-byte into the ES 7, * ES 8 and ES 9 modules, which compile them against elastic4s 7.17.x (one-argument - * `SearchBodyBuilderFn`) and 8.x / 9.x (two-argument, with a `customAggregation` handler). Anything - * a single major can do therefore lives in that major's CLIENT module and is injected here: the - * client module puts an implicit `SearchBodySerializer` in scope of the `SingleSearch` conversions - * (`requestToElasticSearchRequest`, `sqlQueryToAggregations`), and every serialisation door - * consumes it. Without one, [[SearchBodySerializer.Default]] applies. + * `SearchBodyBuilderFn`) and 8.x / 9.x (two-argument, with a `customAggregation` handler). + * Anything a single major can do therefore lives in that major's CLIENT module and is injected + * here: the client module puts an implicit `SearchBodySerializer` in scope of the `SingleSearch` + * conversions (`requestToElasticSearchRequest`, `sqlQueryToAggregations`), and every serialisation + * door consumes it. Without one, [[SearchBodySerializer.Default]] applies. */ trait SearchBodySerializer { @@ -37,7 +37,8 @@ trait SearchBodySerializer { object SearchBodySerializer { /** True when the request carries an `extended_stats` over a transformed expression -- any - * [[ScriptedExtendedStatsAggregation]] anywhere in its aggregation tree (plain or windowed bind). + * [[ScriptedExtendedStatsAggregation]] anywhere in its aggregation tree (plain or windowed + * bind). */ def hasTransformExtendedStats(search: SearchRequest): Boolean = ScriptedExtendedStatsAggregation.existsIn(search.aggs) diff --git a/bridge/src/test/scala/app/softnetwork/elastic/sql/ExtendedStatsEmissionSpec.scala b/bridge/src/test/scala/app/softnetwork/elastic/sql/ExtendedStatsEmissionSpec.scala index ba14bfacf..1f128f7ee 100644 --- a/bridge/src/test/scala/app/softnetwork/elastic/sql/ExtendedStatsEmissionSpec.scala +++ b/bridge/src/test/scala/app/softnetwork/elastic/sql/ExtendedStatsEmissionSpec.scala @@ -12,12 +12,12 @@ import java.time.ZonedDateTime /** Issue #222 (story BIDC-3): `STDDEV` / `VARIANCE` (the `extended_stats` family) over a * TRANSFORMED expression -- the shared bridge template's half. * - * The template cannot render the shape (that needs the ES 8 / ES 9 client modules' serializer), - * so what it owns is asserted here: the shape DISCRIMINATOR, both binds (plain and windowed) and - * both doors; the marker carrying field + script; the Default serializer's LOUD, named refusal - * instead of the silent raw-field statistic the library used to emit; and the raw-field - * `extended_stats` emission, pinned byte-for-byte (AC 4 -- these fixtures did not exist before and - * they must not move). + * The template cannot render the shape (that needs the ES 8 / ES 9 client modules' serializer), so + * what it owns is asserted here: the shape DISCRIMINATOR, both binds (plain and windowed) and both + * doors; the marker carrying field + script; the Default serializer's LOUD, named refusal instead + * of the silent raw-field statistic the library used to emit; and the raw-field `extended_stats` + * emission, pinned byte-for-byte (AC 4 -- these fixtures did not exist before and they must not + * move). * * Captured on the base commit (T2, both bridge copies, both doors), before the fix: * `STDDEV(YEAR(createdAt))` -> `"extended_stats":{"field":"createdAt"}` (no script: the standard @@ -49,7 +49,9 @@ class ExtendedStatsEmissionSpec extends AnyFlatSpec with Matchers { s"SELECT id, name, $fn($operand) OVER (PARTITION BY id) AS s FROM t" /** Every marker in the aggregation tree of `search`. */ - private def markersOf(aggs: Iterable[AbstractAggregation]): Seq[ScriptedExtendedStatsAggregation] = + private def markersOf( + aggs: Iterable[AbstractAggregation] + ): Seq[ScriptedExtendedStatsAggregation] = aggs.toSeq.flatMap { case m: ScriptedExtendedStatsAggregation => Seq(m) case a: Aggregation => markersOf(a.subaggs) @@ -96,7 +98,9 @@ class ExtendedStatsEmissionSpec extends AnyFlatSpec with Matchers { } it should "be false for a transform inside ANOTHER aggregate (MAX emits its own script)" in { - requestOf("SELECT id, MAX(YEAR(createdAt)) AS m FROM t GROUP BY id").hasTransformExtendedStats shouldBe false + requestOf( + "SELECT id, MAX(YEAR(createdAt)) AS m FROM t GROUP BY id" + ).hasTransformExtendedStats shouldBe false } it should "flag the aggregation itself on the sqlQueryToAggregations door" in { 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 651935004..c2afd1ddb 100644 --- a/core/src/main/scala/app/softnetwork/elastic/client/SearchApi.scala +++ b/core/src/main/scala/app/softnetwork/elastic/client/SearchApi.scala @@ -1386,10 +1386,10 @@ trait SearchApi extends ElasticConversion with ElasticClientHelpers { /** The window-aggregation request of `request` and its TRANSLATION to an Elasticsearch body. * * Kept separate from the execution (issue #222): the translation is where a client module may - * REFUSE the statement (a status-bearing `ElasticError`, e.g. STDDEV / VARIANCE over a transformed - * expression on ES 6 / ES 7), and a refusal known at translation time must be answered at - * `GatewayApi.run` -- so a caller that executes asynchronously translates here FIRST, on the - * calling thread, and only then schedules the execution. + * REFUSE the statement (a status-bearing `ElasticError`, e.g. STDDEV / VARIANCE over a + * transformed expression on ES 6 / ES 7), and a refusal known at translation time must be + * answered at `GatewayApi.run` -- so a caller that executes asynchronously translates here + * FIRST, on the calling thread, and only then schedules the execution. */ private[client] def windowAggregationQuery( request: SingleSearch diff --git a/core/src/test/scala/app/softnetwork/elastic/client/GatewayRefusalBoundarySpec.scala b/core/src/test/scala/app/softnetwork/elastic/client/GatewayRefusalBoundarySpec.scala index cc7923d38..8442608c7 100644 --- a/core/src/test/scala/app/softnetwork/elastic/client/GatewayRefusalBoundarySpec.scala +++ b/core/src/test/scala/app/softnetwork/elastic/client/GatewayRefusalBoundarySpec.scala @@ -17,13 +17,13 @@ import scala.concurrent.duration._ * The SQL -> Elasticsearch translation runs synchronously inside `searchAsync` / `scroll`, before * any Future exists. A client module refuses a statement there by throwing a status-bearing * `ElasticError` (the ES 6 / ES 7 modules do so for STDDEV / VARIANCE over a transformed - * expression). `GatewayApi.run` must surface that refusal as the `ElasticFailure` every other error - * is -- on EVERY route a DQL statement can take: the `SearchExecutor` (aggregation-shaped, or an - * explicit LIMIT) AND the `CoreDqlExtension`'s quota-capped scroll (an un-LIMITed row query), - * which calls `client.scroll` directly and never enters the executor. That second route is why - * the boundary lives in `run`, not in `SearchExecutor`: a first cut there let the refusal escape - * as a raw exception on exactly the path BI tools take for a plain projection. The windowed row - * query's scroll used to translate LAZILY (inside the stream's Future); its translation is now + * expression). `GatewayApi.run` must surface that refusal as the `ElasticFailure` every other + * error is -- on EVERY route a DQL statement can take: the `SearchExecutor` (aggregation-shaped, + * or an explicit LIMIT) AND the `CoreDqlExtension`'s quota-capped scroll (an un-LIMITed row + * query), which calls `client.scroll` directly and never enters the executor. That second route is + * why the boundary lives in `run`, not in `SearchExecutor`: a first cut there let the refusal + * escape as a raw exception on exactly the path BI tools take for a plain projection. The windowed + * row query's scroll used to translate LAZILY (inside the stream's Future); its translation is now * hoisted onto the calling thread so the contract is uniform: a refusal known at translation time * is answered at `run`, on every route. * diff --git a/es6/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/ElasticAggregation.scala b/es6/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/ElasticAggregation.scala index aa2f61b30..f4aed1c53 100644 --- a/es6/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/ElasticAggregation.scala +++ b/es6/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/ElasticAggregation.scala @@ -207,7 +207,8 @@ object ElasticAggregation { // Issue #222 -- a transform-bearing extended_stats is bound to the bridge's own marker: // elastic4s's ExtendedStatsAggregationBuilder drops `script`, so the library type // would serialise as the statistic of the raw field. See ScriptedExtendedStatsAggregation. - (name, s) => ScriptedExtendedStatsAggregation(extendedStatsAgg(name, sourceField).script(s)) + (name, s) => + ScriptedExtendedStatsAggregation(extendedStatsAgg(name, sourceField).script(s)) ) case th: WindowFunction => th.window match { @@ -234,9 +235,10 @@ object ElasticAggregation { aggWithFieldOrScript( extendedStatsAgg, // Issue #222 -- a transform-bearing extended_stats is bound to the bridge's own marker: - // elastic4s's ExtendedStatsAggregationBuilder drops `script`, so the library type - // would serialise as the statistic of the raw field. See ScriptedExtendedStatsAggregation. - (name, s) => ScriptedExtendedStatsAggregation(extendedStatsAgg(name, sourceField).script(s)) + // elastic4s's ExtendedStatsAggregationBuilder drops `script`, so the library type + // would serialise as the statistic of the raw field. See ScriptedExtendedStatsAggregation. + (name, s) => + ScriptedExtendedStatsAggregation(extendedStatsAgg(name, sourceField).script(s)) ) case PERCENTILE_CONT | PERCENTILE_DISC => // Both map to ES `percentiles` (TDigest). One call → one percent; diff --git a/es6/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/ScriptedExtendedStatsAggregation.scala b/es6/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/ScriptedExtendedStatsAggregation.scala index d6a89da54..13ea6d76f 100644 --- a/es6/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/ScriptedExtendedStatsAggregation.scala +++ b/es6/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/ScriptedExtendedStatsAggregation.scala @@ -23,18 +23,18 @@ import com.sksamuel.elastic4s.searches.aggs.{ } /** An `extended_stats` aggregation over a TRANSFORMED expression -- `STDDEV(YEAR(createdAt))`, - * `VARIANCE(ABS(salary))` and the rest of the family (issue #222). Hand-maintained ES 6 twin of the - * bridge template's marker (elastic4s 6.7.8 package names; same contract). + * `VARIANCE(ABS(salary))` and the rest of the family (issue #222). Hand-maintained ES 6 twin of + * the bridge template's marker (elastic4s 6.7.8 package names; same contract). * * elastic4s's own `ExtendedStatsAggregationBuilder` never emits `agg.script`, so an * `ExtendedStatsAggregation` carrying a script silently serialises as the statistic of the RAW * field -- or as `extended_stats: {}` when there is no raw field to fall back on. The bridge * therefore binds a transform-bearing extended_stats to this marker instead of to the library type * it wraps. The 6.x line has no script-emitting builder and no customisation seam, so the marker - * is never rendered here: [[SearchBodySerializer.Default]] refuses it with a named message, and the - * ES 6 client modules (REST and Jest) refuse it with an `ElasticError` naming their major. Left to - * elastic4s, it would still fail loudly (`AggregationBuilderFn`'s `NotImplementedError`) -- it can - * never leave as silently-wrong JSON. + * is never rendered here: [[SearchBodySerializer.Default]] refuses it with a named message, and + * the ES 6 client modules (REST and Jest) refuse it with an `ElasticError` naming their major. + * Left to elastic4s, it would still fail loudly (`AggregationBuilderFn`'s `NotImplementedError`) + * -- it can never leave as silently-wrong JSON. */ final case class ScriptedExtendedStatsAggregation(inner: ExtendedStatsAggregation) extends Aggregation { diff --git a/es6/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/SearchBodySerializer.scala b/es6/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/SearchBodySerializer.scala index 1d999e086..87c183069 100644 --- a/es6/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/SearchBodySerializer.scala +++ b/es6/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/SearchBodySerializer.scala @@ -23,8 +23,8 @@ import com.sksamuel.elastic4s.searches.SearchRequest * twin of the bridge template's seam (elastic4s 6.7.8 package names; same contract). * * The ES 6 client modules (REST and Jest) put an implicit `SearchBodySerializer` in scope of the - * `SingleSearch` conversions (`requestToElasticSearchRequest`, `sqlQueryToAggregations`), and every - * serialisation door consumes it. Without one, [[SearchBodySerializer.Default]] applies. + * `SingleSearch` conversions (`requestToElasticSearchRequest`, `sqlQueryToAggregations`), and + * every serialisation door consumes it. Without one, [[SearchBodySerializer.Default]] applies. */ trait SearchBodySerializer { @@ -35,7 +35,8 @@ trait SearchBodySerializer { object SearchBodySerializer { /** True when the request carries an `extended_stats` over a transformed expression -- any - * [[ScriptedExtendedStatsAggregation]] anywhere in its aggregation tree (plain or windowed bind). + * [[ScriptedExtendedStatsAggregation]] anywhere in its aggregation tree (plain or windowed + * bind). */ def hasTransformExtendedStats(search: SearchRequest): Boolean = ScriptedExtendedStatsAggregation.existsIn(search.aggs) diff --git a/es6/bridge/src/test/scala/app/softnetwork/elastic/sql/ExtendedStatsEmissionSpec.scala b/es6/bridge/src/test/scala/app/softnetwork/elastic/sql/ExtendedStatsEmissionSpec.scala index 4b0b3bf3a..094edf0db 100644 --- a/es6/bridge/src/test/scala/app/softnetwork/elastic/sql/ExtendedStatsEmissionSpec.scala +++ b/es6/bridge/src/test/scala/app/softnetwork/elastic/sql/ExtendedStatsEmissionSpec.scala @@ -12,12 +12,12 @@ import java.time.ZonedDateTime /** Issue #222 (story BIDC-3): `STDDEV` / `VARIANCE` (the `extended_stats` family) over a * TRANSFORMED expression -- the shared bridge template's half. * - * The template cannot render the shape (that needs the ES 8 / ES 9 client modules' serializer), - * so what it owns is asserted here: the shape DISCRIMINATOR, both binds (plain and windowed) and - * both doors; the marker carrying field + script; the Default serializer's LOUD, named refusal - * instead of the silent raw-field statistic the library used to emit; and the raw-field - * `extended_stats` emission, pinned byte-for-byte (AC 4 -- these fixtures did not exist before and - * they must not move). + * The template cannot render the shape (that needs the ES 8 / ES 9 client modules' serializer), so + * what it owns is asserted here: the shape DISCRIMINATOR, both binds (plain and windowed) and both + * doors; the marker carrying field + script; the Default serializer's LOUD, named refusal instead + * of the silent raw-field statistic the library used to emit; and the raw-field `extended_stats` + * emission, pinned byte-for-byte (AC 4 -- these fixtures did not exist before and they must not + * move). * * Captured on the base commit (T2, both bridge copies, both doors), before the fix: * `STDDEV(YEAR(createdAt))` -> `"extended_stats":{"field":"createdAt"}` (no script: the standard @@ -49,7 +49,9 @@ class ExtendedStatsEmissionSpec extends AnyFlatSpec with Matchers { s"SELECT id, name, $fn($operand) OVER (PARTITION BY id) AS s FROM t" /** Every marker in the aggregation tree of `search`. */ - private def markersOf(aggs: Iterable[AbstractAggregation]): Seq[ScriptedExtendedStatsAggregation] = + private def markersOf( + aggs: Iterable[AbstractAggregation] + ): Seq[ScriptedExtendedStatsAggregation] = aggs.toSeq.flatMap { case m: ScriptedExtendedStatsAggregation => Seq(m) case a: Aggregation => markersOf(a.subaggs) @@ -96,7 +98,9 @@ class ExtendedStatsEmissionSpec extends AnyFlatSpec with Matchers { } it should "be false for a transform inside ANOTHER aggregate (MAX emits its own script)" in { - requestOf("SELECT id, MAX(YEAR(createdAt)) AS m FROM t GROUP BY id").hasTransformExtendedStats shouldBe false + requestOf( + "SELECT id, MAX(YEAR(createdAt)) AS m FROM t GROUP BY id" + ).hasTransformExtendedStats shouldBe false } it should "flag the aggregation itself on the sqlQueryToAggregations door" in { diff --git a/es6/jest/src/main/scala/app/softnetwork/elastic/client/jest/JestSearchBodySerializer.scala b/es6/jest/src/main/scala/app/softnetwork/elastic/client/jest/JestSearchBodySerializer.scala index a54b83cc8..aaae20f83 100644 --- a/es6/jest/src/main/scala/app/softnetwork/elastic/client/jest/JestSearchBodySerializer.scala +++ b/es6/jest/src/main/scala/app/softnetwork/elastic/client/jest/JestSearchBodySerializer.scala @@ -20,16 +20,16 @@ import app.softnetwork.elastic.client.result.ElasticError import app.softnetwork.elastic.sql.bridge.SearchBodySerializer import com.sksamuel.elastic4s.searches.SearchRequest -/** The ES 6 (Jest) search-body serializer (issue #222): the default one-argument elastic4s builder, with a - * LOUD refusal of `STDDEV` / `VARIANCE` (any of the `extended_stats` family) over a transformed - * expression -- plain or windowed -- BEFORE any JSON exists. +/** The ES 6 (Jest) search-body serializer (issue #222): the default one-argument elastic4s builder, + * with a LOUD refusal of `STDDEV` / `VARIANCE` (any of the `extended_stats` family) over a + * transformed expression -- plain or windowed -- BEFORE any JSON exists. * * elastic4s 6.7.8 has neither a script-emitting `ExtendedStatsAggregationBuilder` nor the * `customAggregation` seam the ES 8 / ES 9 modules use to render one, so this module cannot - * compute the statistic over the transform; it used to compute it silently over the RAW field. - * The refusal is an `ElasticError` with status 400 so it reaches the caller as an honest failure. - * The 6.x elastic4s line is unmaintained: there is no upstream path (spec R-1b), so unlike the - * ES 7 module this refusal is permanent. + * compute the statistic over the transform; it used to compute it silently over the RAW field. The + * refusal is an `ElasticError` with status 400 so it reaches the caller as an honest failure. The + * 6.x elastic4s line is unmaintained: there is no upstream path (spec R-1b), so unlike the ES 7 + * module this refusal is permanent. */ object JestSearchBodySerializer extends SearchBodySerializer { diff --git a/es6/jest/src/test/scala/app/softnetwork/elastic/client/JestClientExtendedStatsRejectionSpec.scala b/es6/jest/src/test/scala/app/softnetwork/elastic/client/JestClientExtendedStatsRejectionSpec.scala index a8ce28b22..b7b9c614e 100644 --- a/es6/jest/src/test/scala/app/softnetwork/elastic/client/JestClientExtendedStatsRejectionSpec.scala +++ b/es6/jest/src/test/scala/app/softnetwork/elastic/client/JestClientExtendedStatsRejectionSpec.scala @@ -16,10 +16,7 @@ package app.softnetwork.elastic.client -import app.softnetwork.elastic.client.jest.{ - JestClientApi, - JestSearchBodySerializer -} +import app.softnetwork.elastic.client.jest.{JestClientApi, JestSearchBodySerializer} import app.softnetwork.elastic.client.result.ElasticError import app.softnetwork.elastic.sql.bridge._ import app.softnetwork.elastic.sql.query.{SelectStatement, SingleSearch} @@ -30,8 +27,8 @@ import org.scalatest.wordspec.AnyWordSpec /** Issue #222 (story BIDC-3) -- the ES 6 Jest-client half: `STDDEV` / `VARIANCE` (the whole * `extended_stats` family) over a TRANSFORMED expression is REFUSED before any JSON exists, with a * named `ElasticError` (status 400), for BOTH binds and on BOTH serialisation doors -- never - * executed against the raw field, which is what elastic4s 6.x's script-dropping builder used to - * do silently. A raw-field extended_stats is untouched. + * executed against the raw field, which is what elastic4s 6.x's script-dropping builder used to do + * silently. A raw-field extended_stats is untouched. * * No Docker: `ElasticClientCompanion` builds the underlying client lazily and `apply()` is never * called, so nothing here touches the network. diff --git a/es6/rest/src/main/scala/app/softnetwork/elastic/client/rest/RestHighLevelClientSearchBodySerializer.scala b/es6/rest/src/main/scala/app/softnetwork/elastic/client/rest/RestHighLevelClientSearchBodySerializer.scala index c82b0a334..285ae7f14 100644 --- a/es6/rest/src/main/scala/app/softnetwork/elastic/client/rest/RestHighLevelClientSearchBodySerializer.scala +++ b/es6/rest/src/main/scala/app/softnetwork/elastic/client/rest/RestHighLevelClientSearchBodySerializer.scala @@ -20,16 +20,16 @@ import app.softnetwork.elastic.client.result.ElasticError import app.softnetwork.elastic.sql.bridge.SearchBodySerializer import com.sksamuel.elastic4s.searches.SearchRequest -/** The ES 6 (REST) search-body serializer (issue #222): the default one-argument elastic4s builder, with a - * LOUD refusal of `STDDEV` / `VARIANCE` (any of the `extended_stats` family) over a transformed - * expression -- plain or windowed -- BEFORE any JSON exists. +/** The ES 6 (REST) search-body serializer (issue #222): the default one-argument elastic4s builder, + * with a LOUD refusal of `STDDEV` / `VARIANCE` (any of the `extended_stats` family) over a + * transformed expression -- plain or windowed -- BEFORE any JSON exists. * * elastic4s 6.7.8 has neither a script-emitting `ExtendedStatsAggregationBuilder` nor the * `customAggregation` seam the ES 8 / ES 9 modules use to render one, so this module cannot - * compute the statistic over the transform; it used to compute it silently over the RAW field. - * The refusal is an `ElasticError` with status 400 so it reaches the caller as an honest failure. - * The 6.x elastic4s line is unmaintained: there is no upstream path (spec R-1b), so unlike the - * ES 7 module this refusal is permanent. + * compute the statistic over the transform; it used to compute it silently over the RAW field. The + * refusal is an `ElasticError` with status 400 so it reaches the caller as an honest failure. The + * 6.x elastic4s line is unmaintained: there is no upstream path (spec R-1b), so unlike the ES 7 + * module this refusal is permanent. */ object RestHighLevelClientSearchBodySerializer extends SearchBodySerializer { diff --git a/es6/rest/src/test/scala/app/softnetwork/elastic/client/RestHighLevelClientExtendedStatsRejectionSpec.scala b/es6/rest/src/test/scala/app/softnetwork/elastic/client/RestHighLevelClientExtendedStatsRejectionSpec.scala index fdc4e5953..e545bc5fb 100644 --- a/es6/rest/src/test/scala/app/softnetwork/elastic/client/RestHighLevelClientExtendedStatsRejectionSpec.scala +++ b/es6/rest/src/test/scala/app/softnetwork/elastic/client/RestHighLevelClientExtendedStatsRejectionSpec.scala @@ -30,8 +30,8 @@ import org.scalatest.wordspec.AnyWordSpec /** Issue #222 (story BIDC-3) -- the ES 6 REST-client half: `STDDEV` / `VARIANCE` (the whole * `extended_stats` family) over a TRANSFORMED expression is REFUSED before any JSON exists, with a * named `ElasticError` (status 400), for BOTH binds and on BOTH serialisation doors -- never - * executed against the raw field, which is what elastic4s 6.x's script-dropping builder used to - * do silently. A raw-field extended_stats is untouched. + * executed against the raw field, which is what elastic4s 6.x's script-dropping builder used to do + * silently. A raw-field extended_stats is untouched. * * No Docker: `ElasticClientCompanion` builds the underlying client lazily and `apply()` is never * called, so nothing here touches the network. diff --git a/es7/rest/src/main/scala/app/softnetwork/elastic/client/rest/RestHighLevelClientSearchBodySerializer.scala b/es7/rest/src/main/scala/app/softnetwork/elastic/client/rest/RestHighLevelClientSearchBodySerializer.scala index 6b1b34c86..b642141ab 100644 --- a/es7/rest/src/main/scala/app/softnetwork/elastic/client/rest/RestHighLevelClientSearchBodySerializer.scala +++ b/es7/rest/src/main/scala/app/softnetwork/elastic/client/rest/RestHighLevelClientSearchBodySerializer.scala @@ -26,8 +26,8 @@ import com.sksamuel.elastic4s.requests.searches.SearchRequest * * elastic4s 7.17.x has neither a script-emitting `ExtendedStatsAggregationBuilder` nor the * `customAggregation` seam the ES 8 / ES 9 modules use to render one, so this module cannot - * compute the statistic over the transform; it used to compute it silently over the RAW field. - * The refusal is an `ElasticError` with status 400 so it reaches the caller as an honest failure. + * compute the statistic over the transform; it used to compute it silently over the RAW field. The + * refusal is an `ElasticError` with status 400 so it reaches the caller as an honest failure. * Watch item (spec AD-S3-3): when the elastic4s#4100 backport lands on the 7.17 line and * `Versions.elastic74s` moves past it, replace the refusal with a rendering handler. */ diff --git a/es8/java/src/main/scala/app/softnetwork/elastic/client/java/JavaClientSearchBodySerializer.scala b/es8/java/src/main/scala/app/softnetwork/elastic/client/java/JavaClientSearchBodySerializer.scala index 2af7b5f5c..ea10e62d1 100644 --- a/es8/java/src/main/scala/app/softnetwork/elastic/client/java/JavaClientSearchBodySerializer.scala +++ b/es8/java/src/main/scala/app/softnetwork/elastic/client/java/JavaClientSearchBodySerializer.scala @@ -40,10 +40,10 @@ import com.sksamuel.elastic4s.requests.searches.{SearchBodyBuilderFn, SearchRequ * this serializer is byte-identical to the one-argument default for any request without a marker. * * The handler renders what `ExtendedStatsAggregationBuilder` renders -- `field`, `sigma`, - * `missing` -- PLUS the `script` (through the same `ScriptBuilderFn` the Stats / Max / Avg builders - * use), then the sub-aggregations and metadata. Watch item (spec AD-S3-3): when elastic4s#4100 - * ships and `Versions.elastic84s` / `elastic94s` move past it, the marker can be bound back to the - * library type and this handler deleted. + * `missing` -- PLUS the `script` (through the same `ScriptBuilderFn` the Stats / Max / Avg + * builders use), then the sub-aggregations and metadata. Watch item (spec AD-S3-3): when + * elastic4s#4100 ships and `Versions.elastic84s` / `elastic94s` move past it, the marker can be + * bound back to the library type and this handler deleted. * * Kept byte-identical between the ES 8 and ES 9 modules (their elastic4s builders are identical). */ diff --git a/es9/java/src/main/scala/app/softnetwork/elastic/client/java/JavaClientSearchBodySerializer.scala b/es9/java/src/main/scala/app/softnetwork/elastic/client/java/JavaClientSearchBodySerializer.scala index 2af7b5f5c..ea10e62d1 100644 --- a/es9/java/src/main/scala/app/softnetwork/elastic/client/java/JavaClientSearchBodySerializer.scala +++ b/es9/java/src/main/scala/app/softnetwork/elastic/client/java/JavaClientSearchBodySerializer.scala @@ -40,10 +40,10 @@ import com.sksamuel.elastic4s.requests.searches.{SearchBodyBuilderFn, SearchRequ * this serializer is byte-identical to the one-argument default for any request without a marker. * * The handler renders what `ExtendedStatsAggregationBuilder` renders -- `field`, `sigma`, - * `missing` -- PLUS the `script` (through the same `ScriptBuilderFn` the Stats / Max / Avg builders - * use), then the sub-aggregations and metadata. Watch item (spec AD-S3-3): when elastic4s#4100 - * ships and `Versions.elastic84s` / `elastic94s` move past it, the marker can be bound back to the - * library type and this handler deleted. + * `missing` -- PLUS the `script` (through the same `ScriptBuilderFn` the Stats / Max / Avg + * builders use), then the sub-aggregations and metadata. Watch item (spec AD-S3-3): when + * elastic4s#4100 ships and `Versions.elastic84s` / `elastic94s` move past it, the marker can be + * bound back to the library type and this handler deleted. * * Kept byte-identical between the ES 8 and ES 9 modules (their elastic4s builders are identical). */ From 47ea34b70604a4d5ef03d7477d3783fe28956f83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Manciot?= Date: Sun, 6 Sep 2026 12:10:41 +0200 Subject: [PATCH 07/11] =?UTF-8?q?fix(core,es8,es9,bridge):=20review=20foll?= =?UTF-8?q?ow-up=20=E2=80=94=20deferred=20refusals=20recovered,=20elastic4?= =?UTF-8?q?s=20fallback=20handler=20kept,=20prose=20+=20headers=20(R3-1..R?= =?UTF-8?q?3-9)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Independent review of 398dd5e2: APPROVE-WITH-FIXES (0 HIGH, 3 MEDIUM, 6 LOW). All applied. R3-2 (rebase): rebased onto origin/feature/BIDC-2 10f3f8fc (two further BIDC-2 commits, both touching BOTH AggregationNamingSpec copies). The merged tree now compiles and runs: bridge and es6 bridge 176/176 on 2.12.20 and 2.13.16. A textually clean merge-tree is not a build. R3-8 (route contract): only SYNCHRONOUS throws were converted, so an extension deferring the translation into its own Future still escaped while the PR body, spec and findings log all claimed "answered at run on EVERY route". Implemented rather than scoped: `dispatch(statement).recover { case e: ElasticError => ... }` beside the existing catch, sharing one `refused` helper, pinned by a deferring-extension case in GatewayRefusalBoundarySpec (6/6). A boundary that only catches is half a boundary when the callers include third-party code — the SPI is exactly that. R3-1: the one-case PartialFunction REPLACED elastic4s's `defaultCustomAggregationHandler`, turning a NotImplementedError naming the class into a bare MatchError for anything the library cannot build; now `scriptedExtendedStats orElse defaultCustomAggregationHandler`, with a dummy-Aggregation test in both client modules (es8 + es9 9/9). That test earned itself immediately: the first form declared the composed val ABOVE the val it reads, so the object initialiser saw null and every emission died with ExceptionInInitializerError. Ordering fixed. R3-3: the upstream backports are OPEN and were unnamed — elastic4s#4105 (series/7.x, the one that LIFTS the es7 refusal) and elastic4s#4106 (series/8.x, the one that retires the es8/es9 handler), both cherry-picks of #4100. Retargeted in the watch note and the PR body, no closing keyword. R3-4 / R3-5 / R3-6: prose corrected — the discriminator's scaladoc named a reader that does not read it; the fifth door's justification was wrong (ElasticMultiSearchRequest has ZERO repo-wide references; the verdict and in-code comment were right); TestSerializers.RawDefault claimed a NotImplementedError it never reaches (it returns an asserted sentinel). R3-7: the windowed-scroll fallback arm's new `sql` is kept (it matches the success arm) and is now a release-note item, since it changes that arm's core SQL log line. R3-9: licence headers on the three new bridge/core test files. AD-S3-1' is LEAD-RATIFIED (2026-09-06); AD-S3-1's named mechanism is recorded in the spec as REFUTED BY MEASUREMENT — AggregationBuilderFn matches its typed ExtendedStatsAggregation arm before consulting any custom handler, so a handler keyed on that type is unreachable dead code. Story BIDC-3 Co-Authored-By: Claude Opus 5 (1M context) --- .../sql/bridge/ElasticAggregation.scala | 7 ++- .../sql/ExtendedStatsEmissionSpec.scala | 24 +++++++- .../elastic/client/GatewayApi.scala | 43 ++++++++------ .../client/GatewayRefusalBoundarySpec.scala | 58 ++++++++++++++++++- .../sql/bridge/ElasticAggregation.scala | 7 ++- .../sql/ExtendedStatsEmissionSpec.scala | 24 +++++++- .../java/JavaClientSearchBodySerializer.scala | 19 +++++- .../JavaClientExtendedStatsEmissionSpec.scala | 17 ++++++ .../java/JavaClientSearchBodySerializer.scala | 19 +++++- .../JavaClientExtendedStatsEmissionSpec.scala | 17 ++++++ 10 files changed, 203 insertions(+), 32 deletions(-) diff --git a/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/ElasticAggregation.scala b/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/ElasticAggregation.scala index e0f1e5dd6..f71e92e83 100644 --- a/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/ElasticAggregation.scala +++ b/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/ElasticAggregation.scala @@ -98,7 +98,12 @@ case class ElasticAggregation( /** True when this aggregation is `STDDEV` / `VARIANCE` (any of the `extended_stats` family) over * a TRANSFORMED expression -- the shape a client module must either render with its script or - * refuse (issue #222). The second serialisation door (`sqlQueryToAggregations`) reads it. + * refuse (issue #222). + * + * Nothing in the emission path consults this: on both doors the decision is taken by the + * injected [[SearchBodySerializer]], off the `SearchRequest` it is handed. This is the + * per-aggregation view of the same predicate, for a caller holding an [[ElasticAggregation]] -- + * the client-module tests assert it on the `sqlQueryToAggregations` door. */ def hasTransformExtendedStats: Boolean = ScriptedExtendedStatsAggregation.existsIn(Seq(agg)) } diff --git a/bridge/src/test/scala/app/softnetwork/elastic/sql/ExtendedStatsEmissionSpec.scala b/bridge/src/test/scala/app/softnetwork/elastic/sql/ExtendedStatsEmissionSpec.scala index 1f128f7ee..46c69d427 100644 --- a/bridge/src/test/scala/app/softnetwork/elastic/sql/ExtendedStatsEmissionSpec.scala +++ b/bridge/src/test/scala/app/softnetwork/elastic/sql/ExtendedStatsEmissionSpec.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._ @@ -233,9 +249,11 @@ class ExtendedStatsEmissionSpec extends AnyFlatSpec with Matchers { /** Test-side serializers for the shared template tree. */ object TestSerializers { - /** The library's one-argument builder WITHOUT the Default's refusal -- reaches elastic4s's own - * behaviour on a marker (a `NotImplementedError`), so a test can build the aggregations of a - * transform-bearing statement on the sqlQueryToAggregations door and inspect them. + /** The library's one-argument builder for everything it CAN render, and a sentinel instead of the + * Default's refusal for a marker -- so a test can build the aggregations of a transform-bearing + * statement on the `sqlQueryToAggregations` door and inspect them. It never asks elastic4s to + * render a marker (that would raise `NotImplementedError`); the sentinel is asserted, so the + * substitution cannot pass unnoticed. */ object RawDefault extends SearchBodySerializer { override def serialize(search: SearchRequest): String = diff --git a/core/src/main/scala/app/softnetwork/elastic/client/GatewayApi.scala b/core/src/main/scala/app/softnetwork/elastic/client/GatewayApi.scala index 96440dff1..f7b9e65b0 100644 --- a/core/src/main/scala/app/softnetwork/elastic/client/GatewayApi.scala +++ b/core/src/main/scala/app/softnetwork/elastic/client/GatewayApi.scala @@ -1931,25 +1931,30 @@ trait GatewayApi extends IndicesApi with ElasticClientHelpers { )(implicit system: ActorSystem): Future[ElasticResult[QueryResult]] = { implicit val ec: ExecutionContext = system.dispatcher - // The SQL -> Elasticsearch translation runs SYNCHRONOUSLY inside `searchAsync` / `scroll`, - // before any Future exists, on every route below -- the extension route included - // (`CoreDqlExtension`'s quota-capped scroll calls `client.scroll` directly and never enters an - // executor). A client module may REFUSE a statement there by throwing a status-bearing - // `ElasticError` (issue #222: STDDEV / VARIANCE over a transformed expression on ES 6 / ES 7, - // where the library cannot emit the aggregation script). This ONE boundary, at the front door - // every route converges on, turns that deliberate refusal into the `ElasticFailure` every other - // error is, so the REPL, JDBC and Arrow see an honest 400 instead of a raw exception. Anything - // else escaping translation keeps its current (thrown) route -- a `NonFatal` totality boundary - // for the whole translation layer is #250's shape and a separate change. - try dispatch(statement) - catch { - case refusal: ElasticError => - logger.error(s"❌ ${refusal.message}") - val operation = statement match { - case _: DqlStatement => Some("dql") // the relabel every DQL executor failure carries - case _ => refusal.operation - } - Future.successful(ElasticFailure(refusal.copy(operation = operation))) + // A client module may REFUSE a statement while translating it to Elasticsearch, by throwing a + // status-bearing `ElasticError` (issue #222: STDDEV / VARIANCE over a transformed expression on + // ES 6 / ES 7, where the library cannot emit the aggregation script). This ONE boundary, at the + // front door every route converges on -- executor OR extension (`CoreDqlExtension`'s + // quota-capped scroll calls `client.scroll` directly and never enters an executor) -- turns + // that deliberate refusal into the `ElasticFailure` every other error is, so the REPL, JDBC and + // Arrow see an honest 400 instead of a raw exception. + // + // BOTH halves are needed for the contract "a refusal is answered by `run`, on every route" to + // hold: `catch` covers a translation that runs SYNCHRONOUSLY (every in-tree route -- the + // executors, and `CoreDqlExtension`), `recover` covers one an extension defers into its own + // Future. Anything OTHER than an `ElasticError` keeps its current route -- a `NonFatal` + // totality boundary for the whole translation layer is #250's shape and a separate change. + def refused(refusal: ElasticError): ElasticResult[QueryResult] = { + logger.error(s"❌ ${refusal.message}") + val operation = statement match { + case _: DqlStatement => Some("dql") // the relabel every DQL executor failure carries + case _ => refusal.operation + } + ElasticFailure(refusal.copy(operation = operation)) + } + + try dispatch(statement).recover { case refusal: ElasticError => refused(refusal) } catch { + case refusal: ElasticError => Future.successful(refused(refusal)) } } diff --git a/core/src/test/scala/app/softnetwork/elastic/client/GatewayRefusalBoundarySpec.scala b/core/src/test/scala/app/softnetwork/elastic/client/GatewayRefusalBoundarySpec.scala index 8442608c7..97c910558 100644 --- a/core/src/test/scala/app/softnetwork/elastic/client/GatewayRefusalBoundarySpec.scala +++ b/core/src/test/scala/app/softnetwork/elastic/client/GatewayRefusalBoundarySpec.scala @@ -1,9 +1,26 @@ +/* + * 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 app.softnetwork.elastic.client.result._ import app.softnetwork.elastic.sql.PainlessContextType -import app.softnetwork.elastic.sql.query.SingleSearch +import app.softnetwork.elastic.sql.query.{SingleSearch, Statement} +import com.typesafe.config.ConfigFactory import org.scalatest.BeforeAndAfterAll import org.scalatest.concurrent.ScalaFutures import org.scalatest.flatspec.AnyFlatSpec @@ -11,6 +28,7 @@ import org.scalatest.matchers.should.Matchers import org.slf4j.{Logger, LoggerFactory} import scala.concurrent.duration._ +import scala.concurrent.Future /** Issue #222 (story BIDC-3) -- the ONE core boundary the per-major refusal relies on. * @@ -100,6 +118,44 @@ class GatewayRefusalBoundarySpec ) } + it should "surface a refusal an EXTENSION defers into its own Future (R3-8)" in { + // Every in-tree route translates synchronously, so `catch` alone covered them. An extension is + // third-party code: it may translate inside its own Future, and then the refusal arrives as a + // FAILED future, not a throw. Without `recover` on `dispatch` the caller would get the raw + // exception -- and the documented contract ("answered at `run`, on every route") would be false + // for exactly the callers the SPI exists to serve. + val deferring = new ExtensionSpi { + override def extensionId: String = "deferring-refusal-test" + override def extensionName: String = "Deferring refusal (test double)" + override def version: String = "test" + override def priority: Int = 1 // ahead of CoreDqlExtension (100) + override def initialize( + config: com.typesafe.config.Config, + licenseRefreshStrategy: app.softnetwork.elastic.licensing.LicenseRefreshStrategy + ): Either[String, Unit] = Right(()) + override def canHandle(statement: Statement): Boolean = true + override def execute(statement: Statement, client: ElasticClientApi)(implicit + system: ActorSystem + ): Future[ElasticResult[QueryResult]] = + Future( + throw ElasticError(refusalMessage, statusCode = Some(400), operation = Some("search")) + )( + system.dispatcher + ) + override def supportedSyntax: Seq[String] = Seq.empty + } + + val client = new NopeClientApi { + override protected def logger: Logger = LoggerFactory.getLogger(getClass) + override lazy val extensionRegistry: ExtensionRegistry = + new ExtensionRegistry(ConfigFactory.load(), licenseRefreshStrategy) { + override lazy val extensions: Seq[ExtensionSpi] = Seq(deferring) + } + } + + assertRefused(client.run("SELECT id, name FROM t LIMIT 5").futureValue) + } + it should "not manufacture a refusal for a client that does not refuse" in { // NopeClientApi answers a search with no response body, which core reports as an ordinary // execution failure -- proving the boundary only relabels a refusal the client raised. diff --git a/es6/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/ElasticAggregation.scala b/es6/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/ElasticAggregation.scala index f4aed1c53..99812237f 100644 --- a/es6/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/ElasticAggregation.scala +++ b/es6/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/ElasticAggregation.scala @@ -98,7 +98,12 @@ case class ElasticAggregation( /** True when this aggregation is `STDDEV` / `VARIANCE` (any of the `extended_stats` family) over * a TRANSFORMED expression -- the shape a client module must either render with its script or - * refuse (issue #222). The second serialisation door (`sqlQueryToAggregations`) reads it. + * refuse (issue #222). + * + * Nothing in the emission path consults this: on both doors the decision is taken by the + * injected [[SearchBodySerializer]], off the `SearchRequest` it is handed. This is the + * per-aggregation view of the same predicate, for a caller holding an [[ElasticAggregation]] -- + * the client-module tests assert it on the `sqlQueryToAggregations` door. */ def hasTransformExtendedStats: Boolean = ScriptedExtendedStatsAggregation.existsIn(Seq(agg)) } diff --git a/es6/bridge/src/test/scala/app/softnetwork/elastic/sql/ExtendedStatsEmissionSpec.scala b/es6/bridge/src/test/scala/app/softnetwork/elastic/sql/ExtendedStatsEmissionSpec.scala index 094edf0db..a7f68ee9c 100644 --- a/es6/bridge/src/test/scala/app/softnetwork/elastic/sql/ExtendedStatsEmissionSpec.scala +++ b/es6/bridge/src/test/scala/app/softnetwork/elastic/sql/ExtendedStatsEmissionSpec.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._ @@ -233,9 +249,11 @@ class ExtendedStatsEmissionSpec extends AnyFlatSpec with Matchers { /** Test-side serializers for the shared template tree. */ object TestSerializers { - /** The library's one-argument builder WITHOUT the Default's refusal -- reaches elastic4s's own - * behaviour on a marker (a `NotImplementedError`), so a test can build the aggregations of a - * transform-bearing statement on the sqlQueryToAggregations door and inspect them. + /** The library's one-argument builder for everything it CAN render, and a sentinel instead of the + * Default's refusal for a marker -- so a test can build the aggregations of a transform-bearing + * statement on the `sqlQueryToAggregations` door and inspect them. It never asks elastic4s to + * render a marker (that would raise `NotImplementedError`); the sentinel is asserted, so the + * substitution cannot pass unnoticed. */ object RawDefault extends SearchBodySerializer { override def serialize(search: SearchRequest): String = diff --git a/es8/java/src/main/scala/app/softnetwork/elastic/client/java/JavaClientSearchBodySerializer.scala b/es8/java/src/main/scala/app/softnetwork/elastic/client/java/JavaClientSearchBodySerializer.scala index ea10e62d1..6ff2d46f6 100644 --- a/es8/java/src/main/scala/app/softnetwork/elastic/client/java/JavaClientSearchBodySerializer.scala +++ b/es8/java/src/main/scala/app/softnetwork/elastic/client/java/JavaClientSearchBodySerializer.scala @@ -24,7 +24,11 @@ import com.sksamuel.elastic4s.requests.searches.aggs.{ AggMetaDataFn, SubAggsBuilderFn } -import com.sksamuel.elastic4s.requests.searches.{SearchBodyBuilderFn, SearchRequest} +import com.sksamuel.elastic4s.requests.searches.{ + defaultCustomAggregationHandler, + SearchBodyBuilderFn, + SearchRequest +} /** The ES 8 / ES 9 search-body serializer (issue #222): elastic4s's two-argument * `SearchBodyBuilderFn.apply(request, customAggregation)` with a handler for the bridge's @@ -49,10 +53,21 @@ import com.sksamuel.elastic4s.requests.searches.{SearchBodyBuilderFn, SearchRequ */ object JavaClientSearchBodySerializer extends SearchBodySerializer { - private val handler: PartialFunction[AbstractAggregation, XContentBuilder] = { + private val scriptedExtendedStats: PartialFunction[AbstractAggregation, XContentBuilder] = { case agg: ScriptedExtendedStatsAggregation => extendedStatsWithScript(agg) } + /** Our one case, then elastic4s's own fallback. The `orElse` is not decoration: this partial + * function REPLACES `defaultCustomAggregationHandler`, whose only job is to fail an unknown + * aggregation with a `NotImplementedError` naming the class. Ours alone would raise a bare + * `MatchError` instead -- a strictly worse diagnostic for anything the library cannot build. + * + * Declared AFTER `scriptedExtendedStats`: a `val` reading a `val` defined below it sees `null`, + * and the object's initialiser then dies with `ExceptionInInitializerError` on first use. + */ + private val handler: PartialFunction[AbstractAggregation, XContentBuilder] = + scriptedExtendedStats orElse defaultCustomAggregationHandler + private def extendedStatsWithScript(agg: ScriptedExtendedStatsAggregation): XContentBuilder = { val inner = agg.inner val builder = XContentFactory.jsonBuilder() diff --git a/es8/java/src/test/scala/app/softnetwork/elastic/client/JavaClientExtendedStatsEmissionSpec.scala b/es8/java/src/test/scala/app/softnetwork/elastic/client/JavaClientExtendedStatsEmissionSpec.scala index 5da3bf018..358264899 100644 --- a/es8/java/src/test/scala/app/softnetwork/elastic/client/JavaClientExtendedStatsEmissionSpec.scala +++ b/es8/java/src/test/scala/app/softnetwork/elastic/client/JavaClientExtendedStatsEmissionSpec.scala @@ -182,5 +182,22 @@ class JavaClientExtendedStatsEmissionSpec extends AnyWordSpec with Matchers { "be the serializer the client injects" in { client.searchBodySerializer shouldBe JavaClientSearchBodySerializer } + + "keep elastic4s's own diagnostic for an aggregation NEITHER it nor we can build (R3-1)" in { + // The custom handler REPLACES `defaultCustomAggregationHandler`, whose only job is to fail an + // unknown aggregation with a NotImplementedError naming the class. A one-case handler would + // raise a bare MatchError instead -- strictly worse for anyone debugging a missing builder. + val unknown = new Aggregation { + type T = Aggregation + override def name: String = "unknown" + override def metadata: Map[String, AnyRef] = Map.empty + override def subaggs: Seq[AbstractAggregation] = Seq.empty + override def subAggregations(aggs: Iterable[AbstractAggregation]): T = this + override def metadata(map: Map[String, AnyRef]): T = this + } + val request = com.sksamuel.elastic4s.ElasticApi.search("t").aggregations(unknown) + val thrown = intercept[NotImplementedError](JavaClientSearchBodySerializer.serialize(request)) + thrown.getMessage should include(unknown.getClass.getName) + } } } diff --git a/es9/java/src/main/scala/app/softnetwork/elastic/client/java/JavaClientSearchBodySerializer.scala b/es9/java/src/main/scala/app/softnetwork/elastic/client/java/JavaClientSearchBodySerializer.scala index ea10e62d1..6ff2d46f6 100644 --- a/es9/java/src/main/scala/app/softnetwork/elastic/client/java/JavaClientSearchBodySerializer.scala +++ b/es9/java/src/main/scala/app/softnetwork/elastic/client/java/JavaClientSearchBodySerializer.scala @@ -24,7 +24,11 @@ import com.sksamuel.elastic4s.requests.searches.aggs.{ AggMetaDataFn, SubAggsBuilderFn } -import com.sksamuel.elastic4s.requests.searches.{SearchBodyBuilderFn, SearchRequest} +import com.sksamuel.elastic4s.requests.searches.{ + defaultCustomAggregationHandler, + SearchBodyBuilderFn, + SearchRequest +} /** The ES 8 / ES 9 search-body serializer (issue #222): elastic4s's two-argument * `SearchBodyBuilderFn.apply(request, customAggregation)` with a handler for the bridge's @@ -49,10 +53,21 @@ import com.sksamuel.elastic4s.requests.searches.{SearchBodyBuilderFn, SearchRequ */ object JavaClientSearchBodySerializer extends SearchBodySerializer { - private val handler: PartialFunction[AbstractAggregation, XContentBuilder] = { + private val scriptedExtendedStats: PartialFunction[AbstractAggregation, XContentBuilder] = { case agg: ScriptedExtendedStatsAggregation => extendedStatsWithScript(agg) } + /** Our one case, then elastic4s's own fallback. The `orElse` is not decoration: this partial + * function REPLACES `defaultCustomAggregationHandler`, whose only job is to fail an unknown + * aggregation with a `NotImplementedError` naming the class. Ours alone would raise a bare + * `MatchError` instead -- a strictly worse diagnostic for anything the library cannot build. + * + * Declared AFTER `scriptedExtendedStats`: a `val` reading a `val` defined below it sees `null`, + * and the object's initialiser then dies with `ExceptionInInitializerError` on first use. + */ + private val handler: PartialFunction[AbstractAggregation, XContentBuilder] = + scriptedExtendedStats orElse defaultCustomAggregationHandler + private def extendedStatsWithScript(agg: ScriptedExtendedStatsAggregation): XContentBuilder = { val inner = agg.inner val builder = XContentFactory.jsonBuilder() diff --git a/es9/java/src/test/scala/app/softnetwork/elastic/client/JavaClientExtendedStatsEmissionSpec.scala b/es9/java/src/test/scala/app/softnetwork/elastic/client/JavaClientExtendedStatsEmissionSpec.scala index 5da3bf018..358264899 100644 --- a/es9/java/src/test/scala/app/softnetwork/elastic/client/JavaClientExtendedStatsEmissionSpec.scala +++ b/es9/java/src/test/scala/app/softnetwork/elastic/client/JavaClientExtendedStatsEmissionSpec.scala @@ -182,5 +182,22 @@ class JavaClientExtendedStatsEmissionSpec extends AnyWordSpec with Matchers { "be the serializer the client injects" in { client.searchBodySerializer shouldBe JavaClientSearchBodySerializer } + + "keep elastic4s's own diagnostic for an aggregation NEITHER it nor we can build (R3-1)" in { + // The custom handler REPLACES `defaultCustomAggregationHandler`, whose only job is to fail an + // unknown aggregation with a NotImplementedError naming the class. A one-case handler would + // raise a bare MatchError instead -- strictly worse for anyone debugging a missing builder. + val unknown = new Aggregation { + type T = Aggregation + override def name: String = "unknown" + override def metadata: Map[String, AnyRef] = Map.empty + override def subaggs: Seq[AbstractAggregation] = Seq.empty + override def subAggregations(aggs: Iterable[AbstractAggregation]): T = this + override def metadata(map: Map[String, AnyRef]): T = this + } + val request = com.sksamuel.elastic4s.ElasticApi.search("t").aggregations(unknown) + val thrown = intercept[NotImplementedError](JavaClientSearchBodySerializer.serialize(request)) + thrown.getMessage should include(unknown.getClass.getName) + } } } From 671c68b4fa8d443800302636f896460d523e7c6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Manciot?= Date: Sun, 6 Sep 2026 19:25:51 +0200 Subject: [PATCH 08/11] =?UTF-8?q?feat(es7):=20elastic4s=207.17.26=20?= =?UTF-8?q?=E2=80=94=20ES=207=20now=20COMPUTES=20STDDEV/VARIANCE=20over=20?= =?UTF-8?q?a=20transform=20(#222,=20AD-S3-4)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Scope extension, lead-decided 2026-09-06: elastic4s 7.17.26 is released and carries the series/7.x backport (elastic4s#4105) of the extended_stats script fix, so ES 7 must compute, not refuse. FORK MIGRATION, not a version bump: 7.17.26 exists only under `nl.gn0s1s` (the `com.sksamuel.elastic4s` 7.x line stops at 7.17.4). Both es7 arms of `elastic4sDependencies` and `elastic4sTestkitDependencies` move, plus `Versions.elastic74s`. Package names are unchanged, so the entire es7 tree recompiled with ZERO source changes (`+ es7/compile`, `+ softclient4es7-sql-bridge`, `+ es7rest`, `+ softclient4es7-core-testkit`, both Scala legs). MIGRATION FINDING — a Jackson split that the resolution alone did not reveal. 7.17.26 is a recent release of an old line, so it declares jackson-databind/core/annotations 2.22.2, while this build curates a coherent Jackson per ES major. Resolved, that left databind 2.22.2 beside the curated jackson-module-scala 2.19.0, which validates its databind at class-init: every emitted query died with ExceptionInInitializerError ("Scala module 2.19.0 requires Jackson Databind >= 2.19.0 and < 2.20.0 - Found 2.22.2") on the first XContentBuilder.string. Fixed the way this build already handles it for `org.elasticsearch:elasticsearch`: `excludeAll(jacksonExclusions)` on both es7 elastic4s artifacts, so the curated set governs. Verified by re-resolving the classpath (one coherent 2.19.0 set) AND by running the suites, which exercise that exact path. AD-S3-4 — a THIRD behaviour beside render (ES 8/9) and refuse (ES 6): UNWRAP. 7.17.26 has no `customAggregation` seam (both `SearchBodyBuilderFn.apply` and `AggregationBuilderFn.apply` are one-argument) and does not need one, because its default builder is now correct. So the ES 7 serializer substitutes the bridge's `ScriptedExtendedStatsAggregation` marker for the `ExtendedStatsAggregation` it wraps, at every depth, and lets the stock builder render the script. The unwrap lives in the es7 client module, NOT on the shared trait: ES 8/9 must not unwrap (their pinned elastic4s still drops the script and they need the marker to reach their handler) and ES 6 must not either (it refuses). One major needs it today (Rule of Three). `RestHighLevelClientExtendedStatsRejectionSpec` is REPLACED by `RestHighLevelClientExtendedStatsEmissionSpec` (10/10): the JSON carries the script on both doors and both binds, for all six family members; the unwrap is asserted to preserve the marker's script verbatim and to leave a marker-free request byte-identical to the stock builder. The testkit's #222 cases now branch on `computesTransformedStats` (ES >= 7), so the ES 7 leg asserts the same value oracle as ES 8/9; ES 6 alone keeps the named-400 refusal. Story BIDC-3 Co-Authored-By: Claude Opus 5 (1M context) --- .../client/rest/RestHighLevelClientApi.scala | 6 +- ...tHighLevelClientSearchBodySerializer.scala | 67 ++++-- ...LevelClientExtendedStatsEmissionSpec.scala | 213 ++++++++++++++++++ ...evelClientExtendedStatsRejectionSpec.scala | 122 ---------- project/SoftClient4es.scala | 21 +- project/Versions.scala | 6 +- .../elastic/client/WindowFunctionSpec.scala | 32 ++- 7 files changed, 306 insertions(+), 161 deletions(-) create mode 100644 es7/rest/src/test/scala/app/softnetwork/elastic/client/RestHighLevelClientExtendedStatsEmissionSpec.scala delete mode 100644 es7/rest/src/test/scala/app/softnetwork/elastic/client/RestHighLevelClientExtendedStatsRejectionSpec.scala diff --git a/es7/rest/src/main/scala/app/softnetwork/elastic/client/rest/RestHighLevelClientApi.scala b/es7/rest/src/main/scala/app/softnetwork/elastic/client/rest/RestHighLevelClientApi.scala index 4a97abe61..ef65125fd 100644 --- a/es7/rest/src/main/scala/app/softnetwork/elastic/client/rest/RestHighLevelClientApi.scala +++ b/es7/rest/src/main/scala/app/softnetwork/elastic/client/rest/RestHighLevelClientApi.scala @@ -1093,9 +1093,9 @@ trait RestHighLevelClientSearchApi extends SearchApi with RestHighLevelClientHel /** Issue #222 -- the search-body serializer every `SingleSearch` conversion made from this trait * picks up (implicit scope of the bridge's `requestToElasticSearchRequest` / - * `sqlQueryToAggregations`): elastic4s 7.17.x cannot render an `extended_stats` over a - * transformed expression with its script, so the request is REFUSED with a named `ElasticError` - * before any JSON exists -- never executed against the raw field. See + * `sqlQueryToAggregations`): since elastic4s 7.17.26 the stock builder emits an + * `extended_stats` script itself, so this module UNWRAPS the bridge's marker and lets it -- + * the statistic is computed over the transform, as on ES 8 / ES 9. See * [[RestHighLevelClientSearchBodySerializer]]. */ implicit def searchBodySerializer: SearchBodySerializer = RestHighLevelClientSearchBodySerializer diff --git a/es7/rest/src/main/scala/app/softnetwork/elastic/client/rest/RestHighLevelClientSearchBodySerializer.scala b/es7/rest/src/main/scala/app/softnetwork/elastic/client/rest/RestHighLevelClientSearchBodySerializer.scala index b642141ab..cb360fcfd 100644 --- a/es7/rest/src/main/scala/app/softnetwork/elastic/client/rest/RestHighLevelClientSearchBodySerializer.scala +++ b/es7/rest/src/main/scala/app/softnetwork/elastic/client/rest/RestHighLevelClientSearchBodySerializer.scala @@ -16,36 +16,57 @@ package app.softnetwork.elastic.client.rest -import app.softnetwork.elastic.client.result.ElasticError -import app.softnetwork.elastic.sql.bridge.SearchBodySerializer +import app.softnetwork.elastic.sql.bridge.{ScriptedExtendedStatsAggregation, SearchBodySerializer} import com.sksamuel.elastic4s.requests.searches.SearchRequest +import com.sksamuel.elastic4s.requests.searches.aggs.{AbstractAggregation, Aggregation} -/** The ES 7 search-body serializer (issue #222): the default one-argument elastic4s builder, with a - * LOUD refusal of `STDDEV` / `VARIANCE` (any of the `extended_stats` family) over a transformed - * expression -- plain or windowed -- BEFORE any JSON exists. +/** The ES 7 search-body serializer (issue #222): **UNWRAP**, then serialise with elastic4s's own + * one-argument builder. * - * elastic4s 7.17.x has neither a script-emitting `ExtendedStatsAggregationBuilder` nor the - * `customAggregation` seam the ES 8 / ES 9 modules use to render one, so this module cannot - * compute the statistic over the transform; it used to compute it silently over the RAW field. The - * refusal is an `ElasticError` with status 400 so it reaches the caller as an honest failure. - * Watch item (spec AD-S3-3): when the elastic4s#4100 backport lands on the 7.17 line and - * `Versions.elastic74s` moves past it, replace the refusal with a rendering handler. + * Since elastic4s **7.17.26** (`nl.gn0s1s`, the series/7.x backport elastic4s#4105 of #4100) the + * library's `ExtendedStatsAggregationBuilder` emits `agg.script` natively, so this module computes + * the statistic over the transform like ES 8 / ES 9 do — it no longer refuses. What it does NOT + * have is the two-argument `SearchBodyBuilderFn.apply(request, customAggregation)` seam the ES 8 / + * ES 9 modules render through: on the 7.x line both `SearchBodyBuilderFn.apply` and + * `AggregationBuilderFn.apply` are one-argument, and no custom handler can be injected. + * + * That is the whole reason a third behaviour exists beside *render* (ES 8 / ES 9) and *refuse* + * (ES 6). The version-agnostic bridge template binds a transform-bearing `extended_stats` to + * [[ScriptedExtendedStatsAggregation]] — a type elastic4s does not know — so handing it to the + * 7.x builder would raise `NotImplementedError`. Here the marker is simply substituted back for + * the `ExtendedStatsAggregation` it wraps, throughout the aggregation tree, and the resulting + * request is serialised by the stock builder, which now renders the script itself. + * + * The template stays version-agnostic: it knows only that a marker exists, never which majors can + * render it. The unwrap lives HERE rather than on `SearchBodySerializer` because it is not shared + * behaviour — ES 8 / ES 9 must NOT unwrap (their pinned elastic4s, 8.18.2 / 9.0.0, still drops the + * script; they need the marker to reach their handler) and ES 6 must not either (it refuses). + * One major needs it today, so it is written once, where it is true (Rule of Three). + * + * Watch item (spec AD-S3-3 / AD-S3-4): when an 8.x / 9.x release carrying elastic4s#4106 / #4100 + * exists and those pins move, the ES 8 / ES 9 handler retires the same way and the marker can be + * dropped from the template altogether. */ object RestHighLevelClientSearchBodySerializer extends SearchBodySerializer { val ElasticsearchMajor: Int = 7 - /** The named error a transform-bearing extended_stats is refused with on this module. */ - val TransformExtendedStatsUnsupported: String = - SearchBodySerializer.transformExtendedStatsUnsupportedOn(ElasticsearchMajor) + /** The request with every [[ScriptedExtendedStatsAggregation]] replaced by the + * `ExtendedStatsAggregation` it wraps — at the root and at every depth (the windowed bind sits + * under a partition bucket). Any other request is returned untouched, so a body without a marker + * is byte-identical to what the stock builder produced before this story. + */ + private[client] def unwrap(search: SearchRequest): SearchRequest = + if (!SearchBodySerializer.hasTransformExtendedStats(search)) search + else search.aggregations(unwrapAll(search.aggs)) + + private def unwrapAll(aggs: Iterable[AbstractAggregation]): Seq[AbstractAggregation] = + aggs.toSeq.map { + case marker: ScriptedExtendedStatsAggregation => marker.inner + case agg: Aggregation if agg.subaggs.nonEmpty => agg.subAggregations(unwrapAll(agg.subaggs)) + case other => other + } - override def serialize(search: SearchRequest): String = { - if (SearchBodySerializer.hasTransformExtendedStats(search)) - throw ElasticError( - message = TransformExtendedStatsUnsupported, - statusCode = Some(400), - operation = Some("search") - ) - SearchBodySerializer.Default.serialize(search) - } + override def serialize(search: SearchRequest): String = + SearchBodySerializer.Default.serialize(unwrap(search)) } diff --git a/es7/rest/src/test/scala/app/softnetwork/elastic/client/RestHighLevelClientExtendedStatsEmissionSpec.scala b/es7/rest/src/test/scala/app/softnetwork/elastic/client/RestHighLevelClientExtendedStatsEmissionSpec.scala new file mode 100644 index 000000000..2d4c9bbdd --- /dev/null +++ b/es7/rest/src/test/scala/app/softnetwork/elastic/client/RestHighLevelClientExtendedStatsEmissionSpec.scala @@ -0,0 +1,213 @@ +/* + * 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 app.softnetwork.elastic.client.rest.{ + RestHighLevelClientApi, + RestHighLevelClientSearchBodySerializer +} +import app.softnetwork.elastic.sql.bridge._ +import app.softnetwork.elastic.sql.query.{SelectStatement, SingleSearch} +import com.fasterxml.jackson.databind.ObjectMapper +import com.sksamuel.elastic4s.requests.searches.aggs.{AbstractAggregation, Aggregation} +import com.typesafe.config.{Config, ConfigFactory} +import org.scalatest.matchers.should.Matchers +import org.scalatest.wordspec.AnyWordSpec + +import scala.jdk.CollectionConverters._ + +/** Issue #222 (story BIDC-3) -- the ES 7 half, since the elastic4s 7.17.26 migration (AD-S3-4). + * + * `STDDEV` / `VARIANCE` (the whole `extended_stats` family) over a TRANSFORMED expression carries + * its script in the emitted JSON, on BOTH serialisation doors and for BOTH binds (plain, and + * windowed under a partition bucket) -- exactly as on ES 8 / ES 9, but by a different mechanism: + * this module has no `customAggregation` seam, so it UNWRAPS the bridge's marker and lets the + * stock 7.17.26 builder (which now emits `agg.script`, elastic4s#4105) render it. + * + * This file REPLACES `RestHighLevelClientExtendedStatsRejectionSpec`: until 7.17.26 the same + * statements were refused with a named 400, because the library dropped the script and the + * alternative was a silently wrong statistic over the raw field. + * + * No Docker: `ElasticClientCompanion` builds the underlying client lazily and `apply()` is never + * called, so nothing here touches the network. + */ +class RestHighLevelClientExtendedStatsEmissionSpec extends AnyWordSpec with Matchers { + + private val client: RestHighLevelClientApi = new RestHighLevelClientApi { + override def config: Config = ConfigFactory.load() + } + + implicit private val timestamp: Long = 1767139200000L // 2025-12-31T00:00:00Z + + private val mapper = new ObjectMapper() + + private def single(sql: String): SingleSearch = + SelectStatement(sql).statement match { + case Some(s: SingleSearch) => s + case other => fail(s"Not a single search: $other") + } + + /** Door 1 -- what the client sends: `singleSearchToJsonQuery`. */ + private def emitted(sql: String): String = client.singleSearchToJsonQuery(single(sql)) + + private val family = Seq("STDDEV", "STDDEV_SAMP", "STDDEV_POP", "VARIANCE", "VAR_SAMP", "VAR_POP") + + private def plain(fn: String, operand: String) = + s"SELECT id, $fn($operand) AS s FROM t GROUP BY id" + + private def windowed(fn: String, operand: String) = + s"SELECT id, name, $fn($operand) OVER (PARTITION BY id) AS s FROM t" + + private val yearScript = + """"script":{"lang":"painless","source":"def param1 = (doc['createdAt'].size() == 0 ? null : """ + + """doc['createdAt'].value.toInstant().atZone(ZoneId.of('Z')).get(ChronoField.YEAR)); param1"}""" + + private val absScript = + """"script":{"lang":"painless","source":"def param1 = (doc['salary'].size() == 0 ? null : """ + + """doc['salary'].value); (param1 == null) ? null : Double.valueOf(Math.abs(param1))"}""" + + private val partition = """"terms":{"field":"id","size":65536,"min_doc_count":1}""" + + /** The script of every marker in a tree -- what the unwrap must preserve verbatim. */ + private def markerScriptsOf(aggs: Iterable[AbstractAggregation]): Seq[String] = + aggs.toSeq.flatMap { + case m: ScriptedExtendedStatsAggregation => m.inner.script.map(_.script).toSeq + case a: Aggregation => markerScriptsOf(a.subaggs) + case _ => Seq.empty + } + + "the ES 7 client (elastic4s 7.17.26+)" should { + + "emit the script of STDDEV over a field-derived transform (plain bind) -- issue #222" in { + emitted(plain("STDDEV", "YEAR(createdAt)")) shouldBe + s"""{"query":{"match_all":{}},"size":0,"_source":false,"aggs":{"id":{$partition,""" + + s""""aggs":{"s":{"extended_stats":{"field":"createdAt",$yearScript}}}}}}""" + } + + "emit a pure script when the transform has no raw field behind it (ABS(salary))" in { + // Before 7.17.26: `"extended_stats":{}` -- neither field nor script, rejected by Elasticsearch + // (and refused by this module rather than sent). + emitted(plain("VARIANCE", "ABS(salary)")) shouldBe + s"""{"query":{"match_all":{}},"size":0,"_source":false,"aggs":{"id":{$partition,""" + + s""""aggs":{"s":{"extended_stats":{$absScript}}}}}}""" + } + + "emit the script on the WINDOWED bind (under the partition bucket)" in { + emitted(windowed("STDDEV", "YEAR(createdAt)")) shouldBe + s"""{"query":{"match_all":{}},"size":0,"_source":false,"aggs":{"id":{$partition,""" + + s""""aggs":{"s":{"extended_stats":{"field":"createdAt",$yearScript}}}}}}""" + } + + "emit the script for EVERY family member, plain and windowed" in { + family.foreach { fn => + Seq(plain(fn, "YEAR(createdAt)"), windowed(fn, "YEAR(createdAt)")).foreach { sql => + withClue(s"[$sql] ") { + val json = emitted(sql) + json should include(""""extended_stats":{"field":"createdAt","script":{""") + json should include("ChronoField.YEAR") + json should not include """"extended_stats":{"field":"createdAt"}""" + } + } + Seq(plain(fn, "ABS(salary)"), windowed(fn, "ABS(salary)")).foreach { sql => + withClue(s"[$sql] ") { + val json = emitted(sql) + json should include(""""extended_stats":{"script":{""") + json should not include """"extended_stats":{}""" + } + } + } + } + + "emit the script on the sqlQueryToAggregations door too" in { + implicit val serializer: SearchBodySerializer = client.searchBodySerializer + val aggs: Seq[ElasticAggregation] = SelectStatement(plain("STDDEV_POP", "YEAR(createdAt)")) + aggs should have size 1 + aggs.head.hasTransformExtendedStats shouldBe true + aggs.head.query shouldBe Some( + s"""{"query":{"match_all":{}},"size":0,"aggs":{"s":{"extended_stats":{"field":"createdAt",$yearScript}}}}""" + ) + } + + "render the marker's script VERBATIM -- the script the bridge's null-safety guard vets" in { + family.foreach { fn => + Seq(plain(fn, "YEAR(createdAt)"), windowed(fn, "ABS(salary)")).foreach { sql => + withClue(s"[$sql] ") { + val request: ElasticSearchRequest = requestToElasticSearchRequest(single(sql)) + val vetted = markerScriptsOf(request.search.aggs) + vetted should have size 1 + val rendered = mapper.readTree(emitted(sql)).findValues("script").asScala.flatMap { s => + Option(s.get("source")).map(_.asText()) + } + rendered shouldBe vetted + } + } + } + } + } + + "RestHighLevelClientSearchBodySerializer.unwrap" should { + + "substitute the marker for the aggregation it wraps, at every depth" in { + Seq(plain("STDDEV", "YEAR(createdAt)"), windowed("VARIANCE", "ABS(salary)")).foreach { sql => + withClue(s"[$sql] ") { + val request: ElasticSearchRequest = requestToElasticSearchRequest(single(sql)) + request.hasTransformExtendedStats shouldBe true + val unwrapped = RestHighLevelClientSearchBodySerializer.unwrap(request.search) + // No marker survives, and the scripts are carried over unchanged. + SearchBodySerializer.hasTransformExtendedStats(unwrapped) shouldBe false + markerScriptsOf(request.search.aggs) should have size 1 + mapper + .readTree(SearchBodySerializer.Default.serialize(unwrapped)) + .findValues("script") + .asScala + .flatMap(s => Option(s.get("source")).map(_.asText())) shouldBe + markerScriptsOf(request.search.aggs) + } + } + } + + "leave a request without a marker untouched (byte-identical to the stock builder)" in { + Seq( + plain("STDDEV", "salary"), + windowed("VAR_POP", "salary"), + "SELECT id, MAX(YEAR(createdAt)) AS m FROM t GROUP BY id", + "SELECT id, COUNT(x) AS c FROM t GROUP BY id HAVING MAX(YEAR(createdAt)) > 2020", + "SELECT id, MAX(x) - MIN(x) AS d FROM t GROUP BY id", + """SELECT c.id, COUNT(DISTINCT e.address) AS nb FROM customers c JOIN UNNEST(c.emails) AS e + |GROUP BY c.id HAVING COUNT(DISTINCT e.address) > 1""".stripMargin, + "SELECT name, salary FROM t WHERE salary > 10 ORDER BY salary DESC LIMIT 5" + ).foreach { sql => + withClue(s"[$sql] ") { + val request: ElasticSearchRequest = requestToElasticSearchRequest(single(sql)) + request.hasTransformExtendedStats shouldBe false + RestHighLevelClientSearchBodySerializer.unwrap(request.search) shouldBe request.search + RestHighLevelClientSearchBodySerializer.serialize(request.search) shouldBe + SearchBodySerializer.Default.serialize(request.search) + } + } + } + + "keep a raw-field extended_stats exactly as it was" in { + emitted(plain("STDDEV", "salary")) should include(""""extended_stats":{"field":"salary"}""") + } + + "be the serializer the client injects" in { + client.searchBodySerializer shouldBe RestHighLevelClientSearchBodySerializer + RestHighLevelClientSearchBodySerializer.ElasticsearchMajor shouldBe 7 + } + } +} diff --git a/es7/rest/src/test/scala/app/softnetwork/elastic/client/RestHighLevelClientExtendedStatsRejectionSpec.scala b/es7/rest/src/test/scala/app/softnetwork/elastic/client/RestHighLevelClientExtendedStatsRejectionSpec.scala deleted file mode 100644 index 84e08d75c..000000000 --- a/es7/rest/src/test/scala/app/softnetwork/elastic/client/RestHighLevelClientExtendedStatsRejectionSpec.scala +++ /dev/null @@ -1,122 +0,0 @@ -/* - * 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 app.softnetwork.elastic.client.rest.{ - RestHighLevelClientApi, - RestHighLevelClientSearchBodySerializer -} -import app.softnetwork.elastic.client.result.ElasticError -import app.softnetwork.elastic.sql.bridge._ -import app.softnetwork.elastic.sql.query.{SelectStatement, SingleSearch} -import com.typesafe.config.{Config, ConfigFactory} -import org.scalatest.matchers.should.Matchers -import org.scalatest.wordspec.AnyWordSpec - -/** Issue #222 (story BIDC-3) -- the ES 7 REST-client half: `STDDEV` / `VARIANCE` (the whole - * `extended_stats` family) over a TRANSFORMED expression is REFUSED before any JSON exists, with a - * named `ElasticError` (status 400), for BOTH binds and on BOTH serialisation doors -- never - * executed against the raw field, which is what elastic4s 7.17.x's script-dropping builder used to - * do silently. A raw-field extended_stats is untouched. - * - * No Docker: `ElasticClientCompanion` builds the underlying client lazily and `apply()` is never - * called, so nothing here touches the network. - */ -class RestHighLevelClientExtendedStatsRejectionSpec extends AnyWordSpec with Matchers { - - private val client: RestHighLevelClientApi = new RestHighLevelClientApi { - override def config: Config = ConfigFactory.load() - } - - implicit private val timestamp: Long = 1767139200000L // 2025-12-31T00:00:00Z - - private val expectedMajor = 7 - - private def single(sql: String): SingleSearch = - SelectStatement(sql).statement match { - case Some(s: SingleSearch) => s - case other => fail(s"Not a single search: $other") - } - - private val family = Seq("STDDEV", "STDDEV_SAMP", "STDDEV_POP", "VARIANCE", "VAR_SAMP", "VAR_POP") - - private def plain(fn: String, operand: String) = - s"SELECT id, $fn($operand) AS s FROM t GROUP BY id" - - private def windowed(fn: String, operand: String) = - s"SELECT id, name, $fn($operand) OVER (PARTITION BY id) AS s FROM t" - - private def assertRefused(refusal: ElasticError): Unit = { - refusal.message shouldBe RestHighLevelClientSearchBodySerializer.TransformExtendedStatsUnsupported - refusal.message should include(s"Elasticsearch $expectedMajor") - refusal.message should include("elastic4s#4100") - refusal.statusCode shouldBe Some(400) - refusal.operation shouldBe Some("search") - } - - s"the ES $expectedMajor client" should { - - "refuse a transform-bearing extended_stats before emission, for every family member and both binds" in { - family.foreach { fn => - Seq( - plain(fn, "YEAR(createdAt)"), - plain(fn, "ABS(salary)"), - windowed(fn, "YEAR(createdAt)"), - windowed(fn, "ABS(salary)") - ).foreach { sql => - withClue(s"[$sql] ") { - assertRefused(intercept[ElasticError](client.singleSearchToJsonQuery(single(sql)))) - } - } - } - } - - "refuse it on the sqlQueryToAggregations door too" in { - implicit val serializer: SearchBodySerializer = client.searchBodySerializer - val refusal = intercept[ElasticError] { - val aggs: Seq[ElasticAggregation] = SelectStatement(plain("STDDEV", "YEAR(createdAt)")) - aggs - } - assertRefused(refusal) - } - - "leave a raw-field extended_stats exactly as the default serializer emits it" in { - Seq(plain("STDDEV", "salary"), windowed("VAR_POP", "salary")).foreach { sql => - withClue(s"[$sql] ") { - val request: ElasticSearchRequest = requestToElasticSearchRequest(single(sql)) - request.hasTransformExtendedStats shouldBe false - client.singleSearchToJsonQuery(single(sql)) shouldBe - SearchBodySerializer.Default.serialize(request.search).replace("\"version\":true,", "") - client.singleSearchToJsonQuery(single(sql)) should include( - """"extended_stats":{"field":"salary"}""" - ) - } - } - } - - "leave every OTHER scripted metric untouched (MAX(YEAR(x)) keeps its script)" in { - client.singleSearchToJsonQuery( - single("SELECT id, MAX(YEAR(createdAt)) AS m FROM t GROUP BY id") - ) should include(""""max":{"field":"createdAt","script":{"lang":"painless"""") - } - - "inject the refusing serializer" in { - client.searchBodySerializer shouldBe RestHighLevelClientSearchBodySerializer - RestHighLevelClientSearchBodySerializer.ElasticsearchMajor shouldBe expectedMajor - } - } -} diff --git a/project/SoftClient4es.scala b/project/SoftClient4es.scala index b1013afad..044539108 100644 --- a/project/SoftClient4es.scala +++ b/project/SoftClient4es.scala @@ -98,7 +98,22 @@ trait SoftClient4es { ) case 7 => Seq( - "com.sksamuel.elastic4s" %% "elastic4s-core" % Versions.elastic74s exclude ("org.elasticsearch", "elasticsearch") exclude ("org.slf4j", "slf4j-api") + // `nl.gn0s1s`, not `com.sksamuel.elastic4s` (issue #222): the sksamuel 7.x line stops at + // 7.17.4, and the fix for the dropped `extended_stats` script (elastic4s#4105) ships in + // 7.17.26, which exists only under the fork. Same package names, so no source change. + // + // 🔴 The Jackson exclusion is NOT cosmetic and is unique to this arm. 7.17.26 is a RECENT + // release of an OLD line, so it declares a modern Jackson (databind/core/annotations + // 2.22.2) while this build curates its own coherent set per ES major + // (`jacksonDependencies`). Left in, 2.22.2 wins over the curated `jackson-module-scala`, + // which validates its databind at class-init and dies: + // "Scala module 2.19.0 requires Jackson Databind version >= 2.19.0 and < 2.20.0 - Found + // jackson-databind version 2.22.2" — an ExceptionInInitializerError on the FIRST + // `XContentBuilder.string`, i.e. on every emitted query. Excluding Jackson here is what + // `elasticDependencies` already does for `org.elasticsearch:elasticsearch`: the module's + // curated Jackson governs, and elastic4s uses it. + ("nl.gn0s1s" %% "elastic4s-core" % Versions.elastic74s exclude ("org.elasticsearch", "elasticsearch") exclude ("org.slf4j", "slf4j-api")) + .excludeAll(jacksonExclusions *) // (#168 / jdbc#33 / arrow#167) log4j-api arrives transitively — see elasticDependencies. ) case 8 => @@ -124,7 +139,9 @@ trait SoftClient4es { ) case 7 => Seq( - "com.sksamuel.elastic4s" %% "elastic4s-testkit" % Versions.elastic74s exclude ("org.elasticsearch", "elasticsearch") exclude ("org.slf4j", "slf4j-api") + // `nl.gn0s1s` + the Jackson exclusion -- see elastic4sDependencies case 7 (issue #222). + ("nl.gn0s1s" %% "elastic4s-testkit" % Versions.elastic74s exclude ("org.elasticsearch", "elasticsearch") exclude ("org.slf4j", "slf4j-api")) + .excludeAll(jacksonExclusions *) ) case 8 => Seq( diff --git a/project/Versions.scala b/project/Versions.scala index 1b8e4a3b3..6cb9cf8f5 100644 --- a/project/Versions.scala +++ b/project/Versions.scala @@ -30,7 +30,11 @@ object Versions { val es7 = "7.17.29" - val elastic74s = "7.17.4" + // 7.17.26 (issue #222): the FIRST 7.x release whose `ExtendedStatsAggregationBuilder` emits + // `agg.script` -- elastic4s#4105, the series/7.x backport of #4100. Published ONLY under + // `nl.gn0s1s`: the `com.sksamuel.elastic4s` line stops at 7.17.4, so moving past it is a fork + // migration, not a version bump (see SoftClient4es.elastic4sDependencies case 7). + val elastic74s = "7.17.26" val es8 = "8.18.3" diff --git a/testkit/src/main/scala/app/softnetwork/elastic/client/WindowFunctionSpec.scala b/testkit/src/main/scala/app/softnetwork/elastic/client/WindowFunctionSpec.scala index 014c51104..42ab46c8a 100644 --- a/testkit/src/main/scala/app/softnetwork/elastic/client/WindowFunctionSpec.scala +++ b/testkit/src/main/scala/app/softnetwork/elastic/client/WindowFunctionSpec.scala @@ -2116,11 +2116,15 @@ trait WindowFunctionSpec // ======================================================================== // ISSUE #222 (story BIDC-3) — STDDEV / VARIANCE over a TRANSFORMED expression // - // Elasticsearch 8 / 9: the statistic is computed over the transform (the emitted extended_stats - // carries its script); the oracle is computed from the fixture itself and sits ten orders of - // magnitude away from the statistic of the raw `hire_date` millis the query used to compute - // silently. Elasticsearch 6 / 7: the query is REFUSED with a named 400 -- never executed against - // the raw field. An execution-success assertion is explicitly insufficient here (issue #222). + // Elasticsearch 7, 8 and 9: the statistic is computed over the transform (the emitted + // extended_stats carries its script); the oracle is computed from the fixture itself and sits ten + // orders of magnitude away from the statistic of the raw `hire_date` millis the query used to + // compute silently. ES 8 / 9 render the script through elastic4s's `customAggregation` seam; + // ES 7 (elastic4s 7.17.26+, backport elastic4s#4105) unwraps the bridge's marker and lets the + // stock builder emit it — different mechanism, same answer, so the same oracle applies. + // Elasticsearch 6: the query is REFUSED with a named 400 -- never executed against the raw field + // (dead elastic4s line, no upstream path). An execution-success assertion is explicitly + // insufficient here (issue #222). // ======================================================================== def elasticsearchMajor: Int = @@ -2130,6 +2134,14 @@ trait WindowFunctionSpec fail(s"Failed to retrieve Elasticsearch version: ${error.message}") } + /** Whether this client computes `STDDEV` / `VARIANCE` over a TRANSFORMED expression (issue #222). + * + * True from Elasticsearch 7: the ES 7 module moved to elastic4s 7.17.26, whose builder emits the + * aggregation script (elastic4s#4105), and unwraps the bridge marker to reach it. False on ES 6 + * alone — its elastic4s line is dead, so the query is refused rather than answered wrongly. + */ + def computesTransformedStats: Boolean = elasticsearchMajor >= 7 + /** The hire YEARS per department, read from the fixture rows -- the oracle's input. */ private def hireYearsByDepartment: Map[String, Seq[Int]] = client.searchAs[Employee]( @@ -2167,7 +2179,7 @@ trait WindowFunctionSpec |FROM emp |LIMIT 100""".stripMargin - /** ES 6 / 7: the same statement is refused on the gateway route (REPL / JDBC / Arrow -- an honest + /** ES 6: the same statement is refused on the gateway route (REPL / JDBC / Arrow -- an honest * 400 naming the major) AND on the direct client API (the same `ElasticError`, thrown). */ private def assertRefusedOnThisMajor(sql: String): Unit = { @@ -2189,8 +2201,8 @@ trait WindowFunctionSpec direct.message should include(s"Elasticsearch $major") } - "STDDEV / VARIANCE over a transformed expression" should "compute the statistic over the transform on ES 8+ and refuse loudly on ES 6/7 (issue #222)" in { - if (elasticsearchMajor >= 8) { + "STDDEV / VARIANCE over a transformed expression" should "compute the statistic over the transform on ES 7+ and refuse loudly on ES 6 (issue #222)" in { + if (computesTransformedStats) { val years = hireYearsByDepartment client.searchAs[DepartmentYearStats]( """SELECT department, @@ -2233,8 +2245,8 @@ trait WindowFunctionSpec } } - it should "compute the WINDOWED statistic over the transform on ES 8+ and refuse loudly on ES 6/7 (issue #222)" in { - if (elasticsearchMajor >= 8) { + it should "compute the WINDOWED statistic over the transform on ES 7+ and refuse loudly on ES 6 (issue #222)" in { + if (computesTransformedStats) { val years = hireYearsByDepartment client.searchAs[EmployeeYearStats]( """SELECT department, name, hire_date, From 4a6d3bedb515e8bd1b366dc675a31d3adffc5c1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Manciot?= Date: Sun, 6 Sep 2026 19:30:23 +0200 Subject: [PATCH 09/11] docs: only Elasticsearch 6 refuses STDDEV/VARIANCE over a transform (#222) The per-major table in functions_aggregate.md, the DQL paragraph and the known-limitations section all move with the elastic4s 7.17.26 migration: ES 7, 8 and 9 compute the statistic over the transform; ES 6 alone refuses, permanently, because its client library line is unmaintained. Story BIDC-3 Co-Authored-By: Claude Opus 5 (1M context) --- documentation/sql/dql_statements.md | 8 ++++---- documentation/sql/functions_aggregate.md | 7 +++---- documentation/sql/known_limitations.md | 16 ++++++++-------- .../client/rest/RestHighLevelClientApi.scala | 6 +++--- ...RestHighLevelClientSearchBodySerializer.scala | 16 ++++++++-------- .../elastic/client/WindowFunctionSpec.scala | 4 ++-- 6 files changed, 28 insertions(+), 29 deletions(-) diff --git a/documentation/sql/dql_statements.md b/documentation/sql/dql_statements.md index 171a27989..719b767e5 100644 --- a/documentation/sql/dql_statements.md +++ b/documentation/sql/dql_statements.md @@ -476,10 +476,10 @@ All six map to a single Elasticsearch `extended_stats` aggregation per call; the variants require **Elasticsearch 7.7+**; population variants work on Elasticsearch 6+. Over a **transformed** operand (`STDDEV(YEAR(hire_date))`, `VARIANCE(ABS(salary))`, plain or -windowed) the statistic is computed over the transform on **Elasticsearch 8+**; on Elasticsearch 6 -and 7 the query is **refused** with a `400` naming the release, because the client library cannot -emit the aggregation script there and used to return the statistic of the raw field silently. See -[STDDEV / VARIANCE family](functions_aggregate.md#function-stddev--variance-family). +windowed) the statistic is computed over the transform on **Elasticsearch 7 and later**; on +Elasticsearch 6 the query is **refused** with a `400` naming the release, because the client library +cannot emit the aggregation script there and used to return the statistic of the raw field silently. +See [STDDEV / VARIANCE family](functions_aggregate.md#function-stddev--variance-family). ### Percentiles — `PERCENTILE_CONT` / `PERCENTILE_DISC` diff --git a/documentation/sql/functions_aggregate.md b/documentation/sql/functions_aggregate.md index 9785ae674..6bc596c94 100644 --- a/documentation/sql/functions_aggregate.md +++ b/documentation/sql/functions_aggregate.md @@ -1245,13 +1245,12 @@ STDDEV(expr) OVER (PARTITION BY partition_expr, ...) - `NULL` values are ignored. - The un-suffixed `std_deviation` / `variance` keys are the **population** values (present on Elasticsearch 6+); the `_sampling` keys are the **sample** values (introduced in Elasticsearch 7.7). Consequently the sample variants — including the default `STDDEV` / `VARIANCE` — require Elasticsearch 7.7+. On older clusters the column is returned as `null` and a warning is logged. - Each call emits its own `extended_stats` aggregation; two stat calls over the same column emit two aggregations. -- **Transformed operands are per-major** (`STDDEV(YEAR(created_at))`, `VARIANCE(ABS(salary))`, `STDDEV_POP(DATE_TRUNC(ts, MONTH))`, plain or windowed). The Elasticsearch client library the driver builds on drops the aggregation script of an `extended_stats` on every line (elastic4s#4100), so: +- **Transformed operands** (`STDDEV(YEAR(created_at))`, `VARIANCE(ABS(salary))`, `STDDEV_POP(DATE_TRUNC(ts, MONTH))`, plain or windowed) are computed over the transform on **Elasticsearch 7 and later**. The client library the driver builds on used to drop the aggregation script of an `extended_stats` on every line (elastic4s#4100); that is fixed for the 7.x, 8.x and 9.x lines the driver ships, and cannot be for 6.x: | Elasticsearch | `STDDEV(f(x))` / `VARIANCE(f(x))` | |---------------|-----------------------------------| - | 8.x, 9.x | Computed over the transform — the driver emits the script itself. | - | 7.x | **Refused** with a `400` naming the release (`STDDEV/VARIANCE over a transformed expression is not supported on Elasticsearch 7 …`). Lifted when the upstream fix reaches the 7.17 line. | - | 6.x | **Refused** the same way, permanently (unmaintained library line). | + | 7.x, 8.x, 9.x | Computed over the transform. | + | 6.x | **Refused** with a `400` naming the release (`STDDEV/VARIANCE over a transformed expression is not supported on Elasticsearch 6 …`), permanently — the 6.x client library is unmaintained and cannot emit the script. | Before this rule, every release silently returned the statistic of the **raw** field (or an empty `extended_stats` Elasticsearch rejected). A raw-field operand (`STDDEV(salary)`) is unaffected on every release. diff --git a/documentation/sql/known_limitations.md b/documentation/sql/known_limitations.md index bbb2e2378..04d11bfa5 100644 --- a/documentation/sql/known_limitations.md +++ b/documentation/sql/known_limitations.md @@ -107,16 +107,16 @@ Quoted column names and aliases work in both spellings — see is deliberate: when the dot was allowed to float, `ORDER BY b. DESC` silently parsed as a column named `b.DESC` sorted *ascending*. -## `STDDEV` / `VARIANCE` over a transformed expression — Elasticsearch 6 and 7 refuse it +## `STDDEV` / `VARIANCE` over a transformed expression — Elasticsearch 6 refuses it `STDDEV(YEAR(hire_date))`, `VARIANCE(ABS(salary))` and the rest of the `extended_stats` family over -a transformed operand (plain or `OVER (PARTITION BY …)`) compute correctly on **Elasticsearch 8 and -9**. On **Elasticsearch 6 and 7** the query is refused with a `400` — *"STDDEV/VARIANCE over a -transformed expression is not supported on Elasticsearch 7 …"* — because the client library the -driver builds on drops the aggregation script on those lines (elastic4s#4100); until this rule, the -query silently returned the statistic of the **raw** field. Aggregate over a raw field there, or use -Elasticsearch 8+. The 7.x refusal is lifted once the upstream fix reaches the 7.17 line; the 6.x one -is permanent. See [STDDEV / VARIANCE family](functions_aggregate.md#function-stddev--variance-family). +a transformed operand (plain or `OVER (PARTITION BY …)`) compute correctly on **Elasticsearch 7, 8 +and 9**. On **Elasticsearch 6** the query is refused with a `400` — *"STDDEV/VARIANCE over a +transformed expression is not supported on Elasticsearch 6 …"* — because the client library the +driver builds on drops the aggregation script on that line (elastic4s#4100) and the 6.x line is +unmaintained, so the fix cannot reach it; until this rule, the query silently returned the statistic +of the **raw** field. Aggregate over a raw field there, or use Elasticsearch 7+. That refusal is +permanent. See [STDDEV / VARIANCE family](functions_aggregate.md#function-stddev--variance-family). ## Coming in the upcoming release (Quarter 1 2027) diff --git a/es7/rest/src/main/scala/app/softnetwork/elastic/client/rest/RestHighLevelClientApi.scala b/es7/rest/src/main/scala/app/softnetwork/elastic/client/rest/RestHighLevelClientApi.scala index ef65125fd..b26267076 100644 --- a/es7/rest/src/main/scala/app/softnetwork/elastic/client/rest/RestHighLevelClientApi.scala +++ b/es7/rest/src/main/scala/app/softnetwork/elastic/client/rest/RestHighLevelClientApi.scala @@ -1093,9 +1093,9 @@ trait RestHighLevelClientSearchApi extends SearchApi with RestHighLevelClientHel /** Issue #222 -- the search-body serializer every `SingleSearch` conversion made from this trait * picks up (implicit scope of the bridge's `requestToElasticSearchRequest` / - * `sqlQueryToAggregations`): since elastic4s 7.17.26 the stock builder emits an - * `extended_stats` script itself, so this module UNWRAPS the bridge's marker and lets it -- - * the statistic is computed over the transform, as on ES 8 / ES 9. See + * `sqlQueryToAggregations`): since elastic4s 7.17.26 the stock builder emits an `extended_stats` + * script itself, so this module UNWRAPS the bridge's marker and lets it -- the statistic is + * computed over the transform, as on ES 8 / ES 9. See * [[RestHighLevelClientSearchBodySerializer]]. */ implicit def searchBodySerializer: SearchBodySerializer = RestHighLevelClientSearchBodySerializer diff --git a/es7/rest/src/main/scala/app/softnetwork/elastic/client/rest/RestHighLevelClientSearchBodySerializer.scala b/es7/rest/src/main/scala/app/softnetwork/elastic/client/rest/RestHighLevelClientSearchBodySerializer.scala index cb360fcfd..daeedb66c 100644 --- a/es7/rest/src/main/scala/app/softnetwork/elastic/client/rest/RestHighLevelClientSearchBodySerializer.scala +++ b/es7/rest/src/main/scala/app/softnetwork/elastic/client/rest/RestHighLevelClientSearchBodySerializer.scala @@ -30,18 +30,18 @@ import com.sksamuel.elastic4s.requests.searches.aggs.{AbstractAggregation, Aggre * ES 9 modules render through: on the 7.x line both `SearchBodyBuilderFn.apply` and * `AggregationBuilderFn.apply` are one-argument, and no custom handler can be injected. * - * That is the whole reason a third behaviour exists beside *render* (ES 8 / ES 9) and *refuse* - * (ES 6). The version-agnostic bridge template binds a transform-bearing `extended_stats` to - * [[ScriptedExtendedStatsAggregation]] — a type elastic4s does not know — so handing it to the - * 7.x builder would raise `NotImplementedError`. Here the marker is simply substituted back for - * the `ExtendedStatsAggregation` it wraps, throughout the aggregation tree, and the resulting - * request is serialised by the stock builder, which now renders the script itself. + * That is the whole reason a third behaviour exists beside *render* (ES 8 / ES 9) and *refuse* (ES + * 6). The version-agnostic bridge template binds a transform-bearing `extended_stats` to + * [[ScriptedExtendedStatsAggregation]] — a type elastic4s does not know — so handing it to the 7.x + * builder would raise `NotImplementedError`. Here the marker is simply substituted back for the + * `ExtendedStatsAggregation` it wraps, throughout the aggregation tree, and the resulting request + * is serialised by the stock builder, which now renders the script itself. * * The template stays version-agnostic: it knows only that a marker exists, never which majors can * render it. The unwrap lives HERE rather than on `SearchBodySerializer` because it is not shared * behaviour — ES 8 / ES 9 must NOT unwrap (their pinned elastic4s, 8.18.2 / 9.0.0, still drops the - * script; they need the marker to reach their handler) and ES 6 must not either (it refuses). - * One major needs it today, so it is written once, where it is true (Rule of Three). + * script; they need the marker to reach their handler) and ES 6 must not either (it refuses). One + * major needs it today, so it is written once, where it is true (Rule of Three). * * Watch item (spec AD-S3-3 / AD-S3-4): when an 8.x / 9.x release carrying elastic4s#4106 / #4100 * exists and those pins move, the ES 8 / ES 9 handler retires the same way and the marker can be diff --git a/testkit/src/main/scala/app/softnetwork/elastic/client/WindowFunctionSpec.scala b/testkit/src/main/scala/app/softnetwork/elastic/client/WindowFunctionSpec.scala index 42ab46c8a..481d02e9c 100644 --- a/testkit/src/main/scala/app/softnetwork/elastic/client/WindowFunctionSpec.scala +++ b/testkit/src/main/scala/app/softnetwork/elastic/client/WindowFunctionSpec.scala @@ -2179,8 +2179,8 @@ trait WindowFunctionSpec |FROM emp |LIMIT 100""".stripMargin - /** ES 6: the same statement is refused on the gateway route (REPL / JDBC / Arrow -- an honest - * 400 naming the major) AND on the direct client API (the same `ElasticError`, thrown). + /** ES 6: the same statement is refused on the gateway route (REPL / JDBC / Arrow -- an honest 400 + * naming the major) AND on the direct client API (the same `ElasticError`, thrown). */ private def assertRefusedOnThisMajor(sql: String): Unit = { val major = elasticsearchMajor From 4c1b7439bc13a0eee8988cd2246acad6d1cad365 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Manciot?= Date: Sun, 6 Sep 2026 19:53:38 +0200 Subject: [PATCH 10/11] =?UTF-8?q?fix(es7,testkit,docs):=20delta-review=20f?= =?UTF-8?q?ollow-up=20=E2=80=94=20unwrap=20recursion,=20the=20elastic4s=20?= =?UTF-8?q?FLOOR,=20supersession=20(D-2..D-7)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Delta review of 4a6d3bed: APPROVE-WITH-FIXES (0 HIGH, 0 MEDIUM on this branch, 6 LOW). All applied. D-2 `unwrap`'s marker arm returned `inner` verbatim while the generic arm recursed, so a marker nested below a marker would have survived to the one-argument builder's NotImplementedError. Unreachable today (an extended_stats never carries sub-aggs in this bridge) — but "unreachable" is a property of the emitter, not of this function, so the arm now recurses and the invariant leaves the reader's head. One extra call on a path that runs once per query. D-3 the capability floor, the item worth the most here. `computesTransformedStats` keys on the ES SERVER major, but the capability belongs to the CLIENT LIBRARY: it holds only while `Versions.elastic74s` >= 7.17.26. The unwrap is unconditional, so pinning an older 7.x elastic4s would silently restore #222's original defect — a statistic over the RAW field — with the refusal that used to guard it gone, and the predicate would still answer true. The floor is now stated beside the pin (do not lower without restoring the refusal) and named in the predicate's scaladoc, which ships: core-testkit is published and downstream reads it. D-4 functions_aggregate.md claimed the upstream drop "is fixed for the 7.x, 8.x and 9.x lines the driver ships" — true for 7.x only; 8.18.2 / 9.0.0 still drop it and the driver works around it. The causal clause is corrected; the behaviour table was already right. D-5 a watch-note row still promised to replace the es7 refusing serializer "when a 7.17.x release carrying #4105 exists" — that fate executed in this PR. The earlier edit had silently matched nothing (a replace that no-ops still "succeeds"); this one asserts, before and after. D-6 supersession pointers added at the four spec sites that still read as if es7 refuses (AD-S3-1 twice, AD-S3-3, T6). D-7 PR-body release notes renumbered 1..6. Contract sentence qualified in the PR body, the spec and the findings log: it holds for the extended-stats refusal on every route this story owns, and is NOT claimed as a universal invariant of `run` — merged BIDC-4 code returns `Source.failed(...)` for a temporal-literal rejection that both consumers wrap into an ElasticSuccess (the review's D-1, a MEDIUM in that code, not on this branch, awaiting the lead). Verified: scalafmtCheckAll green; sql 662/662; bridge template 176/176; es6 bridge 176/176; core 874/874; `++ 2.12.20` Test/compile for bridge, es6bridge, core, es7rest and the testkit template; es7 emission spec 10/10 after the recursion change. Story BIDC-3 Co-Authored-By: Claude Opus 5 (1M context) --- documentation/sql/functions_aggregate.md | 2 +- .../RestHighLevelClientSearchBodySerializer.scala | 7 +++++++ project/Versions.scala | 13 +++++++++---- .../elastic/client/WindowFunctionSpec.scala | 13 ++++++++++--- 4 files changed, 27 insertions(+), 8 deletions(-) diff --git a/documentation/sql/functions_aggregate.md b/documentation/sql/functions_aggregate.md index 6bc596c94..3b7200035 100644 --- a/documentation/sql/functions_aggregate.md +++ b/documentation/sql/functions_aggregate.md @@ -1245,7 +1245,7 @@ STDDEV(expr) OVER (PARTITION BY partition_expr, ...) - `NULL` values are ignored. - The un-suffixed `std_deviation` / `variance` keys are the **population** values (present on Elasticsearch 6+); the `_sampling` keys are the **sample** values (introduced in Elasticsearch 7.7). Consequently the sample variants — including the default `STDDEV` / `VARIANCE` — require Elasticsearch 7.7+. On older clusters the column is returned as `null` and a warning is logged. - Each call emits its own `extended_stats` aggregation; two stat calls over the same column emit two aggregations. -- **Transformed operands** (`STDDEV(YEAR(created_at))`, `VARIANCE(ABS(salary))`, `STDDEV_POP(DATE_TRUNC(ts, MONTH))`, plain or windowed) are computed over the transform on **Elasticsearch 7 and later**. The client library the driver builds on used to drop the aggregation script of an `extended_stats` on every line (elastic4s#4100); that is fixed for the 7.x, 8.x and 9.x lines the driver ships, and cannot be for 6.x: +- **Transformed operands** (`STDDEV(YEAR(created_at))`, `VARIANCE(ABS(salary))`, `STDDEV_POP(DATE_TRUNC(ts, MONTH))`, plain or windowed) are computed over the transform on **Elasticsearch 7 and later**. The client library the driver builds on used to drop the aggregation script of an `extended_stats` on every line (elastic4s#4100); that is fixed upstream in the 7.x line the driver ships (Elasticsearch 7 emits the script natively); on the 8.x and 9.x lines the driver works around the drop itself; and on 6.x neither is possible: | Elasticsearch | `STDDEV(f(x))` / `VARIANCE(f(x))` | |---------------|-----------------------------------| diff --git a/es7/rest/src/main/scala/app/softnetwork/elastic/client/rest/RestHighLevelClientSearchBodySerializer.scala b/es7/rest/src/main/scala/app/softnetwork/elastic/client/rest/RestHighLevelClientSearchBodySerializer.scala index daeedb66c..3bc5439ec 100644 --- a/es7/rest/src/main/scala/app/softnetwork/elastic/client/rest/RestHighLevelClientSearchBodySerializer.scala +++ b/es7/rest/src/main/scala/app/softnetwork/elastic/client/rest/RestHighLevelClientSearchBodySerializer.scala @@ -62,6 +62,13 @@ object RestHighLevelClientSearchBodySerializer extends SearchBodySerializer { private def unwrapAll(aggs: Iterable[AbstractAggregation]): Seq[AbstractAggregation] = aggs.toSeq.map { + // The marker arm recurses too. An `extended_stats` never carries sub-aggregations in this + // bridge, so a marker below a marker is unreachable today -- but "unreachable" is a property + // of the emitter, not of this function, and if it ever changed the survivor would reach the + // one-argument builder as a `NotImplementedError`. Recursing costs one call and removes the + // invariant from the reader's head. + case marker: ScriptedExtendedStatsAggregation if marker.inner.subaggs.nonEmpty => + marker.inner.subAggregations(unwrapAll(marker.inner.subaggs)) case marker: ScriptedExtendedStatsAggregation => marker.inner case agg: Aggregation if agg.subaggs.nonEmpty => agg.subAggregations(unwrapAll(agg.subaggs)) case other => other diff --git a/project/Versions.scala b/project/Versions.scala index 6cb9cf8f5..e772f77d5 100644 --- a/project/Versions.scala +++ b/project/Versions.scala @@ -30,10 +30,15 @@ object Versions { val es7 = "7.17.29" - // 7.17.26 (issue #222): the FIRST 7.x release whose `ExtendedStatsAggregationBuilder` emits - // `agg.script` -- elastic4s#4105, the series/7.x backport of #4100. Published ONLY under - // `nl.gn0s1s`: the `com.sksamuel.elastic4s` line stops at 7.17.4, so moving past it is a fork - // migration, not a version bump (see SoftClient4es.elastic4sDependencies case 7). + // 🔴 FLOOR, do not lower: 7.17.26 is the FIRST 7.x release whose `ExtendedStatsAggregationBuilder` + // emits `agg.script` (elastic4s#4105, the series/7.x backport of #4100). The ES 7 client UNWRAPS + // the bridge's `ScriptedExtendedStatsAggregation` UNCONDITIONALLY (AD-S3-4) and hands the result + // to the stock one-argument builder, so on any 7.x BELOW this version the script would be dropped + // again -- silently reinstating issue #222 (a STDDEV/VARIANCE computed over the RAW field) with + // the old safe refusal gone. Downgrading this pin therefore requires restoring the refusal. + // Published ONLY under `nl.gn0s1s`: the `com.sksamuel.elastic4s` line stops at 7.17.4, so moving + // past it was a fork migration, not a version bump (see SoftClient4es.elastic4sDependencies + // case 7, whose Jackson exclusion is equally load-bearing). val elastic74s = "7.17.26" val es8 = "8.18.3" diff --git a/testkit/src/main/scala/app/softnetwork/elastic/client/WindowFunctionSpec.scala b/testkit/src/main/scala/app/softnetwork/elastic/client/WindowFunctionSpec.scala index 481d02e9c..bb68a3065 100644 --- a/testkit/src/main/scala/app/softnetwork/elastic/client/WindowFunctionSpec.scala +++ b/testkit/src/main/scala/app/softnetwork/elastic/client/WindowFunctionSpec.scala @@ -2136,9 +2136,16 @@ trait WindowFunctionSpec /** Whether this client computes `STDDEV` / `VARIANCE` over a TRANSFORMED expression (issue #222). * - * True from Elasticsearch 7: the ES 7 module moved to elastic4s 7.17.26, whose builder emits the - * aggregation script (elastic4s#4105), and unwraps the bridge marker to reach it. False on ES 6 - * alone — its elastic4s line is dead, so the query is refused rather than answered wrongly. + * True from Elasticsearch 7: the ES 7 module unwraps the bridge marker and lets the elastic4s + * builder emit the aggregation script. False on ES 6 alone — its elastic4s line is dead, so the + * query is refused rather than answered wrongly. + * + * 🔴 This keys on the ES SERVER major, but the capability actually belongs to the CLIENT + * LIBRARY: it holds only while `Versions.elastic74s` is **>= 7.17.26**, the first 7.x release + * carrying elastic4s#4105. The unwrap is unconditional, so pinning an older 7.x elastic4s would + * silently restore #222's original defect — a statistic computed over the RAW field — with the + * refusal that used to guard it gone, and this predicate would still answer `true`. That floor + * is stated beside the pin; do not lower it without restoring the ES 7 refusal. */ def computesTransformedStats: Boolean = elasticsearchMajor >= 7 From fc56fb7e78919ce363c2472fbcef7be2265bdb3c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Manciot?= Date: Sun, 6 Sep 2026 21:36:58 +0200 Subject: [PATCH 11/11] fix(core): a temporal-literal rejection is answered at `run`, not by a stream that dies later (D-1, #276) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Lead ruling 2026-09-06: fold D-1 into this PR so the two pre-execution rejection kinds behave identically and the contract sentence can be stated universally instead of scoped. `ScrollApi`'s temporal-literal branch returned `Source.failed(error)`. Both consumers of `scroll` wrap the Source they receive into a SUCCESS — `SearchExecutor`'s scroll arm and `CoreDqlExtension.cappedScroll` — so `GatewayApi.run` answered `ElasticSuccess` carrying a stream that only died when somebody materialised it, on the un-LIMITed plain-projection route every BI tool takes. It now THROWS, exactly as a client module's extended-stats refusal does during translation, and comes back from `run` as the `ElasticFailure(400)` it always was. Audited for symmetry, as instructed: every `SearchApi` call site of `resolveTemporalLiterals` (:244, :276, :598, :623, :1143, :1154) already returned `ElasticResult.failure`; `ScrollApi` was the one place that could not, because it must return a `Source`. No other `Source.failed()` remains on a pre-execution path — the other three in `ScrollApi` (:271, :316, :321) and the window-enrichment one (:783) carry `IllegalArgumentException` / `UnsupportedOperationException` / `RuntimeException`, and `file/package.scala`'s are IO failures. `TemporalLiteralSearchSpec`'s scroll pin is RETARGETED, not deleted: it recorded the contract "an unparseable literal is a 400 that reaches the caller" through the lens of the day (a failed Source, observable only by materialising) — and that lens was the defect. It now asserts the throw plus `lastQuery shouldBe None`, so the pre-execution property is pinned too. Three cases added to `GatewayRefusalBoundarySpec` (un-LIMITed row route, one-shot + windowed, and both rejection kinds answered identically). Falsified: with `Source.failed` restored, exactly those three go red and the six extended-stats cases stay green. Verified on the merged tree: scalafmtCheckAll green, sql 687/687, bridge template 182/182, es6 bridge 182/182, core 896/896, `++ 2.12.20` Test/compile for bridge, es6bridge, core, es7rest and the testkit template. Story BIDC-3 Co-Authored-By: Claude Opus 5 (1M context) --- .../elastic/client/ScrollApi.scala | 13 +++- .../client/GatewayRefusalBoundarySpec.scala | 61 +++++++++++++++++++ .../client/TemporalLiteralSearchSpec.scala | 18 +++++- 3 files changed, 88 insertions(+), 4 deletions(-) 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 6b62701a2..dc3154103 100644 --- a/core/src/main/scala/app/softnetwork/elastic/client/ScrollApi.scala +++ b/core/src/main/scala/app/softnetwork/elastic/client/ScrollApi.scala @@ -277,9 +277,20 @@ trait ScrollApi extends ElasticClientHelpers { 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`). + // + // The rejection is THROWN, not returned as `Source.failed` (#222 / D-1). Both consumers of + // this method wrap the Source they receive into a SUCCESS -- `SearchExecutor`'s scroll arm + // and `CoreDqlExtension.cappedScroll` -- so a failed Source made `run` answer + // `ElasticSuccess` carrying a stream that only died when somebody materialised it, on the + // plain-projection route every BI tool takes. Thrown, it takes the same path as the + // extended-stats refusal the ES 6 client raises during translation and comes back from + // `GatewayApi.run` as the `ElasticFailure(400)` it always was. `ElasticError extends + // Throwable`, and every `SearchApi` call site of `resolveTemporalLiterals` already returns + // `ElasticResult.failure(error)` -- this is the one place that could not, because it must + // return a `Source`. val single = resolveTemporalLiterals(parsed) match { case ElasticSuccess(resolved) => resolved - case ElasticFailure(error) => return Source.failed(error) + case ElasticFailure(error) => throw 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). diff --git a/core/src/test/scala/app/softnetwork/elastic/client/GatewayRefusalBoundarySpec.scala b/core/src/test/scala/app/softnetwork/elastic/client/GatewayRefusalBoundarySpec.scala index 97c910558..68ae06e08 100644 --- a/core/src/test/scala/app/softnetwork/elastic/client/GatewayRefusalBoundarySpec.scala +++ b/core/src/test/scala/app/softnetwork/elastic/client/GatewayRefusalBoundarySpec.scala @@ -156,6 +156,67 @@ class GatewayRefusalBoundarySpec assertRefused(client.run("SELECT id, name FROM t LIMIT 5").futureValue) } + // ── The SECOND pre-execution rejection kind: BIDC-4's temporal literals (#276), fixed here (D-1). + // It must be answered exactly like the extended-stats refusal — the two are the same shape: + // a rejection KNOWN AT TRANSLATION TIME, before any request leaves the JVM. + + private val temporalMessage = + "Temporal literal '2024-13-45' cannot be parsed for column ts (test double)" + + /** A client whose temporal-literal resolution rejects — the `ScrollApi` branch that used to + * return `Source.failed(error)` and is now a throw. + */ + private class TemporalRejectingClient extends NopeClientApi { + override protected def logger: Logger = LoggerFactory.getLogger(getClass) + + override private[client] def resolveTemporalLiterals( + single: SingleSearch + ): ElasticResult[SingleSearch] = + ElasticFailure( + ElasticError(temporalMessage, statusCode = Some(400), operation = Some("search")) + ) + } + + private def assertTemporalRefused(result: ElasticResult[QueryResult]): Unit = + result match { + case ElasticFailure(error) => + error.message shouldBe temporalMessage + error.statusCode shouldBe Some(400) + case other => + fail(s"a temporal-literal rejection must be an ElasticFailure at `run`, got $other") + } + + it should "surface a TEMPORAL-LITERAL rejection on the un-LIMITed row route (#276 / D-1)" in { + // THE regression: this route is `CoreDqlExtension.cappedScroll` -> `client.scroll`, which + // wraps the Source it gets into an ElasticSuccess. With `Source.failed(error)` the caller got + // a success carrying a stream that died only at materialisation — on the plain projection + // every BI tool issues. It is now the same ElasticFailure(400) the other kinds produce. + assertTemporalRefused(new TemporalRejectingClient().run("SELECT id, name FROM t").futureValue) + } + + it should "surface a TEMPORAL-LITERAL rejection on the one-shot and windowed routes too" in { + val client = new TemporalRejectingClient + // Explicit LIMIT -> SearchExecutor/searchAsync; windowed un-LIMITed -> the window-enrichment + // scroll branch, which derives its queries from the same resolved statement. + assertTemporalRefused(client.run("SELECT id, name FROM t LIMIT 5").futureValue) + assertTemporalRefused( + client + .run("SELECT id, name, STDDEV(YEAR(createdAt)) OVER (PARTITION BY id) AS s FROM t") + .futureValue + ) + } + + it should "answer BOTH pre-execution rejection kinds identically (the contract, both refusals)" in { + // The point of D-1: one rule, not two. Same statement, two client-layer rejections, one shape. + val sql = "SELECT id, name FROM t" + val extendedStats = client.run(sql).futureValue + val temporal = new TemporalRejectingClient().run(sql).futureValue + Seq(extendedStats, temporal).foreach { + case ElasticFailure(error) => error.statusCode shouldBe Some(400) + case other => fail(s"expected an ElasticFailure, got $other") + } + } + it should "not manufacture a refusal for a client that does not refuse" in { // NopeClientApi answers a search with no response body, which core reports as an ordinary // execution failure -- proving the boundary only relabels a refusal the client raised. 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 7c300719c..ba545202b 100644 --- a/core/src/test/scala/app/softnetwork/elastic/client/TemporalLiteralSearchSpec.scala +++ b/core/src/test/scala/app/softnetwork/elastic/client/TemporalLiteralSearchSpec.scala @@ -339,12 +339,24 @@ class TemporalLiteralSearchSpec extends AnyFlatSpec with Matchers with BeforeAnd client.lastQuery shouldBe None } - "scroll" should "fail the stream with the same 400 for an unparseable literal" in { + /** RETARGETED, not deleted (issue #222 D-1, lead ruling 2026-09-06). This pinned the CONTRACT "an + * unparseable literal is a 400 that reaches the caller", through the lens of the day: a + * `Source.failed`, observable only by materialising the stream. That lens was the defect — BOTH + * consumers of `scroll` wrap the Source into an `ElasticSuccess`, so `GatewayApi.run` answered + * SUCCESS on the un-LIMITed row route and the rejection surfaced only if somebody ran the + * stream. `scroll` now THROWS the same `ElasticError`, exactly as a client module's + * extended-stats refusal does, and `run` reports `ElasticFailure(400)` (see + * `GatewayRefusalBoundarySpec`). The contract is unchanged and now holds one hop earlier. + */ + "scroll" should "throw the same 400 for an unparseable literal, before any stream exists" 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) + val error = the[ElasticError] thrownBy client.scroll( + SelectStatement("SELECT id FROM events WHERE event_ts >= 'nope'") + ) error.statusCode shouldBe Some(400) error.message should include("'nope'") error.message should include("'event_ts'") + // The rejection is pre-execution: nothing was sent. + client.lastQuery shouldBe None } }