From d3c4c5b48bd12730ef8f50c0ba679bbb384f385d Mon Sep 17 00:00:00 2001 From: nikhilkumarjadhav-toast Date: Thu, 6 Aug 2026 14:55:24 -0700 Subject: [PATCH 1/7] Add approximatePercentile aggregated value function Adds an approximate-percentile aggregation to the aggregatedValues API for Float, Int, JsonSafeLong, LongString, Date, DateTime, and LocalTime fields. Callers request a specific percentile rank via a `percentile` argument (e.g. `percentile: 50` for the median), and can request multiple ranks in a single query by aliasing the field selection. The aliasing-based shape keeps query validity statically verifiable: each aliased selection independently validates its own rank and always returns exactly one value, with no ambiguity around duplicate or out-of-range requests the way a list-argument API would have. An out-of-range `percentile` resolves to `null` with a precisely-pathed GraphQL error rather than failing the entire aggregations subtree, so sibling fields (including other, valid `approximatePercentile` selections) still resolve normally. --- config/schema/artifacts/runtime_metadata.yaml | 28 +++++ config/schema/artifacts/schema.graphql | 112 ++++++++++++++++++ .../runtime_metadata.yaml | 28 +++++ .../artifacts_with_apollo/schema.graphql | 112 ++++++++++++++++++ .../BluegrassArtistLifetimeSales.graphql | 3 + .../aggregations/aggregated-values.md | 16 ++- .../graphql/aggregation/function_adapter.rb | 38 ++++++ .../graphql/aggregation/query_adapter.rb | 14 ++- .../resolvers/aggregated_values.rb | 29 ++++- .../graphql/aggregation/resolvers/node.rb | 2 +- .../graphql/aggregation/function_adapter.rbs | 16 ++- .../resolvers/aggregated_values.rbs | 5 + elasticgraph-graphql/sig/graphql_gem.rbs | 2 + .../spec/acceptance/aggregations_spec.rb | 61 ++++++++++ .../spec/acceptance/graphql_types_spec.rb | 9 ++ .../aggregation/function_adapter_spec.rb | 46 ++++++- .../resolvers/aggregation_resolver_support.rb | 4 +- .../resolvers/aggregations_spec.rb | 40 +++++++ .../runtime_metadata/schema_element_names.rb | 2 +- .../runtime_metadata/schema_element_names.rbs | 2 + .../schema_elements/built_in_types.rb | 35 +++++- .../graphql_schema/built_in_types_spec.rb | 52 ++++++++ .../for_built_in_types_spec.rb | 12 +- 23 files changed, 649 insertions(+), 19 deletions(-) diff --git a/config/schema/artifacts/runtime_metadata.yaml b/config/schema/artifacts/runtime_metadata.yaml index f615e096a..3c04f2480 100644 --- a/config/schema/artifacts/runtime_metadata.yaml +++ b/config/schema/artifacts/runtime_metadata.yaml @@ -4033,6 +4033,10 @@ object_types_by_name: computation_function: cardinality resolver: name: object_with_lookahead + approximate_percentile: + computation_function: percentiles + resolver: + name: object_with_lookahead exact_max: computation_function: max resolver: @@ -4067,6 +4071,10 @@ object_types_by_name: computation_function: cardinality resolver: name: object_with_lookahead + approximate_percentile: + computation_function: percentiles + resolver: + name: object_with_lookahead exact_max: computation_function: max resolver: @@ -4413,6 +4421,10 @@ object_types_by_name: computation_function: cardinality resolver: name: object_with_lookahead + approximate_percentile: + computation_function: percentiles + resolver: + name: object_with_lookahead approximate_sum: computation_function: sum resolver: @@ -4673,6 +4685,10 @@ object_types_by_name: computation_function: cardinality resolver: name: object_with_lookahead + approximate_percentile: + computation_function: percentiles + resolver: + name: object_with_lookahead approximate_sum: computation_function: sum resolver: @@ -4826,6 +4842,10 @@ object_types_by_name: computation_function: cardinality resolver: name: object_with_lookahead + approximate_percentile: + computation_function: percentiles + resolver: + name: object_with_lookahead approximate_sum: computation_function: sum resolver: @@ -4858,6 +4878,10 @@ object_types_by_name: computation_function: cardinality resolver: name: object_with_lookahead + approximate_percentile: + computation_function: percentiles + resolver: + name: object_with_lookahead exact_max: computation_function: max resolver: @@ -4886,6 +4910,10 @@ object_types_by_name: computation_function: min resolver: name: object_with_lookahead + approximate_percentile: + computation_function: percentiles + resolver: + name: object_with_lookahead approximate_sum: computation_function: sum resolver: diff --git a/config/schema/artifacts/schema.graphql b/config/schema/artifacts/schema.graphql index edca147a5..2797a6c9f 100644 --- a/config/schema/artifacts/schema.graphql +++ b/config/schema/artifacts/schema.graphql @@ -2169,6 +2169,22 @@ type DateAggregatedValues { """ approximate_distinct_value_count: JsonSafeLong + """ + An approximate percentile of the field values within this grouping. + + Percentiles are computed using an approximate algorithm, so the returned value may not be + exact. Request a specific percentile via the `percentile` argument (e.g. + `percentile: 50` for the median). To request multiple percentiles in a single query, + use a GraphQL alias for each: `p50: approximate_percentile(percentile: 50)`. + """ + approximate_percentile( + """ + The percentile rank to compute, from `0` to `100` (e.g. `0` for the min, + `100` for the max, `50` for the median, `99` for the 99th percentile). + """ + percentile: Float! + ): Date + """ The maximum of the field values within this grouping. @@ -2486,6 +2502,22 @@ type DateTimeAggregatedValues { """ approximate_distinct_value_count: JsonSafeLong + """ + An approximate percentile of the field values within this grouping. + + Percentiles are computed using an approximate algorithm, so the returned value may not be + exact. Request a specific percentile via the `percentile` argument (e.g. + `percentile: 50` for the median). To request multiple percentiles in a single query, + use a GraphQL alias for each: `p50: approximate_percentile(percentile: 50)`. + """ + approximate_percentile( + """ + The percentile rank to compute, from `0` to `100` (e.g. `0` for the min, + `100` for the max, `50` for the median, `99` for the 99th percentile). + """ + percentile: Float! + ): DateTime + """ The maximum of the field values within this grouping. @@ -3778,6 +3810,22 @@ type FloatAggregatedValues { """ approximate_distinct_value_count: JsonSafeLong + """ + An approximate percentile of the field values within this grouping. + + Percentiles are computed using an approximate algorithm, so the returned value may not be + exact. Request a specific percentile via the `percentile` argument (e.g. + `percentile: 50` for the median). To request multiple percentiles in a single query, + use a GraphQL alias for each: `p50: approximate_percentile(percentile: 50)`. + """ + approximate_percentile( + """ + The percentile rank to compute, from `0` to `100` (e.g. `0` for the min, + `100` for the max, `50` for the median, `99` for the 99th percentile). + """ + percentile: Float! + ): Float + """ The sum of the field values within this grouping. @@ -4577,6 +4625,22 @@ type IntAggregatedValues { """ approximate_distinct_value_count: JsonSafeLong + """ + An approximate percentile of the field values within this grouping. + + Percentiles are computed using an approximate algorithm, so the returned value may not be + exact. Request a specific percentile via the `percentile` argument (e.g. + `percentile: 50` for the median). To request multiple percentiles in a single query, + use a GraphQL alias for each: `p50: approximate_percentile(percentile: 50)`. + """ + approximate_percentile( + """ + The percentile rank to compute, from `0` to `100` (e.g. `0` for the min, + `100` for the max, `50` for the median, `99` for the 99th percentile). + """ + percentile: Float! + ): Float + """ The (approximate) sum of the field values within this grouping. @@ -5162,6 +5226,22 @@ type JsonSafeLongAggregatedValues { """ approximate_distinct_value_count: JsonSafeLong + """ + An approximate percentile of the field values within this grouping. + + Percentiles are computed using an approximate algorithm, so the returned value may not be + exact. Request a specific percentile via the `percentile` argument (e.g. + `percentile: 50` for the median). To request multiple percentiles in a single query, + use a GraphQL alias for each: `p50: approximate_percentile(percentile: 50)`. + """ + approximate_percentile( + """ + The percentile rank to compute, from `0` to `100` (e.g. `0` for the min, + `100` for the max, `50` for the median, `99` for the 99th percentile). + """ + percentile: Float! + ): Float + """ The (approximate) sum of the field values within this grouping. @@ -5413,6 +5493,22 @@ type LocalTimeAggregatedValues { """ approximate_distinct_value_count: JsonSafeLong + """ + An approximate percentile of the field values within this grouping. + + Percentiles are computed using an approximate algorithm, so the returned value may not be + exact. Request a specific percentile via the `percentile` argument (e.g. + `percentile: 50` for the median). To request multiple percentiles in a single query, + use a GraphQL alias for each: `p50: approximate_percentile(percentile: 50)`. + """ + approximate_percentile( + """ + The percentile rank to compute, from `0` to `100` (e.g. `0` for the min, + `100` for the max, `50` for the median, `99` for the 99th percentile). + """ + percentile: Float! + ): LocalTime + """ The maximum of the field values within this grouping. @@ -5623,6 +5719,22 @@ type LongStringAggregatedValues { """ approximate_min: LongString + """ + An approximate percentile of the field values within this grouping. + + Percentiles are computed using an approximate algorithm, so the returned value may not be + exact. Request a specific percentile via the `percentile` argument (e.g. + `percentile: 50` for the median). To request multiple percentiles in a single query, + use a GraphQL alias for each: `p50: approximate_percentile(percentile: 50)`. + """ + approximate_percentile( + """ + The percentile rank to compute, from `0` to `100` (e.g. `0` for the min, + `100` for the max, `50` for the median, `99` for the 99th percentile). + """ + percentile: Float! + ): Float + """ The (approximate) sum of the field values within this grouping. diff --git a/config/schema/artifacts_with_apollo/runtime_metadata.yaml b/config/schema/artifacts_with_apollo/runtime_metadata.yaml index 3bdc412ac..0763127a2 100644 --- a/config/schema/artifacts_with_apollo/runtime_metadata.yaml +++ b/config/schema/artifacts_with_apollo/runtime_metadata.yaml @@ -4135,6 +4135,10 @@ object_types_by_name: computation_function: cardinality resolver: name: object_with_lookahead + approximate_percentile: + computation_function: percentiles + resolver: + name: object_with_lookahead exact_max: computation_function: max resolver: @@ -4169,6 +4173,10 @@ object_types_by_name: computation_function: cardinality resolver: name: object_with_lookahead + approximate_percentile: + computation_function: percentiles + resolver: + name: object_with_lookahead exact_max: computation_function: max resolver: @@ -4515,6 +4523,10 @@ object_types_by_name: computation_function: cardinality resolver: name: object_with_lookahead + approximate_percentile: + computation_function: percentiles + resolver: + name: object_with_lookahead approximate_sum: computation_function: sum resolver: @@ -4775,6 +4787,10 @@ object_types_by_name: computation_function: cardinality resolver: name: object_with_lookahead + approximate_percentile: + computation_function: percentiles + resolver: + name: object_with_lookahead approximate_sum: computation_function: sum resolver: @@ -4928,6 +4944,10 @@ object_types_by_name: computation_function: cardinality resolver: name: object_with_lookahead + approximate_percentile: + computation_function: percentiles + resolver: + name: object_with_lookahead approximate_sum: computation_function: sum resolver: @@ -4960,6 +4980,10 @@ object_types_by_name: computation_function: cardinality resolver: name: object_with_lookahead + approximate_percentile: + computation_function: percentiles + resolver: + name: object_with_lookahead exact_max: computation_function: max resolver: @@ -4988,6 +5012,10 @@ object_types_by_name: computation_function: min resolver: name: object_with_lookahead + approximate_percentile: + computation_function: percentiles + resolver: + name: object_with_lookahead approximate_sum: computation_function: sum resolver: diff --git a/config/schema/artifacts_with_apollo/schema.graphql b/config/schema/artifacts_with_apollo/schema.graphql index 78006c098..c6308e642 100644 --- a/config/schema/artifacts_with_apollo/schema.graphql +++ b/config/schema/artifacts_with_apollo/schema.graphql @@ -2436,6 +2436,22 @@ type DateAggregatedValues @shareable { """ approximate_distinct_value_count: JsonSafeLong + """ + An approximate percentile of the field values within this grouping. + + Percentiles are computed using an approximate algorithm, so the returned value may not be + exact. Request a specific percentile via the `percentile` argument (e.g. + `percentile: 50` for the median). To request multiple percentiles in a single query, + use a GraphQL alias for each: `p50: approximate_percentile(percentile: 50)`. + """ + approximate_percentile( + """ + The percentile rank to compute, from `0` to `100` (e.g. `0` for the min, + `100` for the max, `50` for the median, `99` for the 99th percentile). + """ + percentile: Float! + ): Date + """ The maximum of the field values within this grouping. @@ -2753,6 +2769,22 @@ type DateTimeAggregatedValues @shareable { """ approximate_distinct_value_count: JsonSafeLong + """ + An approximate percentile of the field values within this grouping. + + Percentiles are computed using an approximate algorithm, so the returned value may not be + exact. Request a specific percentile via the `percentile` argument (e.g. + `percentile: 50` for the median). To request multiple percentiles in a single query, + use a GraphQL alias for each: `p50: approximate_percentile(percentile: 50)`. + """ + approximate_percentile( + """ + The percentile rank to compute, from `0` to `100` (e.g. `0` for the min, + `100` for the max, `50` for the median, `99` for the 99th percentile). + """ + percentile: Float! + ): DateTime + """ The maximum of the field values within this grouping. @@ -4060,6 +4092,22 @@ type FloatAggregatedValues @shareable { """ approximate_distinct_value_count: JsonSafeLong + """ + An approximate percentile of the field values within this grouping. + + Percentiles are computed using an approximate algorithm, so the returned value may not be + exact. Request a specific percentile via the `percentile` argument (e.g. + `percentile: 50` for the median). To request multiple percentiles in a single query, + use a GraphQL alias for each: `p50: approximate_percentile(percentile: 50)`. + """ + approximate_percentile( + """ + The percentile rank to compute, from `0` to `100` (e.g. `0` for the min, + `100` for the max, `50` for the median, `99` for the 99th percentile). + """ + percentile: Float! + ): Float + """ The sum of the field values within this grouping. @@ -4859,6 +4907,22 @@ type IntAggregatedValues @shareable { """ approximate_distinct_value_count: JsonSafeLong + """ + An approximate percentile of the field values within this grouping. + + Percentiles are computed using an approximate algorithm, so the returned value may not be + exact. Request a specific percentile via the `percentile` argument (e.g. + `percentile: 50` for the median). To request multiple percentiles in a single query, + use a GraphQL alias for each: `p50: approximate_percentile(percentile: 50)`. + """ + approximate_percentile( + """ + The percentile rank to compute, from `0` to `100` (e.g. `0` for the min, + `100` for the max, `50` for the median, `99` for the 99th percentile). + """ + percentile: Float! + ): Float + """ The (approximate) sum of the field values within this grouping. @@ -5444,6 +5508,22 @@ type JsonSafeLongAggregatedValues @shareable { """ approximate_distinct_value_count: JsonSafeLong + """ + An approximate percentile of the field values within this grouping. + + Percentiles are computed using an approximate algorithm, so the returned value may not be + exact. Request a specific percentile via the `percentile` argument (e.g. + `percentile: 50` for the median). To request multiple percentiles in a single query, + use a GraphQL alias for each: `p50: approximate_percentile(percentile: 50)`. + """ + approximate_percentile( + """ + The percentile rank to compute, from `0` to `100` (e.g. `0` for the min, + `100` for the max, `50` for the median, `99` for the 99th percentile). + """ + percentile: Float! + ): Float + """ The (approximate) sum of the field values within this grouping. @@ -5695,6 +5775,22 @@ type LocalTimeAggregatedValues @shareable { """ approximate_distinct_value_count: JsonSafeLong + """ + An approximate percentile of the field values within this grouping. + + Percentiles are computed using an approximate algorithm, so the returned value may not be + exact. Request a specific percentile via the `percentile` argument (e.g. + `percentile: 50` for the median). To request multiple percentiles in a single query, + use a GraphQL alias for each: `p50: approximate_percentile(percentile: 50)`. + """ + approximate_percentile( + """ + The percentile rank to compute, from `0` to `100` (e.g. `0` for the min, + `100` for the max, `50` for the median, `99` for the 99th percentile). + """ + percentile: Float! + ): LocalTime + """ The maximum of the field values within this grouping. @@ -5905,6 +6001,22 @@ type LongStringAggregatedValues @shareable { """ approximate_min: LongString + """ + An approximate percentile of the field values within this grouping. + + Percentiles are computed using an approximate algorithm, so the returned value may not be + exact. Request a specific percentile via the `percentile` argument (e.g. + `percentile: 50` for the median). To request multiple percentiles in a single query, + use a GraphQL alias for each: `p50: approximate_percentile(percentile: 50)`. + """ + approximate_percentile( + """ + The percentile rank to compute, from `0` to `100` (e.g. `0` for the min, + `100` for the max, `50` for the median, `99` for the 99th percentile). + """ + percentile: Float! + ): Float + """ The (approximate) sum of the field values within this grouping. diff --git a/config/site/examples/music/queries/aggregations/BluegrassArtistLifetimeSales.graphql b/config/site/examples/music/queries/aggregations/BluegrassArtistLifetimeSales.graphql index 1c0ae8926..2529ca528 100644 --- a/config/site/examples/music/queries/aggregations/BluegrassArtistLifetimeSales.graphql +++ b/config/site/examples/music/queries/aggregations/BluegrassArtistLifetimeSales.graphql @@ -16,6 +16,9 @@ query BluegrassArtistLifetimeSales { approximateSum approximateAvg + + median: approximatePercentile(percentile: 50) + p90: approximatePercentile(percentile: 90) } } } diff --git a/config/site/src/query-api/aggregations/aggregated-values.md b/config/site/src/query-api/aggregations/aggregated-values.md index 2c26c794b..1896b8032 100644 --- a/config/site/src/query-api/aggregations/aggregated-values.md +++ b/config/site/src/query-api/aggregations/aggregated-values.md @@ -10,7 +10,7 @@ Here's an example: {% include copyable_code_snippet.html language="graphql" data="music_queries.aggregations.BluegrassArtistLifetimeSales" %} -This example query aggregates the values of the `Artist.lifetimeSales` field using all 4 of the standard numeric +This example query aggregates the values of the `Artist.lifetimeSales` field using all 4 of the basic numeric aggregated values: `min`, `max`, `avg`, and `sum`. These are qualified with `approximate` or `exact` to indicate the level of precision they offer. The documentation for `approximateSum` and `exactSum` provides more detail: @@ -28,7 +28,19 @@ the level of precision they offer. The documentation for `approximateSum` and `e fit in a `JsonSafeLong`. In that case, `null` will be returned, and `approximateSum` can be used to get an approximate value. -Besides these standard numeric aggregated values, ElasticGraph offers one more: +The same example also requests two percentiles of `lifetimeSales`, using aliases (`median`/`p90`) to request +both in a single query: + +`approximatePercentile` +: An approximate percentile of the field values within this grouping. The `percentile` argument specifies + the desired percentile rank, from `0` to `100` (e.g. `50` for the median, `90` for the 90th percentile). + + Percentiles are computed using an approximate algorithm, so the returned value may not be exact--this is + true regardless of the field's type, so there is no `exactPercentile` counterpart the way there is for + `min`/`max`/`sum`. To request multiple percentiles in a single query, use a GraphQL alias for each + selection, as the example above does. + +Besides these basic numeric aggregated values, ElasticGraph offers one more: {% include copyable_code_snippet.html language="graphql" data="music_queries.aggregations.SkaArtistHomeCountries" %} diff --git a/elasticgraph-graphql/lib/elastic_graph/graphql/aggregation/function_adapter.rb b/elasticgraph-graphql/lib/elastic_graph/graphql/aggregation/function_adapter.rb index b91d469fc..1bb124a07 100644 --- a/elasticgraph-graphql/lib/elastic_graph/graphql/aggregation/function_adapter.rb +++ b/elasticgraph-graphql/lib/elastic_graph/graphql/aggregation/function_adapter.rb @@ -6,6 +6,8 @@ # # frozen_string_literal: true +require "graphql" + module ElasticGraph class GraphQL module Aggregation @@ -39,12 +41,48 @@ def empty_bucket_result end end + # Adapter for the `percentiles` aggregation, which takes a `percentile` argument and + # returns a nested `{"values" => [{"key" => ..., "value" => ...}]}` response rather than + # the flat `{"value" => ...}` shape `SimpleMetric` handles. + # + # @private + class Percentile < ::Data.define(:datastore_function_name) + def extract_args(args, element_names) + percentile = args.fetch(element_names.percentile) + + unless percentile.is_a?(::Numeric) && percentile >= 0 && percentile <= 100 + raise ::GraphQL::ExecutionError, "`#{element_names.percentile}` must be between 0 and 100, but is #{percentile.inspect}." + end + + {percentile: percentile} + end + + def clause_options(function_args) + # `keyed: false` gives us an array response (`"values" => [{"key" => ..., "value" => ...}]`) + # instead of a string-keyed hash (`"values" => {"50.0" => ...}`), which avoids having to + # reconstruct the datastore's float-formatted string key to look up our single requested value. + {"percents" => [function_args.fetch(:percentile)], "keyed" => false} + end + + def extract_result(raw) + # We always request exactly one percentile per computation (multiple requested ranks are + # expressed as multiple aliased GraphQL field selections, each becoming its own computation), + # so the single entry in `values` is always the one we want. + raw.fetch("values").first + end + + def empty_bucket_result + {"values" => [{"value" => nil}]} + end + end + # The registered adapters, keyed by the function name used in runtime metadata. BY_NAME = { avg: SimpleMetric.new(datastore_function_name: "avg", empty_bucket_value: nil), cardinality: SimpleMetric.new(datastore_function_name: "cardinality", empty_bucket_value: 0), max: SimpleMetric.new(datastore_function_name: "max", empty_bucket_value: nil), min: SimpleMetric.new(datastore_function_name: "min", empty_bucket_value: nil), + percentiles: Percentile.new(datastore_function_name: "percentiles"), sum: SimpleMetric.new(datastore_function_name: "sum", empty_bucket_value: 0) }.freeze end diff --git a/elasticgraph-graphql/lib/elastic_graph/graphql/aggregation/query_adapter.rb b/elasticgraph-graphql/lib/elastic_graph/graphql/aggregation/query_adapter.rb index 924d425bd..4a3805850 100644 --- a/elasticgraph-graphql/lib/elastic_graph/graphql/aggregation/query_adapter.rb +++ b/elasticgraph-graphql/lib/elastic_graph/graphql/aggregation/query_adapter.rb @@ -17,6 +17,7 @@ require "elastic_graph/graphql/schema/arguments" require "elastic_graph/support/hash_util" require "elastic_graph/support/memoizable_data" +require "graphql" module ElasticGraph class GraphQL @@ -186,15 +187,24 @@ def build_computations_from(node_node, from_field_path: []) if field.aggregated? field_path = from_field_path + field_path - get_children_nodes(node).map do |fn_node| + get_children_nodes(node).filter_map do |fn_node| computed_field = field_from_node(fn_node) function_adapter = computed_field.function_adapter # : FunctionAdapter::adapter + # An invalid argument (e.g. an out-of-range `percentile`) raises here. Skip building a + # `Computation` for it rather than letting that fail the whole aggregations field--the + # resolver re-runs `extract_args` at resolve time and attributes the error precisely. + begin + function_args = function_adapter.extract_args(computed_field.args_to_schema_form(fn_node.arguments), element_names) + rescue ::GraphQL::ExecutionError + next + end + Aggregation::Computation.new( source_field_path: field_path, leaf: PathSegment.for(field: computed_field, lookahead: fn_node), function_adapter: function_adapter, - function_args: function_adapter.extract_args(computed_field.args_to_schema_form(fn_node.arguments), element_names) + function_args: function_args ) end end diff --git a/elasticgraph-graphql/lib/elastic_graph/graphql/aggregation/resolvers/aggregated_values.rb b/elasticgraph-graphql/lib/elastic_graph/graphql/aggregation/resolvers/aggregated_values.rb index 51d583c9e..36db4a073 100644 --- a/elasticgraph-graphql/lib/elastic_graph/graphql/aggregation/resolvers/aggregated_values.rb +++ b/elasticgraph-graphql/lib/elastic_graph/graphql/aggregation/resolvers/aggregated_values.rb @@ -9,22 +9,47 @@ require "elastic_graph/graphql/aggregation/key" require "elastic_graph/graphql/aggregation/path_segment" require "elastic_graph/support/hash_util" +require "graphql" module ElasticGraph class GraphQL module Aggregation module Resolvers - class AggregatedValues < ::Data.define(:aggregation_name, :bucket, :field_path) + class AggregatedValues < ::Data.define(:schema, :aggregation_name, :bucket, :field_path) def resolve(field:, object:, args:, context:, lookahead:) return with(field_path: field_path + [PathSegment.for(field: field, lookahead: lookahead)]) if field.type.object? + function_adapter = field.function_adapter # : FunctionAdapter::adapter + + # `QueryAdapter` already validated this field's args when the query was built, but it can't + # raise there without failing resolution of the entire aggregations field rather than just + # this one leaf--so it skips building a datastore clause for an invalid field instead. Here, + # at resolve time, we're actually resolving this specific field, so we re-run the same + # validation and can attribute an error to the correct, precise path when it fails. + # + # `args` is already in schema form here (`GraphQLAdapterBuilder` converts it before calling + # `resolve`), unlike at query-build time where `QueryAdapter` converts the raw AST arguments + # itself--so, unlike there, we pass `args` through as-is rather than re-converting it. + begin + function_adapter.extract_args(args, schema.element_names) + rescue ::GraphQL::ExecutionError => e + # Neither `context.add_error` nor `context.execution_errors.add` sets `path` for us--that + # only happens automatically when an `ExecutionError` is raised and returned as a field's + # own resolution result, which isn't the case here since we're continuing on to resolve + # sibling fields normally. So we set `path` ourselves from `context.current_path`, which + # is already the precise path to this field (e.g. `[..., "aggregatedValues", "amountCents", + # "p150"]`), so the error in the response is attributed to this specific field. + e.path ||= context.current_path + context.add_error(e) + return nil + end + key = Key::AggregatedValue.new( aggregation_name: aggregation_name, field_path: field_path.map(&:name_in_graphql_query), function_name: PathSegment.for(field: field, lookahead: lookahead).name_in_graphql_query ) - function_adapter = field.function_adapter # : FunctionAdapter::adapter result = function_adapter.extract_result(Support::HashUtil.verbose_fetch(bucket, key.encode)) # Aggregated value results always have a `value` key; in addition, for `date` field, they also have a `value_as_string`. diff --git a/elasticgraph-graphql/lib/elastic_graph/graphql/aggregation/resolvers/node.rb b/elasticgraph-graphql/lib/elastic_graph/graphql/aggregation/resolvers/node.rb index f36329b0e..e98ca326b 100644 --- a/elasticgraph-graphql/lib/elastic_graph/graphql/aggregation/resolvers/node.rb +++ b/elasticgraph-graphql/lib/elastic_graph/graphql/aggregation/resolvers/node.rb @@ -24,7 +24,7 @@ def grouped_by end def aggregated_values - @aggregated_values ||= AggregatedValues.new(query.name, bucket, field_path) + @aggregated_values ||= AggregatedValues.new(schema, query.name, bucket, field_path) end def sub_aggregations diff --git a/elasticgraph-graphql/sig/elastic_graph/graphql/aggregation/function_adapter.rbs b/elasticgraph-graphql/sig/elastic_graph/graphql/aggregation/function_adapter.rbs index 0c3af1a2a..14e1d8602 100644 --- a/elasticgraph-graphql/sig/elastic_graph/graphql/aggregation/function_adapter.rbs +++ b/elasticgraph-graphql/sig/elastic_graph/graphql/aggregation/function_adapter.rbs @@ -5,7 +5,7 @@ module ElasticGraph # Adapters need not all be data class instances. A function whose behavior is not a # parameterization of anything is registered as a singleton module instead, and joins # this union as a `singleton(...)` member. - type adapter = SimpleMetric + type adapter = SimpleMetric | Percentile type functionArgs = ::Hash[::Symbol, untyped] @@ -31,6 +31,20 @@ module ElasticGraph def empty_bucket_result: () -> ::Hash[::String, untyped] end + class PercentileSupertype < Data + attr_reader datastore_function_name: ::String + + def initialize: (datastore_function_name: ::String) -> void + def self.new: (datastore_function_name: ::String) -> Percentile + end + + class Percentile < PercentileSupertype + def extract_args: (::Hash[::String, untyped], SchemaArtifacts::RuntimeMetadata::SchemaElementNames) -> functionArgs + def clause_options: (functionArgs) -> ::Hash[::String, untyped] + def extract_result: (::Hash[::String, untyped]) -> ::Hash[::String, untyped] + def empty_bucket_result: () -> ::Hash[::String, untyped] + end + BY_NAME: ::Hash[::Symbol, adapter] end end diff --git a/elasticgraph-graphql/sig/elastic_graph/graphql/aggregation/resolvers/aggregated_values.rbs b/elasticgraph-graphql/sig/elastic_graph/graphql/aggregation/resolvers/aggregated_values.rbs index e0323ff66..e66d0a0e9 100644 --- a/elasticgraph-graphql/sig/elastic_graph/graphql/aggregation/resolvers/aggregated_values.rbs +++ b/elasticgraph-graphql/sig/elastic_graph/graphql/aggregation/resolvers/aggregated_values.rbs @@ -3,27 +3,32 @@ module ElasticGraph module Aggregation module Resolvers class AggregatedValuesSupertype + attr_reader schema: Schema attr_reader aggregation_name: ::String attr_reader bucket: ::Hash[::String, untyped] attr_reader field_path: ::Array[PathSegment] def initialize: ( + schema: Schema, aggregation_name: ::String, bucket: ::Hash[::String, untyped], field_path: ::Array[PathSegment] ) -> void def self.new: ( + schema: Schema, aggregation_name: ::String, bucket: ::Hash[::String, untyped], field_path: ::Array[PathSegment] ) -> instance | ( + Schema, ::String, ::Hash[::String, untyped], ::Array[PathSegment] ) -> instance def with: ( + ?schema: Schema, ?aggregation_name: ::String, ?bucket: ::Hash[::String, untyped], ?field_path: ::Array[PathSegment] diff --git a/elasticgraph-graphql/sig/graphql_gem.rbs b/elasticgraph-graphql/sig/graphql_gem.rbs index 4b63287e3..d08e43233 100644 --- a/elasticgraph-graphql/sig/graphql_gem.rbs +++ b/elasticgraph-graphql/sig/graphql_gem.rbs @@ -36,6 +36,7 @@ module GraphQL end class ExecutionError < StandardError + attr_accessor path: ::Array[::String | ::Integer]? end module Language @@ -127,6 +128,7 @@ module GraphQL def fetch: (untyped) -> untyped def add_error: (ExecutionError) -> void def dataloader: () -> Dataloader + def current_path: () -> ::Array[::String | ::Integer]? end class Result diff --git a/elasticgraph-graphql/spec/acceptance/aggregations_spec.rb b/elasticgraph-graphql/spec/acceptance/aggregations_spec.rb index 2429639b3..6026c3df9 100644 --- a/elasticgraph-graphql/spec/acceptance/aggregations_spec.rb +++ b/elasticgraph-graphql/spec/acceptance/aggregations_spec.rb @@ -166,6 +166,55 @@ module ElasticGraph } }) + # Verify that two aliased `approximate_percentile` selections under one field, each requesting a + # different rank, resolve independently rather than colliding on a single datastore aggregation. + aliased_percentiles = widget_ungrouped_aggregated_values_for(<<~QUERY) + #{amount_cents} { + p0: #{case_correctly("approximate_percentile")}(#{case_correctly("percentile")}: 0) + p100: #{case_correctly("approximate_percentile")}(#{case_correctly("percentile")}: 100) + } + QUERY + expect(aliased_percentiles).to eq({ + amount_cents => { + "p0" => 100.0, + "p100" => 300.0 + } + }) + + # Verify that an out-of-range `percentile` resolves to `null` (with a precisely-pathed error) + # rather than failing the entire `aggregated_values` subtree--sibling fields (including another, + # valid `approximate_percentile` alias) still resolve normally. + out_of_range_response = nil + expect { + out_of_range_response = call_graphql_query(<<~QUERY, allow_errors: true) + query { + #{case_correctly("widget_aggregations")} { + nodes { + #{aggregated_values} { + #{amount_cents} { + #{case_correctly("exact_min")} + good: #{case_correctly("approximate_percentile")}(#{case_correctly("percentile")}: 50) + bad: #{case_correctly("approximate_percentile")}(#{case_correctly("percentile")}: 150) + } + } + } + } + } + QUERY + }.to log_warning(a_string_including("percentile` must be between 0 and 100")) + + expect(out_of_range_response.dig("data", case_correctly("widget_aggregations"), "nodes", 0, aggregated_values, amount_cents)).to eq({ + case_correctly("exact_min") => 100, + "good" => 200.0, + "bad" => nil + }) + expect(out_of_range_response.fetch("errors")).to contain_exactly( + hash_including( + "message" => "`#{case_correctly("percentile")}` must be between 0 and 100, but is 150.0.", + "path" => [case_correctly("widget_aggregations"), "nodes", 0, aggregated_values, amount_cents, "bad"] + ) + ) + aggregations = group_widget_currencies_by_widget_name expect(aggregations).to eq [ {"count" => 1, case_correctly("grouped_by") => {case_correctly("widget_name") => "w100"}}, @@ -879,6 +928,10 @@ def expected_aggregated_amounts_of(*raw_values) case_correctly("approximate_sum") => float_of(sum), case_correctly("exact_sum") => int_of(sum), case_correctly("approximate_avg") => float_of(avg), + # `percentile: 0`/`percentile: 100` are equivalent to min/max, so we reuse `float_of` with the + # same expected values to verify `approximate_percentile` without needing separate fixture data. + "p0" => float_of(min), + "p100" => float_of(max), case_correctly("exact_min") => int_of(min), case_correctly("exact_max") => int_of(max) } @@ -1770,6 +1823,8 @@ def amount_aggregation(*fields, **agg_args) approximate_sum exact_sum approximate_avg + p0: approximate_percentile(percentile: 0) + p100: approximate_percentile(percentile: 100) exact_min exact_max } @@ -1779,6 +1834,8 @@ def amount_aggregation(*fields, **agg_args) approximate_sum exact_sum approximate_avg + p0: approximate_percentile(percentile: 0) + p100: approximate_percentile(percentile: 100) exact_min exact_max } @@ -1802,6 +1859,8 @@ def all_amount_aggregations approximate_sum exact_sum approximate_avg + p0: approximate_percentile(percentile: 0) + p100: approximate_percentile(percentile: 100) exact_min exact_max } @@ -1811,6 +1870,8 @@ def all_amount_aggregations approximate_sum exact_sum approximate_avg + p0: approximate_percentile(percentile: 0) + p100: approximate_percentile(percentile: 100) exact_min exact_max } diff --git a/elasticgraph-graphql/spec/acceptance/graphql_types_spec.rb b/elasticgraph-graphql/spec/acceptance/graphql_types_spec.rb index 1d5174407..1d7aa43b3 100644 --- a/elasticgraph-graphql/spec/acceptance/graphql_types_spec.rb +++ b/elasticgraph-graphql/spec/acceptance/graphql_types_spec.rb @@ -203,6 +203,11 @@ module ElasticGraph expect(value_at_path(aggregations.first, aggregated_values, "weight_in_ng_str", "exact_sum")).to be nil expect(value_at_path(aggregations.first, aggregated_values, "weight_in_ng", "approximate_avg")).to be_a(::Float).and be_approximately(weight_in_ngs.sum / weight_in_ngs.size) expect(value_at_path(aggregations.first, aggregated_values, "weight_in_ng_str", "approximate_avg")).to be_a(::Float).and be_approximately(weight_in_ng_strs.sum / weight_in_ng_strs.size) + # `approximate_percentile` returns `Float` even for these integral types, since the percentile + # computation interpolates between adjacent values and can produce a non-integer result. Using + # `percentile: 0` (equivalent to min) lets us assert an exact expected value here. + expect(value_at_path(aggregations.first, aggregated_values, "weight_in_ng", "approximate_percentile")).to be_a(::Float).and be_approximately(weight_in_ngs.min) + expect(value_at_path(aggregations.first, aggregated_values, "weight_in_ng_str", "approximate_percentile")).to be_a(::Float).and be_approximately(weight_in_ng_strs.min) expect(value_at_path(aggregations.first, aggregated_values, "weight_in_ng", "exact_min")).to be_a(::Integer).and eq(weight_in_ngs.min) expect(value_at_path(aggregations.first, aggregated_values, "weight_in_ng_str", "exact_min")).to be_a(::Integer).and eq(weight_in_ng_strs.min) expect(value_at_path(aggregations.first, aggregated_values, "weight_in_ng_str", "approximate_min")).to be_a(::String).and eq(weight_in_ng_strs.min.to_s) @@ -228,6 +233,8 @@ module ElasticGraph expect(value_at_path(aggregations.first, aggregated_values, "weight_in_ng_str", "exact_sum")).to be_a(::Integer).and eq(weight_in_ng_strs.sum) expect(value_at_path(aggregations.first, aggregated_values, "weight_in_ng", "approximate_avg")).to be_a(::Float).and be_approximately(weight_in_ngs.sum / weight_in_ngs.size) expect(value_at_path(aggregations.first, aggregated_values, "weight_in_ng_str", "approximate_avg")).to be_a(::Float).and be_approximately(weight_in_ng_strs.sum / weight_in_ng_strs.size) + expect(value_at_path(aggregations.first, aggregated_values, "weight_in_ng", "approximate_percentile")).to be_a(::Float).and be_approximately(weight_in_ngs.min) + expect(value_at_path(aggregations.first, aggregated_values, "weight_in_ng_str", "approximate_percentile")).to be_a(::Float).and be_approximately(weight_in_ng_strs.min) expect(value_at_path(aggregations.first, aggregated_values, "weight_in_ng", "exact_min")).to be_a(::Integer).and eq(weight_in_ngs.min) expect(value_at_path(aggregations.first, aggregated_values, "weight_in_ng_str", "exact_min")).to be_a(::Integer).and eq(weight_in_ng_strs.min) expect(value_at_path(aggregations.first, aggregated_values, "weight_in_ng_str", "approximate_min")).to be_a(::String).and eq(weight_in_ng_strs.min.to_s) @@ -367,6 +374,7 @@ def all_weight_in_ng_aggregations(**agg_args) approximate_sum exact_sum approximate_avg + approximate_percentile(percentile: 0) exact_min exact_max } @@ -375,6 +383,7 @@ def all_weight_in_ng_aggregations(**agg_args) approximate_sum exact_sum approximate_avg + approximate_percentile(percentile: 0) exact_min approximate_min exact_max diff --git a/elasticgraph-graphql/spec/unit/elastic_graph/graphql/aggregation/function_adapter_spec.rb b/elasticgraph-graphql/spec/unit/elastic_graph/graphql/aggregation/function_adapter_spec.rb index 854057bee..7823d6700 100644 --- a/elasticgraph-graphql/spec/unit/elastic_graph/graphql/aggregation/function_adapter_spec.rb +++ b/elasticgraph-graphql/spec/unit/elastic_graph/graphql/aggregation/function_adapter_spec.rb @@ -15,7 +15,7 @@ module Aggregation RSpec.describe FunctionAdapter do describe "BY_NAME" do it "registers an adapter for each supported aggregated value function" do - expect(FunctionAdapter::BY_NAME.keys).to contain_exactly(:avg, :cardinality, :max, :min, :sum) + expect(FunctionAdapter::BY_NAME.keys).to contain_exactly(:avg, :cardinality, :max, :min, :percentiles, :sum) end it "maps each function to its datastore aggregation name" do @@ -24,6 +24,7 @@ module Aggregation cardinality: "cardinality", max: "max", min: "min", + percentiles: "percentiles", sum: "sum" ) end @@ -34,6 +35,7 @@ module Aggregation cardinality: {"value" => 0}, max: {"value" => nil}, min: {"value" => nil}, + percentiles: {"values" => [{"value" => nil}]}, sum: {"value" => 0} ) end @@ -64,6 +66,48 @@ module Aggregation expect(FunctionAdapter::SimpleMetric.new(datastore_function_name: "sum", empty_bucket_value: 0).empty_bucket_result).to eq({"value" => 0}) end end + + describe FunctionAdapter::Percentile do + let(:element_names) { SchemaArtifacts::RuntimeMetadata::SchemaElementNames.new(form: :snake_case, overrides: {}) } + let(:adapter) { FunctionAdapter::Percentile.new(datastore_function_name: "percentiles") } + + it "exposes the datastore function name it was configured with" do + expect(adapter.datastore_function_name).to eq "percentiles" + end + + it "extracts the requested percentile rank from the args" do + expect(adapter.extract_args({"percentile" => 50.0}, element_names)).to eq({percentile: 50.0}) + end + + it "accepts the boundary values 0 and 100" do + expect(adapter.extract_args({"percentile" => 0}, element_names)).to eq({percentile: 0}) + expect(adapter.extract_args({"percentile" => 100}, element_names)).to eq({percentile: 100}) + end + + it "raises a GraphQL::ExecutionError when the requested percentile is below 0" do + expect { + adapter.extract_args({"percentile" => -1}, element_names) + }.to raise_error(::GraphQL::ExecutionError, "`percentile` must be between 0 and 100, but is -1.") + end + + it "raises a GraphQL::ExecutionError when the requested percentile is above 100" do + expect { + adapter.extract_args({"percentile" => 150}, element_names) + }.to raise_error(::GraphQL::ExecutionError, "`percentile` must be between 0 and 100, but is 150.") + end + + it "requests a single unkeyed percentile in the clause options" do + expect(adapter.clause_options({percentile: 99.9})).to eq({"percents" => [99.9], "keyed" => false}) + end + + it "extracts the single value from the datastore's array-shaped response" do + expect(adapter.extract_result({"values" => [{"key" => 50.0, "value" => 3.7}]})).to eq({"key" => 50.0, "value" => 3.7}) + end + + it "fabricates an empty bucket response with a nil value" do + expect(adapter.empty_bucket_result).to eq({"values" => [{"value" => nil}]}) + end + end end end end diff --git a/elasticgraph-graphql/spec/unit/elastic_graph/graphql/aggregation/resolvers/aggregation_resolver_support.rb b/elasticgraph-graphql/spec/unit/elastic_graph/graphql/aggregation/resolvers/aggregation_resolver_support.rb index d514e59d4..f2a735222 100644 --- a/elasticgraph-graphql/spec/unit/elastic_graph/graphql/aggregation/resolvers/aggregation_resolver_support.rb +++ b/elasticgraph-graphql/spec/unit/elastic_graph/graphql/aggregation/resolvers/aggregation_resolver_support.rb @@ -22,11 +22,13 @@ def resolve_target_nodes( target_buckets: [], hit_count: nil, aggs: {"target" => {"buckets" => target_buckets}}, - path: ["data", "target", "nodes"] + path: ["data", "target", "nodes"], + allow_errors: false ) allow(datastore_client).to receive(:msearch).and_return({"responses" => [datastore_response_payload_with_aggs(aggs, hit_count)]}) response = graphql.graphql_query_executor.execute("query { #{inner_query} }") + return response if allow_errors expect(response["errors"]).to eq([]).or eq(nil) response.dig(*path) end diff --git a/elasticgraph-graphql/spec/unit/elastic_graph/graphql/aggregation/resolvers/aggregations_spec.rb b/elasticgraph-graphql/spec/unit/elastic_graph/graphql/aggregation/resolvers/aggregations_spec.rb index 59040b0b4..b7efd4605 100644 --- a/elasticgraph-graphql/spec/unit/elastic_graph/graphql/aggregation/resolvers/aggregations_spec.rb +++ b/elasticgraph-graphql/spec/unit/elastic_graph/graphql/aggregation/resolvers/aggregations_spec.rb @@ -412,6 +412,46 @@ module Aggregation ] end + it "resolves sibling aggregated value fields normally and returns `null` (with a precisely-pathed error) for an out-of-range `approximate_percentile`, rather than failing the whole query" do + aggs = { + aggregated_value_key_of("amount_cents", "exact_sum") => {"value" => 900.0}, + aggregated_value_key_of("amount_cents", "good") => {"values" => [{"value" => 500.0}]} + } + + response = resolve_target_nodes(<<~QUERY, aggs: aggs, allow_errors: true) + target: widget_aggregations { + nodes { + aggregated_values { + amount_cents { + exact_sum + good: approximate_percentile(percentile: 50) + bad: approximate_percentile(percentile: 150) + } + } + } + } + QUERY + + expect(response.dig("data", "target", "nodes")).to eq [ + { + "aggregated_values" => { + "amount_cents" => { + "exact_sum" => 900, + "good" => 500, + "bad" => nil + } + } + } + ] + + expect(response.fetch("errors")).to contain_exactly( + hash_including( + "message" => "`percentile` must be between 0 and 100, but is 150.0.", + "path" => ["target", "nodes", 0, "aggregated_values", "amount_cents", "bad"] + ) + ) + end + def aggregated_value_key_of(*field_path, function_name, aggregation_name: "target") super(*field_path, function_name, aggregation_name: "target").encode end diff --git a/elasticgraph-schema_artifacts/lib/elastic_graph/schema_artifacts/runtime_metadata/schema_element_names.rb b/elasticgraph-schema_artifacts/lib/elastic_graph/schema_artifacts/runtime_metadata/schema_element_names.rb index 5fd8d7693..2d0d6180f 100644 --- a/elasticgraph-schema_artifacts/lib/elastic_graph/schema_artifacts/runtime_metadata/schema_element_names.rb +++ b/elasticgraph-schema_artifacts/lib/elastic_graph/schema_artifacts/runtime_metadata/schema_element_names.rb @@ -151,7 +151,7 @@ def validate_no_name_collisions(canonical_name_by_exposed_name, exposed_name_by_ :query, :phrase, :query_with_prefix, :allowed_edits_per_term, :require_all_terms, # Aggregated values field names: :exact_min, :exact_max, :approximate_min, :approximate_max, :approximate_avg, :approximate_sum, :exact_sum, - :approximate_distinct_value_count + :approximate_distinct_value_count, :approximate_percentile, :percentile ) end end diff --git a/elasticgraph-schema_artifacts/sig/elastic_graph/schema_artifacts/runtime_metadata/schema_element_names.rbs b/elasticgraph-schema_artifacts/sig/elastic_graph/schema_artifacts/runtime_metadata/schema_element_names.rbs index 253a8e2ee..475547613 100644 --- a/elasticgraph-schema_artifacts/sig/elastic_graph/schema_artifacts/runtime_metadata/schema_element_names.rbs +++ b/elasticgraph-schema_artifacts/sig/elastic_graph/schema_artifacts/runtime_metadata/schema_element_names.rbs @@ -96,6 +96,8 @@ module ElasticGraph attr_reader approximate_sum: ::String attr_reader exact_sum: ::String attr_reader approximate_distinct_value_count: ::String + attr_reader approximate_percentile: ::String + attr_reader percentile: ::String def normalize_case: (::String) -> ::String def self.from_hash: (::Hash[::String, untyped]) -> SchemaElementNames diff --git a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/built_in_types.rb b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/built_in_types.rb index adfe0106f..924203c62 100644 --- a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/built_in_types.rb +++ b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/built_in_types.rb @@ -711,6 +711,8 @@ def register_standard_graphql_scalars outside the `JsonSafeLong` range (#{format_number(JSON_SAFE_LONG_MIN)} to #{format_number(JSON_SAFE_LONG_MAX)}). EOS end + + define_approximate_percentile_on_aggregated_values(avt, "Float") end end @@ -791,7 +793,7 @@ def register_custom_elastic_graph_scalars EOS t.customize_aggregated_values_type do |avt| - define_exact_min_max_and_approx_avg_on_aggregated_values(avt, "Date") do |adjective:, full_name:| + define_temporal_aggregated_values(avt, "Date") do |adjective:, full_name:| <<~EOS So long as the grouping contains at least one non-null value for the underlying indexed field, this will return an exact non-null value. @@ -827,7 +829,7 @@ def register_custom_elastic_graph_scalars end t.customize_aggregated_values_type do |avt| - define_exact_min_max_and_approx_avg_on_aggregated_values(avt, "DateTime") do |adjective:, full_name:| + define_temporal_aggregated_values(avt, "DateTime") do |adjective:, full_name:| <<~EOS So long as the grouping contains at least one non-null value for the underlying indexed field, this will return an exact non-null value. @@ -893,7 +895,7 @@ def register_custom_elastic_graph_scalars t.mapping type: "date", format: "HH:mm:ss||HH:mm:ss.S||HH:mm:ss.SS||HH:mm:ss.SSS" t.customize_aggregated_values_type do |avt| - define_exact_min_max_and_approx_avg_on_aggregated_values(avt, "LocalTime") do |adjective:, full_name:| + define_temporal_aggregated_values(avt, "LocalTime") do |adjective:, full_name:| <<~EOS So long as the grouping contains at least one non-null value for the underlying indexed field, this will return an exact non-null value. @@ -1070,6 +1072,8 @@ def register_custom_elastic_graph_scalars to #{format_number(JSON_SAFE_LONG_MAX)}). EOS end + + define_approximate_percentile_on_aggregated_values(avt, "Float") end end end @@ -1573,10 +1577,12 @@ def define_integral_aggregated_values_for(scalar_type, long_type: "JsonSafeLong" to #{format_number(JSON_SAFE_LONG_MAX)}). EOS end + + define_approximate_percentile_on_aggregated_values(t, "Float") end end - def define_exact_min_max_and_approx_avg_on_aggregated_values(aggregated_values_type, scalar_type, &block) + def define_temporal_aggregated_values(aggregated_values_type, scalar_type, &block) define_exact_min_and_max_on_aggregated_values(aggregated_values_type, scalar_type, &block) aggregated_values_type.field names.approximate_avg, scalar_type, graphql_only: true do |f| @@ -1587,6 +1593,27 @@ def define_exact_min_max_and_approx_avg_on_aggregated_values(aggregated_values_t The returned value will be rounded to the nearest `#{scalar_type}` value. EOS end + + define_approximate_percentile_on_aggregated_values(aggregated_values_type, scalar_type) + end + + def define_approximate_percentile_on_aggregated_values(aggregated_values_type, scalar_type) + aggregated_values_type.field names.approximate_percentile, scalar_type, graphql_only: true do |f| + f.computes :percentiles + + f.argument names.percentile, "Float!" do |a| + a.documentation "The percentile rank to compute, from `0` to `100` (e.g. `0` for the min, `100` for the max, `50` for the median, `99` for the 99th percentile)." + end + + f.documentation <<~EOS + An approximate percentile of the field values within this grouping. + + Percentiles are computed using an approximate algorithm, so the returned value may not be + exact. Request a specific percentile via the `#{names.percentile}` argument (e.g. + `#{names.percentile}: 50` for the median). To request multiple percentiles in a single query, + use a GraphQL alias for each: `p50: #{names.approximate_percentile}(#{names.percentile}: 50)`. + EOS + end end def define_exact_min_and_max_on_aggregated_values(aggregated_values_type, scalar_type) diff --git a/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/graphql_schema/built_in_types_spec.rb b/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/graphql_schema/built_in_types_spec.rb index 65162ca5a..d931350fd 100644 --- a/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/graphql_schema/built_in_types_spec.rb +++ b/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/graphql_schema/built_in_types_spec.rb @@ -1134,6 +1134,19 @@ def expect_list_filter(scalar, fields_description: "`[#{scalar}]` fields") outside the `JsonSafeLong` range (-9,007,199,254,740,991 to 9,007,199,254,740,991). """ #{schema_elements.approximate_avg}: Float + """ + An approximate percentile of the field values within this grouping. + + Percentiles are computed using an approximate algorithm, so the returned value may not be + exact. Request a specific percentile via the `#{schema_elements.percentile}` argument (e.g. + `#{schema_elements.percentile}: 50` for the median). To request multiple percentiles in a single query, + use a GraphQL alias for each: `p50: #{schema_elements.approximate_percentile}(#{schema_elements.percentile}: 50)`. + """ + #{schema_elements.approximate_percentile}( + """ + The percentile rank to compute, from `0` to `100` (e.g. `0` for the min, `100` for the max, `50` for the median, `99` for the 99th percentile). + """ + #{schema_elements.percentile}: Float!): Float } EOS end @@ -1173,6 +1186,19 @@ def expect_list_filter(scalar, fields_description: "`[#{scalar}]` fields") The returned value will be rounded to the nearest `#{scalar_type}` value. """ #{schema_elements.approximate_avg}: #{scalar_type} + """ + An approximate percentile of the field values within this grouping. + + Percentiles are computed using an approximate algorithm, so the returned value may not be + exact. Request a specific percentile via the `#{schema_elements.percentile}` argument (e.g. + `#{schema_elements.percentile}: 50` for the median). To request multiple percentiles in a single query, + use a GraphQL alias for each: `p50: #{schema_elements.approximate_percentile}(#{schema_elements.percentile}: 50)`. + """ + #{schema_elements.approximate_percentile}( + """ + The percentile rank to compute, from `0` to `100` (e.g. `0` for the min, `100` for the max, `50` for the median, `99` for the 99th percentile). + """ + #{schema_elements.percentile}: Float!): #{scalar_type} } EOS end @@ -1290,6 +1316,19 @@ def expect_list_filter(scalar, fields_description: "`[#{scalar}]` fields") to 9,007,199,254,740,991). """ #{schema_elements.approximate_avg}: Float + """ + An approximate percentile of the field values within this grouping. + + Percentiles are computed using an approximate algorithm, so the returned value may not be + exact. Request a specific percentile via the `#{schema_elements.percentile}` argument (e.g. + `#{schema_elements.percentile}: 50` for the median). To request multiple percentiles in a single query, + use a GraphQL alias for each: `p50: #{schema_elements.approximate_percentile}(#{schema_elements.percentile}: 50)`. + """ + #{schema_elements.approximate_percentile}( + """ + The percentile rank to compute, from `0` to `100` (e.g. `0` for the min, `100` for the max, `50` for the median, `99` for the 99th percentile). + """ + #{schema_elements.percentile}: Float!): Float } EOS end @@ -1377,6 +1416,19 @@ def expect_list_filter(scalar, fields_description: "`[#{scalar}]` fields") to 9,007,199,254,740,991). """ #{schema_elements.approximate_avg}: Float + """ + An approximate percentile of the field values within this grouping. + + Percentiles are computed using an approximate algorithm, so the returned value may not be + exact. Request a specific percentile via the `#{schema_elements.percentile}` argument (e.g. + `#{schema_elements.percentile}: 50` for the median). To request multiple percentiles in a single query, + use a GraphQL alias for each: `p50: #{schema_elements.approximate_percentile}(#{schema_elements.percentile}: 50)`. + """ + #{schema_elements.approximate_percentile}( + """ + The percentile rank to compute, from `0` to `100` (e.g. `0` for the min, `100` for the max, `50` for the median, `99` for the 99th percentile). + """ + #{schema_elements.percentile}: Float!): Float } EOS end diff --git a/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/object_types_by_name/for_built_in_types_spec.rb b/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/object_types_by_name/for_built_in_types_spec.rb index 3dcf40f26..59b53cdeb 100644 --- a/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/object_types_by_name/for_built_in_types_spec.rb +++ b/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/object_types_by_name/for_built_in_types_spec.rb @@ -30,7 +30,8 @@ module SchemaDefinition "exact_sum" => :sum, "exact_max" => :max, "exact_min" => :min, - "approximate_distinct_value_count" => :cardinality + "approximate_distinct_value_count" => :cardinality, + "approximate_percentile" => :percentiles ) end @@ -49,7 +50,8 @@ module SchemaDefinition "approximate_sum" => :sum, "exact_max" => :max, "exact_min" => :min, - "approximate_distinct_value_count" => :cardinality + "approximate_distinct_value_count" => :cardinality, + "approximate_percentile" => :percentiles ) end @@ -69,7 +71,8 @@ module SchemaDefinition "exact_sum" => :sum, "exact_max" => :max, "exact_min" => :min, - "approximate_distinct_value_count" => :cardinality + "approximate_distinct_value_count" => :cardinality, + "approximate_percentile" => :percentiles ) end @@ -91,7 +94,8 @@ module SchemaDefinition "exact_max" => :max, "approximate_min" => :min, "exact_min" => :min, - "approximate_distinct_value_count" => :cardinality + "approximate_distinct_value_count" => :cardinality, + "approximate_percentile" => :percentiles ) end end From 056741e64b943659c6dbb88fdb823852199a8bd1 Mon Sep 17 00:00:00 2001 From: nikhilkumarjadhav-toast Date: Thu, 6 Aug 2026 15:08:57 -0700 Subject: [PATCH 2/7] Apply the 0=min/100=max clarification to the guide doc too The percentile arg doc in built_in_types.rb already spelled this out; the guide's prose was missed when applying the same suggestion there. --- config/site/src/query-api/aggregations/aggregated-values.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/config/site/src/query-api/aggregations/aggregated-values.md b/config/site/src/query-api/aggregations/aggregated-values.md index 1896b8032..8e5b978f7 100644 --- a/config/site/src/query-api/aggregations/aggregated-values.md +++ b/config/site/src/query-api/aggregations/aggregated-values.md @@ -33,7 +33,8 @@ both in a single query: `approximatePercentile` : An approximate percentile of the field values within this grouping. The `percentile` argument specifies - the desired percentile rank, from `0` to `100` (e.g. `50` for the median, `90` for the 90th percentile). + the desired percentile rank, from `0` to `100` (e.g. `0` for the min, `100` for the max, `50` for the + median, `90` for the 90th percentile). Percentiles are computed using an approximate algorithm, so the returned value may not be exact--this is true regardless of the field's type, so there is no `exactPercentile` counterpart the way there is for From 8e4b0b5ad6d6d960a6ce052592bce9a4c4171597 Mon Sep 17 00:00:00 2001 From: Nikhilkumar Jadhav Date: Fri, 7 Aug 2026 11:13:04 -0600 Subject: [PATCH 3/7] Update config/site/src/query-api/aggregations/aggregated-values.md Co-authored-by: Myron Marston --- config/site/src/query-api/aggregations/aggregated-values.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/site/src/query-api/aggregations/aggregated-values.md b/config/site/src/query-api/aggregations/aggregated-values.md index 8e5b978f7..6239091d9 100644 --- a/config/site/src/query-api/aggregations/aggregated-values.md +++ b/config/site/src/query-api/aggregations/aggregated-values.md @@ -33,7 +33,7 @@ both in a single query: `approximatePercentile` : An approximate percentile of the field values within this grouping. The `percentile` argument specifies - the desired percentile rank, from `0` to `100` (e.g. `0` for the min, `100` for the max, `50` for the + the desired percentile, from `0` to `100` (e.g. `0` for the min, `100` for the max, `50` for the median, `90` for the 90th percentile). Percentiles are computed using an approximate algorithm, so the returned value may not be exact--this is From ba5e9ede4c50e7901e98efdf24e0716ca352d09d Mon Sep 17 00:00:00 2001 From: Nikhilkumar Jadhav Date: Fri, 7 Aug 2026 11:13:12 -0600 Subject: [PATCH 4/7] Update elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/built_in_types.rb Co-authored-by: Myron Marston --- .../schema_definition/schema_elements/built_in_types.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/built_in_types.rb b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/built_in_types.rb index 924203c62..878352930 100644 --- a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/built_in_types.rb +++ b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/built_in_types.rb @@ -1602,7 +1602,7 @@ def define_approximate_percentile_on_aggregated_values(aggregated_values_type, s f.computes :percentiles f.argument names.percentile, "Float!" do |a| - a.documentation "The percentile rank to compute, from `0` to `100` (e.g. `0` for the min, `100` for the max, `50` for the median, `99` for the 99th percentile)." + a.documentation "The percentile to compute, from `0` to `100` (e.g. `0` for the min, `100` for the max, `50` for the median, `99` for the 99th percentile)." end f.documentation <<~EOS From 86e20b1ad35823755f60b4b63002d07ae5f958ef Mon Sep 17 00:00:00 2001 From: Nikhilkumar Jadhav Date: Fri, 7 Aug 2026 11:13:19 -0600 Subject: [PATCH 5/7] Update elasticgraph-graphql/lib/elastic_graph/graphql/aggregation/function_adapter.rb Co-authored-by: Myron Marston --- .../lib/elastic_graph/graphql/aggregation/function_adapter.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/elasticgraph-graphql/lib/elastic_graph/graphql/aggregation/function_adapter.rb b/elasticgraph-graphql/lib/elastic_graph/graphql/aggregation/function_adapter.rb index 1bb124a07..957f60b06 100644 --- a/elasticgraph-graphql/lib/elastic_graph/graphql/aggregation/function_adapter.rb +++ b/elasticgraph-graphql/lib/elastic_graph/graphql/aggregation/function_adapter.rb @@ -82,7 +82,7 @@ def empty_bucket_result cardinality: SimpleMetric.new(datastore_function_name: "cardinality", empty_bucket_value: 0), max: SimpleMetric.new(datastore_function_name: "max", empty_bucket_value: nil), min: SimpleMetric.new(datastore_function_name: "min", empty_bucket_value: nil), - percentiles: Percentile.new(datastore_function_name: "percentiles"), + percentile: Percentile.new(datastore_function_name: "percentiles"), sum: SimpleMetric.new(datastore_function_name: "sum", empty_bucket_value: 0) }.freeze end From a87be68b69543bb7b90b21aacaeab05f7ed5bded Mon Sep 17 00:00:00 2001 From: nikhilkumarjadhav-toast Date: Fri, 7 Aug 2026 10:40:33 -0700 Subject: [PATCH 6/7] Apply Myron's review feedback: block-based validation, percentile rename, stronger test - Replace exception-based control flow in FunctionAdapter#extract_args with a block-yielded error message, propagated through QueryAdapter and the resolver. Avoids relying on exceptions for control flow, documents the validation contract explicitly, and skips the cost of capturing a stack trace for something that isn't actually exceptional. - Rename the BY_NAME registry key and computes tag from :percentiles to :percentile (singular, matching every other function), updating all specs and regenerating schema artifacts. - Rename resolve_target_nodes's allow_errors param to expect_errors and assert errors are actually present when set, so it can't silently pass on a test that has no errors. - Strengthen expected_aggregated_amounts_of with verified p50/p75 assertions (exact values confirmed against a real OpenSearch percentiles aggregation) so the acceptance suite can't be satisfied by a min/max-only implementation. --- config/schema/artifacts/runtime_metadata.yaml | 14 +++---- config/schema/artifacts/schema.graphql | 28 ++++++------- .../runtime_metadata.yaml | 14 +++---- .../artifacts_with_apollo/schema.graphql | 28 ++++++------- .../graphql/aggregation/function_adapter.rb | 10 +++-- .../graphql/aggregation/query_adapter.rb | 39 +++++++++---------- .../resolvers/aggregated_values.rb | 20 +++++----- .../graphql/aggregation/function_adapter.rbs | 4 +- .../graphql/aggregation/query_adapter.rbs | 5 +++ .../spec/acceptance/aggregations_spec.rb | 33 ++++++++++++++++ .../aggregation/function_adapter_spec.rb | 35 +++++++++-------- .../resolvers/aggregation_resolver_support.rb | 12 ++++-- .../resolvers/aggregations_spec.rb | 2 +- .../schema_elements/built_in_types.rb | 2 +- .../graphql_schema/built_in_types_spec.rb | 8 ++-- .../for_built_in_types_spec.rb | 8 ++-- 16 files changed, 155 insertions(+), 107 deletions(-) diff --git a/config/schema/artifacts/runtime_metadata.yaml b/config/schema/artifacts/runtime_metadata.yaml index 3c04f2480..49bb6e083 100644 --- a/config/schema/artifacts/runtime_metadata.yaml +++ b/config/schema/artifacts/runtime_metadata.yaml @@ -4034,7 +4034,7 @@ object_types_by_name: resolver: name: object_with_lookahead approximate_percentile: - computation_function: percentiles + computation_function: percentile resolver: name: object_with_lookahead exact_max: @@ -4072,7 +4072,7 @@ object_types_by_name: resolver: name: object_with_lookahead approximate_percentile: - computation_function: percentiles + computation_function: percentile resolver: name: object_with_lookahead exact_max: @@ -4422,7 +4422,7 @@ object_types_by_name: resolver: name: object_with_lookahead approximate_percentile: - computation_function: percentiles + computation_function: percentile resolver: name: object_with_lookahead approximate_sum: @@ -4686,7 +4686,7 @@ object_types_by_name: resolver: name: object_with_lookahead approximate_percentile: - computation_function: percentiles + computation_function: percentile resolver: name: object_with_lookahead approximate_sum: @@ -4843,7 +4843,7 @@ object_types_by_name: resolver: name: object_with_lookahead approximate_percentile: - computation_function: percentiles + computation_function: percentile resolver: name: object_with_lookahead approximate_sum: @@ -4879,7 +4879,7 @@ object_types_by_name: resolver: name: object_with_lookahead approximate_percentile: - computation_function: percentiles + computation_function: percentile resolver: name: object_with_lookahead exact_max: @@ -4911,7 +4911,7 @@ object_types_by_name: resolver: name: object_with_lookahead approximate_percentile: - computation_function: percentiles + computation_function: percentile resolver: name: object_with_lookahead approximate_sum: diff --git a/config/schema/artifacts/schema.graphql b/config/schema/artifacts/schema.graphql index 2797a6c9f..2573b21ea 100644 --- a/config/schema/artifacts/schema.graphql +++ b/config/schema/artifacts/schema.graphql @@ -2179,8 +2179,8 @@ type DateAggregatedValues { """ approximate_percentile( """ - The percentile rank to compute, from `0` to `100` (e.g. `0` for the min, - `100` for the max, `50` for the median, `99` for the 99th percentile). + The percentile to compute, from `0` to `100` (e.g. `0` for the min, `100` + for the max, `50` for the median, `99` for the 99th percentile). """ percentile: Float! ): Date @@ -2512,8 +2512,8 @@ type DateTimeAggregatedValues { """ approximate_percentile( """ - The percentile rank to compute, from `0` to `100` (e.g. `0` for the min, - `100` for the max, `50` for the median, `99` for the 99th percentile). + The percentile to compute, from `0` to `100` (e.g. `0` for the min, `100` + for the max, `50` for the median, `99` for the 99th percentile). """ percentile: Float! ): DateTime @@ -3820,8 +3820,8 @@ type FloatAggregatedValues { """ approximate_percentile( """ - The percentile rank to compute, from `0` to `100` (e.g. `0` for the min, - `100` for the max, `50` for the median, `99` for the 99th percentile). + The percentile to compute, from `0` to `100` (e.g. `0` for the min, `100` + for the max, `50` for the median, `99` for the 99th percentile). """ percentile: Float! ): Float @@ -4635,8 +4635,8 @@ type IntAggregatedValues { """ approximate_percentile( """ - The percentile rank to compute, from `0` to `100` (e.g. `0` for the min, - `100` for the max, `50` for the median, `99` for the 99th percentile). + The percentile to compute, from `0` to `100` (e.g. `0` for the min, `100` + for the max, `50` for the median, `99` for the 99th percentile). """ percentile: Float! ): Float @@ -5236,8 +5236,8 @@ type JsonSafeLongAggregatedValues { """ approximate_percentile( """ - The percentile rank to compute, from `0` to `100` (e.g. `0` for the min, - `100` for the max, `50` for the median, `99` for the 99th percentile). + The percentile to compute, from `0` to `100` (e.g. `0` for the min, `100` + for the max, `50` for the median, `99` for the 99th percentile). """ percentile: Float! ): Float @@ -5503,8 +5503,8 @@ type LocalTimeAggregatedValues { """ approximate_percentile( """ - The percentile rank to compute, from `0` to `100` (e.g. `0` for the min, - `100` for the max, `50` for the median, `99` for the 99th percentile). + The percentile to compute, from `0` to `100` (e.g. `0` for the min, `100` + for the max, `50` for the median, `99` for the 99th percentile). """ percentile: Float! ): LocalTime @@ -5729,8 +5729,8 @@ type LongStringAggregatedValues { """ approximate_percentile( """ - The percentile rank to compute, from `0` to `100` (e.g. `0` for the min, - `100` for the max, `50` for the median, `99` for the 99th percentile). + The percentile to compute, from `0` to `100` (e.g. `0` for the min, `100` + for the max, `50` for the median, `99` for the 99th percentile). """ percentile: Float! ): Float diff --git a/config/schema/artifacts_with_apollo/runtime_metadata.yaml b/config/schema/artifacts_with_apollo/runtime_metadata.yaml index 0763127a2..648ec6964 100644 --- a/config/schema/artifacts_with_apollo/runtime_metadata.yaml +++ b/config/schema/artifacts_with_apollo/runtime_metadata.yaml @@ -4136,7 +4136,7 @@ object_types_by_name: resolver: name: object_with_lookahead approximate_percentile: - computation_function: percentiles + computation_function: percentile resolver: name: object_with_lookahead exact_max: @@ -4174,7 +4174,7 @@ object_types_by_name: resolver: name: object_with_lookahead approximate_percentile: - computation_function: percentiles + computation_function: percentile resolver: name: object_with_lookahead exact_max: @@ -4524,7 +4524,7 @@ object_types_by_name: resolver: name: object_with_lookahead approximate_percentile: - computation_function: percentiles + computation_function: percentile resolver: name: object_with_lookahead approximate_sum: @@ -4788,7 +4788,7 @@ object_types_by_name: resolver: name: object_with_lookahead approximate_percentile: - computation_function: percentiles + computation_function: percentile resolver: name: object_with_lookahead approximate_sum: @@ -4945,7 +4945,7 @@ object_types_by_name: resolver: name: object_with_lookahead approximate_percentile: - computation_function: percentiles + computation_function: percentile resolver: name: object_with_lookahead approximate_sum: @@ -4981,7 +4981,7 @@ object_types_by_name: resolver: name: object_with_lookahead approximate_percentile: - computation_function: percentiles + computation_function: percentile resolver: name: object_with_lookahead exact_max: @@ -5013,7 +5013,7 @@ object_types_by_name: resolver: name: object_with_lookahead approximate_percentile: - computation_function: percentiles + computation_function: percentile resolver: name: object_with_lookahead approximate_sum: diff --git a/config/schema/artifacts_with_apollo/schema.graphql b/config/schema/artifacts_with_apollo/schema.graphql index c6308e642..9705331f0 100644 --- a/config/schema/artifacts_with_apollo/schema.graphql +++ b/config/schema/artifacts_with_apollo/schema.graphql @@ -2446,8 +2446,8 @@ type DateAggregatedValues @shareable { """ approximate_percentile( """ - The percentile rank to compute, from `0` to `100` (e.g. `0` for the min, - `100` for the max, `50` for the median, `99` for the 99th percentile). + The percentile to compute, from `0` to `100` (e.g. `0` for the min, `100` + for the max, `50` for the median, `99` for the 99th percentile). """ percentile: Float! ): Date @@ -2779,8 +2779,8 @@ type DateTimeAggregatedValues @shareable { """ approximate_percentile( """ - The percentile rank to compute, from `0` to `100` (e.g. `0` for the min, - `100` for the max, `50` for the median, `99` for the 99th percentile). + The percentile to compute, from `0` to `100` (e.g. `0` for the min, `100` + for the max, `50` for the median, `99` for the 99th percentile). """ percentile: Float! ): DateTime @@ -4102,8 +4102,8 @@ type FloatAggregatedValues @shareable { """ approximate_percentile( """ - The percentile rank to compute, from `0` to `100` (e.g. `0` for the min, - `100` for the max, `50` for the median, `99` for the 99th percentile). + The percentile to compute, from `0` to `100` (e.g. `0` for the min, `100` + for the max, `50` for the median, `99` for the 99th percentile). """ percentile: Float! ): Float @@ -4917,8 +4917,8 @@ type IntAggregatedValues @shareable { """ approximate_percentile( """ - The percentile rank to compute, from `0` to `100` (e.g. `0` for the min, - `100` for the max, `50` for the median, `99` for the 99th percentile). + The percentile to compute, from `0` to `100` (e.g. `0` for the min, `100` + for the max, `50` for the median, `99` for the 99th percentile). """ percentile: Float! ): Float @@ -5518,8 +5518,8 @@ type JsonSafeLongAggregatedValues @shareable { """ approximate_percentile( """ - The percentile rank to compute, from `0` to `100` (e.g. `0` for the min, - `100` for the max, `50` for the median, `99` for the 99th percentile). + The percentile to compute, from `0` to `100` (e.g. `0` for the min, `100` + for the max, `50` for the median, `99` for the 99th percentile). """ percentile: Float! ): Float @@ -5785,8 +5785,8 @@ type LocalTimeAggregatedValues @shareable { """ approximate_percentile( """ - The percentile rank to compute, from `0` to `100` (e.g. `0` for the min, - `100` for the max, `50` for the median, `99` for the 99th percentile). + The percentile to compute, from `0` to `100` (e.g. `0` for the min, `100` + for the max, `50` for the median, `99` for the 99th percentile). """ percentile: Float! ): LocalTime @@ -6011,8 +6011,8 @@ type LongStringAggregatedValues @shareable { """ approximate_percentile( """ - The percentile rank to compute, from `0` to `100` (e.g. `0` for the min, - `100` for the max, `50` for the median, `99` for the 99th percentile). + The percentile to compute, from `0` to `100` (e.g. `0` for the min, `100` + for the max, `50` for the median, `99` for the 99th percentile). """ percentile: Float! ): Float diff --git a/elasticgraph-graphql/lib/elastic_graph/graphql/aggregation/function_adapter.rb b/elasticgraph-graphql/lib/elastic_graph/graphql/aggregation/function_adapter.rb index 957f60b06..c2e3d545b 100644 --- a/elasticgraph-graphql/lib/elastic_graph/graphql/aggregation/function_adapter.rb +++ b/elasticgraph-graphql/lib/elastic_graph/graphql/aggregation/function_adapter.rb @@ -6,8 +6,6 @@ # # frozen_string_literal: true -require "graphql" - module ElasticGraph class GraphQL module Aggregation @@ -17,6 +15,11 @@ module Aggregation # locate its value in the datastore response, and what response to fabricate for a bucket # the datastore omitted. # + # When an adapter's `extract_args` is given args it considers invalid, it yields an error + # message instead of returning extracted args. Each caller passes a block that exits + # non-locally (the args of an invalid field are never used), which lets a field with invalid + # args be handled without disrupting the sibling fields being processed alongside it. + # # @private module FunctionAdapter # Adapter for metric aggregations that take no arguments and return a flat @@ -24,6 +27,7 @@ module FunctionAdapter # # @private class SimpleMetric < ::Data.define(:datastore_function_name, :empty_bucket_value) + # These functions take no args, so there's nothing to extract and nothing that can be invalid. def extract_args(args, element_names) {} end @@ -51,7 +55,7 @@ def extract_args(args, element_names) percentile = args.fetch(element_names.percentile) unless percentile.is_a?(::Numeric) && percentile >= 0 && percentile <= 100 - raise ::GraphQL::ExecutionError, "`#{element_names.percentile}` must be between 0 and 100, but is #{percentile.inspect}." + return yield "`#{element_names.percentile}` must be between 0 and 100, but is #{percentile.inspect}." end {percentile: percentile} diff --git a/elasticgraph-graphql/lib/elastic_graph/graphql/aggregation/query_adapter.rb b/elasticgraph-graphql/lib/elastic_graph/graphql/aggregation/query_adapter.rb index 4a3805850..531d89820 100644 --- a/elasticgraph-graphql/lib/elastic_graph/graphql/aggregation/query_adapter.rb +++ b/elasticgraph-graphql/lib/elastic_graph/graphql/aggregation/query_adapter.rb @@ -186,31 +186,28 @@ def build_computations_from(node_node, from_field_path: []) build_clauses_from(aggregated_values_node) do |node, field, field_path| if field.aggregated? field_path = from_field_path + field_path - - get_children_nodes(node).filter_map do |fn_node| - computed_field = field_from_node(fn_node) - function_adapter = computed_field.function_adapter # : FunctionAdapter::adapter - - # An invalid argument (e.g. an out-of-range `percentile`) raises here. Skip building a - # `Computation` for it rather than letting that fail the whole aggregations field--the - # resolver re-runs `extract_args` at resolve time and attributes the error precisely. - begin - function_args = function_adapter.extract_args(computed_field.args_to_schema_form(fn_node.arguments), element_names) - rescue ::GraphQL::ExecutionError - next - end - - Aggregation::Computation.new( - source_field_path: field_path, - leaf: PathSegment.for(field: computed_field, lookahead: fn_node), - function_adapter: function_adapter, - function_args: function_args - ) - end + get_children_nodes(node).filter_map { |fn_node| computation_for(fn_node, field_path) } end end end + # Builds the `Computation` for an aggregated value function node, or returns `nil` if the node + # has invalid args (e.g. an out-of-range `percentile`). We omit the computation rather than + # failing the whole aggregations field here: the resolver detects the same invalid args when it + # resolves this specific field, and can attribute the error to that field's precise path. + def computation_for(fn_node, field_path) + computed_field = field_from_node(fn_node) + function_adapter = computed_field.function_adapter # : FunctionAdapter::adapter + args = computed_field.args_to_schema_form(fn_node.arguments) + + Aggregation::Computation.new( + source_field_path: field_path, + leaf: PathSegment.for(field: computed_field, lookahead: fn_node), + function_adapter: function_adapter, + function_args: function_adapter.extract_args(args, element_names) { return nil } + ) + end + def build_groupings_from(node_node, aggregation_name, from_field_path: []) grouped_by_node = selection_above_grouping_fields(node_node, element_names.grouped_by, aggregation_name) diff --git a/elasticgraph-graphql/lib/elastic_graph/graphql/aggregation/resolvers/aggregated_values.rb b/elasticgraph-graphql/lib/elastic_graph/graphql/aggregation/resolvers/aggregated_values.rb index 36db4a073..7e9875afe 100644 --- a/elasticgraph-graphql/lib/elastic_graph/graphql/aggregation/resolvers/aggregated_values.rb +++ b/elasticgraph-graphql/lib/elastic_graph/graphql/aggregation/resolvers/aggregated_values.rb @@ -21,26 +21,26 @@ def resolve(field:, object:, args:, context:, lookahead:) function_adapter = field.function_adapter # : FunctionAdapter::adapter - # `QueryAdapter` already validated this field's args when the query was built, but it can't - # raise there without failing resolution of the entire aggregations field rather than just - # this one leaf--so it skips building a datastore clause for an invalid field instead. Here, - # at resolve time, we're actually resolving this specific field, so we re-run the same - # validation and can attribute an error to the correct, precise path when it fails. + # `QueryAdapter` detected any invalid args when the query was built, but it can't report an + # error there without failing resolution of the entire aggregations field rather than just + # this one leaf--so it omits the datastore clause for an invalid field instead. Here, at + # resolve time, we're resolving this specific field, so we can report the error at the + # correct, precise path. # # `args` is already in schema form here (`GraphQLAdapterBuilder` converts it before calling # `resolve`), unlike at query-build time where `QueryAdapter` converts the raw AST arguments # itself--so, unlike there, we pass `args` through as-is rather than re-converting it. - begin - function_adapter.extract_args(args, schema.element_names) - rescue ::GraphQL::ExecutionError => e + function_adapter.extract_args(args, schema.element_names) do |message| + error = ::GraphQL::ExecutionError.new(message) + # Neither `context.add_error` nor `context.execution_errors.add` sets `path` for us--that # only happens automatically when an `ExecutionError` is raised and returned as a field's # own resolution result, which isn't the case here since we're continuing on to resolve # sibling fields normally. So we set `path` ourselves from `context.current_path`, which # is already the precise path to this field (e.g. `[..., "aggregatedValues", "amountCents", # "p150"]`), so the error in the response is attributed to this specific field. - e.path ||= context.current_path - context.add_error(e) + error.path = context.current_path + context.add_error(error) return nil end diff --git a/elasticgraph-graphql/sig/elastic_graph/graphql/aggregation/function_adapter.rbs b/elasticgraph-graphql/sig/elastic_graph/graphql/aggregation/function_adapter.rbs index 14e1d8602..933ab2527 100644 --- a/elasticgraph-graphql/sig/elastic_graph/graphql/aggregation/function_adapter.rbs +++ b/elasticgraph-graphql/sig/elastic_graph/graphql/aggregation/function_adapter.rbs @@ -25,7 +25,7 @@ module ElasticGraph end class SimpleMetric < SimpleMetricSupertype - def extract_args: (::Hash[::String, untyped], SchemaArtifacts::RuntimeMetadata::SchemaElementNames) -> functionArgs + def extract_args: (::Hash[::String, untyped], SchemaArtifacts::RuntimeMetadata::SchemaElementNames) { (::String) -> bot } -> functionArgs def clause_options: (functionArgs) -> ::Hash[::String, untyped] def extract_result: (::Hash[::String, untyped]) -> ::Hash[::String, untyped] def empty_bucket_result: () -> ::Hash[::String, untyped] @@ -39,7 +39,7 @@ module ElasticGraph end class Percentile < PercentileSupertype - def extract_args: (::Hash[::String, untyped], SchemaArtifacts::RuntimeMetadata::SchemaElementNames) -> functionArgs + def extract_args: (::Hash[::String, untyped], SchemaArtifacts::RuntimeMetadata::SchemaElementNames) { (::String) -> bot } -> functionArgs def clause_options: (functionArgs) -> ::Hash[::String, untyped] def extract_result: (::Hash[::String, untyped]) -> ::Hash[::String, untyped] def empty_bucket_result: () -> ::Hash[::String, untyped] diff --git a/elasticgraph-graphql/sig/elastic_graph/graphql/aggregation/query_adapter.rbs b/elasticgraph-graphql/sig/elastic_graph/graphql/aggregation/query_adapter.rbs index 95656e129..9937c0bbe 100644 --- a/elasticgraph-graphql/sig/elastic_graph/graphql/aggregation/query_adapter.rbs +++ b/elasticgraph-graphql/sig/elastic_graph/graphql/aggregation/query_adapter.rbs @@ -91,6 +91,11 @@ module ElasticGraph ?from_field_path: ::Array[PathSegment] ) -> ::Set[Computation] + def computation_for: ( + ::GraphQL::Execution::Lookahead, + ::Array[PathSegment] + ) -> Computation? + def build_groupings_from: ( ::GraphQL::Execution::Lookahead, ::String, diff --git a/elasticgraph-graphql/spec/acceptance/aggregations_spec.rb b/elasticgraph-graphql/spec/acceptance/aggregations_spec.rb index 6026c3df9..475c7cbc2 100644 --- a/elasticgraph-graphql/spec/acceptance/aggregations_spec.rb +++ b/elasticgraph-graphql/spec/acceptance/aggregations_spec.rb @@ -923,6 +923,10 @@ def expected_aggregated_amounts_of(*raw_values) max = raw_values.max sum = raw_values.sum avg = raw_values.empty? ? nil : sum.to_f / raw_values.size + # Exact values for `percentile: 50`/`75`, verified against a real OpenSearch `percentiles` + # aggregation for each of this file's fixture shapes (empty, singletons, pairs, and the full + # triple)--every shape here lands on a clean value, not a t-digest approximation artifact. + median, p75 = percentile_50_and_75_of(raw_values) { case_correctly("approximate_sum") => float_of(sum), @@ -932,11 +936,32 @@ def expected_aggregated_amounts_of(*raw_values) # same expected values to verify `approximate_percentile` without needing separate fixture data. "p0" => float_of(min), "p100" => float_of(max), + # `p50`/`p75` (unlike p0/p100) can't be satisfied by an implementation that only supports + # min/max, so they're what actually proves full percentile support works. + "p50" => float_of(median), + "p75" => float_of(p75), case_correctly("exact_min") => int_of(min), case_correctly("exact_max") => int_of(max) } end + # OpenSearch's percentile algorithm (t-digest) is generally approximate, but for these small, + # duplicate-free fixture sizes it always lands on one of the input values, so we can hardcode the + # expectation. Verified directly against a real OpenSearch `percentiles` aggregation, not derived + # by formula, since t-digest doesn't guarantee a standard percentile-interpolation formula's result. + def percentile_50_and_75_of(raw_values) + case raw_values.sort + when [] then [nil, nil] + when [100] then [100.0, 100.0] + when [200] then [200.0, 200.0] + when [300] then [300.0, 300.0] + when [100, 200] then [200.0, 200.0] + when [100, 300] then [300.0, 300.0] + when [100, 200, 300] then [200.0, 300.0] + else raise "No verified p50/p75 for #{raw_values.inspect}--verify against a real datastore and add it here." + end + end + def verify_all_timestamp_groupings_valid(widget_id, truncation_unit_type:, field:) truncation_unit_type = apply_derived_type_customizations(truncation_unit_type) datetime_granularities = graphql.schema.type_named(truncation_unit_type).graphql_type.values.keys @@ -1824,6 +1849,8 @@ def amount_aggregation(*fields, **agg_args) exact_sum approximate_avg p0: approximate_percentile(percentile: 0) + p50: approximate_percentile(percentile: 50) + p75: approximate_percentile(percentile: 75) p100: approximate_percentile(percentile: 100) exact_min exact_max @@ -1835,6 +1862,8 @@ def amount_aggregation(*fields, **agg_args) exact_sum approximate_avg p0: approximate_percentile(percentile: 0) + p50: approximate_percentile(percentile: 50) + p75: approximate_percentile(percentile: 75) p100: approximate_percentile(percentile: 100) exact_min exact_max @@ -1860,6 +1889,8 @@ def all_amount_aggregations exact_sum approximate_avg p0: approximate_percentile(percentile: 0) + p50: approximate_percentile(percentile: 50) + p75: approximate_percentile(percentile: 75) p100: approximate_percentile(percentile: 100) exact_min exact_max @@ -1871,6 +1902,8 @@ def all_amount_aggregations exact_sum approximate_avg p0: approximate_percentile(percentile: 0) + p50: approximate_percentile(percentile: 50) + p75: approximate_percentile(percentile: 75) p100: approximate_percentile(percentile: 100) exact_min exact_max diff --git a/elasticgraph-graphql/spec/unit/elastic_graph/graphql/aggregation/function_adapter_spec.rb b/elasticgraph-graphql/spec/unit/elastic_graph/graphql/aggregation/function_adapter_spec.rb index 7823d6700..d2e69bed1 100644 --- a/elasticgraph-graphql/spec/unit/elastic_graph/graphql/aggregation/function_adapter_spec.rb +++ b/elasticgraph-graphql/spec/unit/elastic_graph/graphql/aggregation/function_adapter_spec.rb @@ -15,7 +15,7 @@ module Aggregation RSpec.describe FunctionAdapter do describe "BY_NAME" do it "registers an adapter for each supported aggregated value function" do - expect(FunctionAdapter::BY_NAME.keys).to contain_exactly(:avg, :cardinality, :max, :min, :percentiles, :sum) + expect(FunctionAdapter::BY_NAME.keys).to contain_exactly(:avg, :cardinality, :max, :min, :percentile, :sum) end it "maps each function to its datastore aggregation name" do @@ -24,7 +24,7 @@ module Aggregation cardinality: "cardinality", max: "max", min: "min", - percentiles: "percentiles", + percentile: "percentiles", sum: "sum" ) end @@ -35,7 +35,7 @@ module Aggregation cardinality: {"value" => 0}, max: {"value" => nil}, min: {"value" => nil}, - percentiles: {"values" => [{"value" => nil}]}, + percentile: {"values" => [{"value" => nil}]}, sum: {"value" => 0} ) end @@ -50,7 +50,7 @@ module Aggregation end it "extracts no args, since these functions take none" do - expect(adapter.extract_args({"some_arg" => 3}, element_names)).to eq({}) + expect(adapter.extract_args({"some_arg" => 3}, element_names) { |msg| raise msg }).to eq({}) end it "contributes no extra clause options beyond the `field` the clause builder provides" do @@ -75,25 +75,22 @@ module Aggregation expect(adapter.datastore_function_name).to eq "percentiles" end - it "extracts the requested percentile rank from the args" do - expect(adapter.extract_args({"percentile" => 50.0}, element_names)).to eq({percentile: 50.0}) + it "extracts the requested percentile from the args" do + expect(extract_args({"percentile" => 50.0})).to eq({percentile: 50.0}) end it "accepts the boundary values 0 and 100" do - expect(adapter.extract_args({"percentile" => 0}, element_names)).to eq({percentile: 0}) - expect(adapter.extract_args({"percentile" => 100}, element_names)).to eq({percentile: 100}) + expect(extract_args({"percentile" => 0})).to eq({percentile: 0}) + expect(extract_args({"percentile" => 100})).to eq({percentile: 100}) end - it "raises a GraphQL::ExecutionError when the requested percentile is below 0" do - expect { - adapter.extract_args({"percentile" => -1}, element_names) - }.to raise_error(::GraphQL::ExecutionError, "`percentile` must be between 0 and 100, but is -1.") + it "yields an error message--rather than raising--when the requested percentile is outside 0 to 100" do + expect(extract_args({"percentile" => -1})).to eq "`percentile` must be between 0 and 100, but is -1." + expect(extract_args({"percentile" => 150})).to eq "`percentile` must be between 0 and 100, but is 150." end - it "raises a GraphQL::ExecutionError when the requested percentile is above 100" do - expect { - adapter.extract_args({"percentile" => 150}, element_names) - }.to raise_error(::GraphQL::ExecutionError, "`percentile` must be between 0 and 100, but is 150.") + it "yields an error message when the requested percentile is not numeric" do + expect(extract_args({"percentile" => "50"})).to eq '`percentile` must be between 0 and 100, but is "50".' end it "requests a single unkeyed percentile in the clause options" do @@ -107,6 +104,12 @@ module Aggregation it "fabricates an empty bucket response with a nil value" do expect(adapter.empty_bucket_result).to eq({"values" => [{"value" => nil}]}) end + + # Returns the extracted args, or--when the args are invalid--the yielded error message, + # so that a single expectation can cover either outcome. + def extract_args(args) + adapter.extract_args(args, element_names) { |message| return message } + end end end end diff --git a/elasticgraph-graphql/spec/unit/elastic_graph/graphql/aggregation/resolvers/aggregation_resolver_support.rb b/elasticgraph-graphql/spec/unit/elastic_graph/graphql/aggregation/resolvers/aggregation_resolver_support.rb index f2a735222..8d0c2b07f 100644 --- a/elasticgraph-graphql/spec/unit/elastic_graph/graphql/aggregation/resolvers/aggregation_resolver_support.rb +++ b/elasticgraph-graphql/spec/unit/elastic_graph/graphql/aggregation/resolvers/aggregation_resolver_support.rb @@ -23,13 +23,19 @@ def resolve_target_nodes( hit_count: nil, aggs: {"target" => {"buckets" => target_buckets}}, path: ["data", "target", "nodes"], - allow_errors: false + expect_errors: false ) allow(datastore_client).to receive(:msearch).and_return({"responses" => [datastore_response_payload_with_aggs(aggs, hit_count)]}) response = graphql.graphql_query_executor.execute("query { #{inner_query} }") - return response if allow_errors - expect(response["errors"]).to eq([]).or eq(nil) + + if expect_errors + expect(response["errors"]).not_to be_empty + else + expect(response["errors"]).to eq([]).or eq(nil) + end + + return response if expect_errors response.dig(*path) end diff --git a/elasticgraph-graphql/spec/unit/elastic_graph/graphql/aggregation/resolvers/aggregations_spec.rb b/elasticgraph-graphql/spec/unit/elastic_graph/graphql/aggregation/resolvers/aggregations_spec.rb index b7efd4605..b0881af3d 100644 --- a/elasticgraph-graphql/spec/unit/elastic_graph/graphql/aggregation/resolvers/aggregations_spec.rb +++ b/elasticgraph-graphql/spec/unit/elastic_graph/graphql/aggregation/resolvers/aggregations_spec.rb @@ -418,7 +418,7 @@ module Aggregation aggregated_value_key_of("amount_cents", "good") => {"values" => [{"value" => 500.0}]} } - response = resolve_target_nodes(<<~QUERY, aggs: aggs, allow_errors: true) + response = resolve_target_nodes(<<~QUERY, aggs: aggs, expect_errors: true) target: widget_aggregations { nodes { aggregated_values { diff --git a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/built_in_types.rb b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/built_in_types.rb index 878352930..368b9ed19 100644 --- a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/built_in_types.rb +++ b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/built_in_types.rb @@ -1599,7 +1599,7 @@ def define_temporal_aggregated_values(aggregated_values_type, scalar_type, &bloc def define_approximate_percentile_on_aggregated_values(aggregated_values_type, scalar_type) aggregated_values_type.field names.approximate_percentile, scalar_type, graphql_only: true do |f| - f.computes :percentiles + f.computes :percentile f.argument names.percentile, "Float!" do |a| a.documentation "The percentile to compute, from `0` to `100` (e.g. `0` for the min, `100` for the max, `50` for the median, `99` for the 99th percentile)." diff --git a/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/graphql_schema/built_in_types_spec.rb b/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/graphql_schema/built_in_types_spec.rb index d931350fd..454742141 100644 --- a/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/graphql_schema/built_in_types_spec.rb +++ b/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/graphql_schema/built_in_types_spec.rb @@ -1144,7 +1144,7 @@ def expect_list_filter(scalar, fields_description: "`[#{scalar}]` fields") """ #{schema_elements.approximate_percentile}( """ - The percentile rank to compute, from `0` to `100` (e.g. `0` for the min, `100` for the max, `50` for the median, `99` for the 99th percentile). + The percentile to compute, from `0` to `100` (e.g. `0` for the min, `100` for the max, `50` for the median, `99` for the 99th percentile). """ #{schema_elements.percentile}: Float!): Float } @@ -1196,7 +1196,7 @@ def expect_list_filter(scalar, fields_description: "`[#{scalar}]` fields") """ #{schema_elements.approximate_percentile}( """ - The percentile rank to compute, from `0` to `100` (e.g. `0` for the min, `100` for the max, `50` for the median, `99` for the 99th percentile). + The percentile to compute, from `0` to `100` (e.g. `0` for the min, `100` for the max, `50` for the median, `99` for the 99th percentile). """ #{schema_elements.percentile}: Float!): #{scalar_type} } @@ -1326,7 +1326,7 @@ def expect_list_filter(scalar, fields_description: "`[#{scalar}]` fields") """ #{schema_elements.approximate_percentile}( """ - The percentile rank to compute, from `0` to `100` (e.g. `0` for the min, `100` for the max, `50` for the median, `99` for the 99th percentile). + The percentile to compute, from `0` to `100` (e.g. `0` for the min, `100` for the max, `50` for the median, `99` for the 99th percentile). """ #{schema_elements.percentile}: Float!): Float } @@ -1426,7 +1426,7 @@ def expect_list_filter(scalar, fields_description: "`[#{scalar}]` fields") """ #{schema_elements.approximate_percentile}( """ - The percentile rank to compute, from `0` to `100` (e.g. `0` for the min, `100` for the max, `50` for the median, `99` for the 99th percentile). + The percentile to compute, from `0` to `100` (e.g. `0` for the min, `100` for the max, `50` for the median, `99` for the 99th percentile). """ #{schema_elements.percentile}: Float!): Float } diff --git a/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/object_types_by_name/for_built_in_types_spec.rb b/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/object_types_by_name/for_built_in_types_spec.rb index 59b53cdeb..493159ef3 100644 --- a/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/object_types_by_name/for_built_in_types_spec.rb +++ b/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/object_types_by_name/for_built_in_types_spec.rb @@ -31,7 +31,7 @@ module SchemaDefinition "exact_max" => :max, "exact_min" => :min, "approximate_distinct_value_count" => :cardinality, - "approximate_percentile" => :percentiles + "approximate_percentile" => :percentile ) end @@ -51,7 +51,7 @@ module SchemaDefinition "exact_max" => :max, "exact_min" => :min, "approximate_distinct_value_count" => :cardinality, - "approximate_percentile" => :percentiles + "approximate_percentile" => :percentile ) end @@ -72,7 +72,7 @@ module SchemaDefinition "exact_max" => :max, "exact_min" => :min, "approximate_distinct_value_count" => :cardinality, - "approximate_percentile" => :percentiles + "approximate_percentile" => :percentile ) end @@ -95,7 +95,7 @@ module SchemaDefinition "approximate_min" => :min, "exact_min" => :min, "approximate_distinct_value_count" => :cardinality, - "approximate_percentile" => :percentiles + "approximate_percentile" => :percentile ) end end From b3d18ac6e163b775a22dd124027b7c7639002438 Mon Sep 17 00:00:00 2001 From: nikhilkumarjadhav-toast Date: Fri, 7 Aug 2026 12:07:15 -0700 Subject: [PATCH 7/7] Fix cross-backend percentile assertion that broke CI The previous commit hardcoded exact p50/p75 values verified against only one local OpenSearch 3.6.0 instance. CI runs this suite against four backends (Elasticsearch 9.0.0/9.4.2, OpenSearch 2.19.0/3.6.0), and for small datasets different backends' percentile algorithms legitimately use different (equally valid) interpolation conventions for ranks that don't land exactly on one data point--e.g. p50 of [100, 200] is 150 on some backends and 200 on others. Replace the exact-value assertions with a bounds check (a real Float within [min, max]), which every backend's percentile algorithm must satisfy regardless of interpolation convention, while still ruling out an implementation that only special-cases percentile 0/100. Verified locally against both OpenSearch 3.6.0 and Elasticsearch 9.4.2. --- .../spec/acceptance/aggregations_spec.rb | 31 ++++++------------- 1 file changed, 9 insertions(+), 22 deletions(-) diff --git a/elasticgraph-graphql/spec/acceptance/aggregations_spec.rb b/elasticgraph-graphql/spec/acceptance/aggregations_spec.rb index 475c7cbc2..de5e9be6b 100644 --- a/elasticgraph-graphql/spec/acceptance/aggregations_spec.rb +++ b/elasticgraph-graphql/spec/acceptance/aggregations_spec.rb @@ -923,10 +923,6 @@ def expected_aggregated_amounts_of(*raw_values) max = raw_values.max sum = raw_values.sum avg = raw_values.empty? ? nil : sum.to_f / raw_values.size - # Exact values for `percentile: 50`/`75`, verified against a real OpenSearch `percentiles` - # aggregation for each of this file's fixture shapes (empty, singletons, pairs, and the full - # triple)--every shape here lands on a clean value, not a t-digest approximation artifact. - median, p75 = percentile_50_and_75_of(raw_values) { case_correctly("approximate_sum") => float_of(sum), @@ -937,29 +933,20 @@ def expected_aggregated_amounts_of(*raw_values) "p0" => float_of(min), "p100" => float_of(max), # `p50`/`p75` (unlike p0/p100) can't be satisfied by an implementation that only supports - # min/max, so they're what actually proves full percentile support works. - "p50" => float_of(median), - "p75" => float_of(p75), + # min/max, so they're what actually proves full percentile support works. We can't pin an + # exact expected value here: different datastore versions/backends use different (equally + # valid) interpolation conventions for a rank that doesn't land exactly on one data point, so + # we only assert it's a real number within [min, max] (a property every backend guarantees). + "p50" => percentile_of(min, max), + "p75" => percentile_of(min, max), case_correctly("exact_min") => int_of(min), case_correctly("exact_max") => int_of(max) } end - # OpenSearch's percentile algorithm (t-digest) is generally approximate, but for these small, - # duplicate-free fixture sizes it always lands on one of the input values, so we can hardcode the - # expectation. Verified directly against a real OpenSearch `percentiles` aggregation, not derived - # by formula, since t-digest doesn't guarantee a standard percentile-interpolation formula's result. - def percentile_50_and_75_of(raw_values) - case raw_values.sort - when [] then [nil, nil] - when [100] then [100.0, 100.0] - when [200] then [200.0, 200.0] - when [300] then [300.0, 300.0] - when [100, 200] then [200.0, 200.0] - when [100, 300] then [300.0, 300.0] - when [100, 200, 300] then [200.0, 300.0] - else raise "No verified p50/p75 for #{raw_values.inspect}--verify against a real datastore and add it here." - end + def percentile_of(min, max) + return nil if min.nil? + (be >= min).and(be <= max).and a_kind_of(::Float) end def verify_all_timestamp_groupings_valid(widget_id, truncation_unit_type:, field:)