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 f01cfa5e..f71e92e8 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,23 @@ 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). + * + * 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)) } object ElasticAggregation { @@ -196,7 +208,11 @@ 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 +238,11 @@ 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 c6e74b03..0388fc6f 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 cfe03ba2..3337d604 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 00000000..eeaef493 --- /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 00000000..5082ecc1 --- /dev/null +++ b/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/SearchBodySerializer.scala @@ -0,0 +1,74 @@ +/* + * 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 63fc7d0d..b720d45c 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/bridge/src/test/scala/app/softnetwork/elastic/sql/AggregationNamingSpec.scala b/bridge/src/test/scala/app/softnetwork/elastic/sql/AggregationNamingSpec.scala index 543fec33..921d832f 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 00000000..46c69d42 --- /dev/null +++ b/bridge/src/test/scala/app/softnetwork/elastic/sql/ExtendedStatsEmissionSpec.scala @@ -0,0 +1,263 @@ +/* + * 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._ +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 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 = + 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 e4c8d0ad..f7b9e65b 100644 --- a/core/src/main/scala/app/softnetwork/elastic/client/GatewayApi.scala +++ b/core/src/main/scala/app/softnetwork/elastic/client/GatewayApi.scala @@ -1931,6 +1931,37 @@ trait GatewayApi extends IndicesApi with ElasticClientHelpers { )(implicit system: ActorSystem): Future[ElasticResult[QueryResult]] = { implicit val ec: ExecutionContext = system.dispatcher + // 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)) + } + } + + 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/main/scala/app/softnetwork/elastic/client/ScrollApi.scala b/core/src/main/scala/app/softnetwork/elastic/client/ScrollApi.scala index 8aa98221..dc315410 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). @@ -725,12 +736,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) @@ -746,11 +768,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 @@ -780,10 +798,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 7a26ba4d..72ebdbb1 100644 --- a/core/src/main/scala/app/softnetwork/elastic/client/SearchApi.scala +++ b/core/src/main/scala/app/softnetwork/elastic/client/SearchApi.scala @@ -1537,20 +1537,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 new file mode 100644 index 00000000..68ae06e0 --- /dev/null +++ b/core/src/test/scala/app/softnetwork/elastic/client/GatewayRefusalBoundarySpec.scala @@ -0,0 +1,233 @@ +/* + * 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, Statement} +import com.typesafe.config.ConfigFactory +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._ +import scala.concurrent.Future + +/** 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), 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. + * + * 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 { + // 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 "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) + } + + // ── 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. + 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/core/src/test/scala/app/softnetwork/elastic/client/TemporalLiteralSearchSpec.scala b/core/src/test/scala/app/softnetwork/elastic/client/TemporalLiteralSearchSpec.scala index 7c300719..ba545202 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 } } diff --git a/documentation/sql/dql_statements.md b/documentation/sql/dql_statements.md index 19935444..6b1c40a2 100644 --- a/documentation/sql/dql_statements.md +++ b/documentation/sql/dql_statements.md @@ -512,6 +512,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 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` - `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 8a74355a..3b720003 100644 --- a/documentation/sql/functions_aggregate.md +++ b/documentation/sql/functions_aggregate.md @@ -1245,6 +1245,14 @@ 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 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))` | + |---------------|-----------------------------------| + | 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. **Examples:** ```sql diff --git a/documentation/sql/known_limitations.md b/documentation/sql/known_limitations.md index 5f5bb574..04d11bfa 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 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 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) - **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/main/scala/app/softnetwork/elastic/sql/bridge/ElasticAggregation.scala b/es6/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/ElasticAggregation.scala index 41ccf34e..99812237 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,23 @@ 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). + * + * 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)) } object ElasticAggregation { @@ -197,7 +209,11 @@ 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 +239,11 @@ 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 0a904a6d..13762f6c 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 ff2463e8..6782773f 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 00000000..13ea6d76 --- /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 00000000..87c18306 --- /dev/null +++ b/es6/bridge/src/main/scala/app/softnetwork/elastic/sql/bridge/SearchBodySerializer.scala @@ -0,0 +1,70 @@ +/* + * 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 38e3e4a9..050100a8 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/bridge/src/test/scala/app/softnetwork/elastic/sql/AggregationNamingSpec.scala b/es6/bridge/src/test/scala/app/softnetwork/elastic/sql/AggregationNamingSpec.scala index 543fec33..63acdbc5 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 00000000..a7f68ee9 --- /dev/null +++ b/es6/bridge/src/test/scala/app/softnetwork/elastic/sql/ExtendedStatsEmissionSpec.scala @@ -0,0 +1,263 @@ +/* + * 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._ +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 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 = + if (SearchBodySerializer.hasTransformExtendedStats(search)) "" + else SearchBodySerializer.Default.serialize(search) + } +} 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 5a34f282..5fdb8ecc 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 00000000..aaae20f8 --- /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/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 00000000..b7b9c614 --- /dev/null +++ b/es6/jest/src/test/scala/app/softnetwork/elastic/client/JestClientExtendedStatsRejectionSpec.scala @@ -0,0 +1,119 @@ +/* + * 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/main/scala/app/softnetwork/elastic/client/rest/RestHighLevelClientApi.scala b/es6/rest/src/main/scala/app/softnetwork/elastic/client/rest/RestHighLevelClientApi.scala index a5ffc81f..7031b951 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 00000000..285ae7f1 --- /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/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 00000000..e545bc5f --- /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/main/scala/app/softnetwork/elastic/client/rest/RestHighLevelClientApi.scala b/es7/rest/src/main/scala/app/softnetwork/elastic/client/rest/RestHighLevelClientApi.scala index b4a25a5e..b2626707 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`): 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 + 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 00000000..3bc5439e --- /dev/null +++ b/es7/rest/src/main/scala/app/softnetwork/elastic/client/rest/RestHighLevelClientSearchBodySerializer.scala @@ -0,0 +1,79 @@ +/* + * 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.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): **UNWRAP**, then serialise with elastic4s's own + * one-argument builder. + * + * 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 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 { + // 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 + } + + 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 00000000..2d4c9bbd --- /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/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 7a5ee78d..9fac1977 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 @@ -1026,6 +1026,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 00000000..6ff2d46f --- /dev/null +++ b/es8/java/src/main/scala/app/softnetwork/elastic/client/java/JavaClientSearchBodySerializer.scala @@ -0,0 +1,91 @@ +/* + * 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.{ + 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 + * [[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 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() + 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/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 00000000..35826489 --- /dev/null +++ b/es8/java/src/test/scala/app/softnetwork/elastic/client/JavaClientExtendedStatsEmissionSpec.scala @@ -0,0 +1,203 @@ +/* + * 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.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 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 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 { + 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}}}}""" + ) + } + + "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 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 + } + } + } + } + } + + "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 + } + + "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/JavaClientApi.scala b/es9/java/src/main/scala/app/softnetwork/elastic/client/java/JavaClientApi.scala index 1892d1fa..09681fda 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 @@ -1026,6 +1026,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 00000000..6ff2d46f --- /dev/null +++ b/es9/java/src/main/scala/app/softnetwork/elastic/client/java/JavaClientSearchBodySerializer.scala @@ -0,0 +1,91 @@ +/* + * 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.{ + 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 + * [[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 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() + 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/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 00000000..35826489 --- /dev/null +++ b/es9/java/src/test/scala/app/softnetwork/elastic/client/JavaClientExtendedStatsEmissionSpec.scala @@ -0,0 +1,203 @@ +/* + * 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.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 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 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 { + 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}}}}""" + ) + } + + "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 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 + } + } + } + } + } + + "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 + } + + "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/project/SoftClient4es.scala b/project/SoftClient4es.scala index b1013afa..04453910 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 1b8e4a3b..e772f77d 100644 --- a/project/Versions.scala +++ b/project/Versions.scala @@ -30,7 +30,16 @@ object Versions { val es7 = "7.17.29" - val elastic74s = "7.17.4" + // 🔴 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 eadd93e1..bb68a306 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,172 @@ trait WindowFunctionSpec } } + // ======================================================================== + // ISSUE #222 (story BIDC-3) — STDDEV / VARIANCE over a TRANSFORMED expression + // + // 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 = + client.asInstanceOf[VersionApi].version match { + case ElasticSuccess(v) => v.split("\\.").head.toInt + case ElasticFailure(error) => + 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 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 + + /** 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: 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 7+ and refuse loudly on ES 6 (issue #222)" in { + if (computesTransformedStats) { + 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 7+ and refuse loudly on ES 6 (issue #222)" in { + if (computesTransformedStats) { + 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 a9b83085..746f816e 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. */