diff --git a/elasticgraph-graphql/lib/elastic_graph/graphql/aggregation/computation.rb b/elasticgraph-graphql/lib/elastic_graph/graphql/aggregation/computation.rb index dd8e5b938..d577a390c 100644 --- a/elasticgraph-graphql/lib/elastic_graph/graphql/aggregation/computation.rb +++ b/elasticgraph-graphql/lib/elastic_graph/graphql/aggregation/computation.rb @@ -6,8 +6,9 @@ # # frozen_string_literal: true -require "elastic_graph/graphql/aggregation/key" require "elastic_graph/graphql/aggregation/field_path_encoder" +require "elastic_graph/graphql/aggregation/key" +require "elastic_graph/graphql/aggregation/path_segment" module ElasticGraph class GraphQL @@ -18,14 +19,14 @@ module Aggregation # https://www.elastic.co/guide/en/elasticsearch/reference/7.12/search-aggregations-metrics-max-aggregation.html # https://www.elastic.co/guide/en/elasticsearch/reference/7.12/search-aggregations-metrics-min-aggregation.html # https://www.elastic.co/guide/en/elasticsearch/reference/7.12/search-aggregations-metrics-sum-aggregation.html - Computation = ::Data.define(:source_field_path, :computed_index_field_name, :detail) do + Computation = ::Data.define(:source_field_path, :leaf, :detail) do # @implements Computation def key(aggregation_name:) Key::AggregatedValue.new( aggregation_name: aggregation_name, field_path: source_field_path.map(&:name_in_graphql_query), - function_name: computed_index_field_name + function_name: leaf.name_in_graphql_query ).encode 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 16130667c..fe62a4bf2 100644 --- a/elasticgraph-graphql/lib/elastic_graph/graphql/aggregation/query_adapter.rb +++ b/elasticgraph-graphql/lib/elastic_graph/graphql/aggregation/query_adapter.rb @@ -188,11 +188,11 @@ def build_computations_from(node_node, from_field_path: []) get_children_nodes(node).map do |fn_node| computed_field = field_from_node(fn_node) - computation_detail = field_from_node(fn_node).computation_detail # : SchemaArtifacts::RuntimeMetadata::ComputationDetail + computation_detail = computed_field.computation_detail # : SchemaArtifacts::RuntimeMetadata::ComputationDetail Aggregation::Computation.new( source_field_path: field_path, - computed_index_field_name: computed_field.name_in_index, + leaf: PathSegment.for(field: computed_field, lookahead: fn_node), detail: computation_detail ) 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 1ef2735ea..2df1cdb53 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,7 +21,7 @@ def resolve(field:, object:, args:, context:, lookahead:) key = Key::AggregatedValue.new( aggregation_name: aggregation_name, field_path: field_path.map(&:name_in_graphql_query), - function_name: field.name_in_index + function_name: PathSegment.for(field: field, lookahead: lookahead).name_in_graphql_query ) result = Support::HashUtil.verbose_fetch(bucket, key.encode) diff --git a/elasticgraph-graphql/sig/elastic_graph/graphql/aggregation/computation.rbs b/elasticgraph-graphql/sig/elastic_graph/graphql/aggregation/computation.rbs index bf198c54d..50825a14e 100644 --- a/elasticgraph-graphql/sig/elastic_graph/graphql/aggregation/computation.rbs +++ b/elasticgraph-graphql/sig/elastic_graph/graphql/aggregation/computation.rbs @@ -3,7 +3,7 @@ module ElasticGraph module Aggregation class Computation attr_reader source_field_path: fieldPath - attr_reader computed_index_field_name: ::String + attr_reader leaf: PathSegment attr_reader detail: SchemaArtifacts::RuntimeMetadata::ComputationDetail def key: (aggregation_name: ::String) -> ::String @@ -11,13 +11,13 @@ module ElasticGraph def initialize : ( source_field_path: fieldPath, - computed_index_field_name: ::String, + leaf: PathSegment, detail: SchemaArtifacts::RuntimeMetadata::ComputationDetail ) -> void def with: ( ?source_field_path: fieldPath, - ?computed_index_field_name: ::String, + ?leaf: PathSegment, ?detail: SchemaArtifacts::RuntimeMetadata::ComputationDetail ) -> instance end diff --git a/elasticgraph-graphql/spec/acceptance/aggregations_spec.rb b/elasticgraph-graphql/spec/acceptance/aggregations_spec.rb index 9672bab16..2429639b3 100644 --- a/elasticgraph-graphql/spec/acceptance/aggregations_spec.rb +++ b/elasticgraph-graphql/spec/acceptance/aggregations_spec.rb @@ -151,6 +151,21 @@ module ElasticGraph size_uniq_count = widget_ungrouped_aggregated_values_for("size { approximate_distinct_value_count }") expect(size_uniq_count).to eq({"size" => {case_correctly("approximate_distinct_value_count") => 2}}) + # Verify that two aliases of the same aggregated value function under one field each resolve correctly, + # rather than colliding on a single datastore aggregation. + aliased_mins = widget_ungrouped_aggregated_values_for(<<~QUERY) + #{amount_cents} { + #{case_correctly("exact_min")} + #{case_correctly("my_min")}: #{case_correctly("exact_min")} + } + QUERY + expect(aliased_mins).to eq({ + amount_cents => { + case_correctly("exact_min") => 100, + case_correctly("my_min") => 100 + } + }) + aggregations = group_widget_currencies_by_widget_name expect(aggregations).to eq [ {"count" => 1, case_correctly("grouped_by") => {case_correctly("widget_name") => "w100"}}, diff --git a/elasticgraph-graphql/spec/support/aggregations_helpers.rb b/elasticgraph-graphql/spec/support/aggregations_helpers.rb index cfec7dc72..90156c2cc 100644 --- a/elasticgraph-graphql/spec/support/aggregations_helpers.rb +++ b/elasticgraph-graphql/spec/support/aggregations_helpers.rb @@ -21,12 +21,12 @@ module ElasticGraph module AggregationsHelpers - def computation_of(*field_names_in_index, function, computed_field_name: function.to_s, field_names_in_graphql_query: field_names_in_index) + def computation_of(*field_names_in_index, function, computed_field_name: function.to_s, leaf_alias: computed_field_name, field_names_in_graphql_query: field_names_in_index) source_field_path = build_field_path(names_in_index: field_names_in_index, names_in_graphql_query: field_names_in_graphql_query) GraphQL::Aggregation::Computation.new( source_field_path: source_field_path, - computed_index_field_name: computed_field_name, + leaf: GraphQL::Aggregation::PathSegment.new(name_in_graphql_query: leaf_alias, name_in_index: computed_field_name), detail: SchemaArtifacts::RuntimeMetadata::ComputationDetail.new( function: function, empty_bucket_value: (function == :sum || function == :cardinality) ? 0 : nil diff --git a/elasticgraph-graphql/spec/unit/elastic_graph/graphql/aggregation/computation_spec.rb b/elasticgraph-graphql/spec/unit/elastic_graph/graphql/aggregation/computation_spec.rb index a191c21df..2304f891a 100644 --- a/elasticgraph-graphql/spec/unit/elastic_graph/graphql/aggregation/computation_spec.rb +++ b/elasticgraph-graphql/spec/unit/elastic_graph/graphql/aggregation/computation_spec.rb @@ -27,6 +27,20 @@ module Aggregation expect(computation.key(aggregation_name: "my_aggs")).to eq aggregated_value_key_of("oof", "rab", "average", aggregation_name: "my_aggs").encode end + + it "uses the leaf's GraphQL query alias rather than its name in the index" do + computation = computation_of("foo", "bar", :min, computed_field_name: "exact_min", leaf_alias: "myMin") + + expect(computation.key(aggregation_name: "my_aggs")).to eq aggregated_value_key_of("foo", "bar", "myMin", aggregation_name: "my_aggs").encode + end + + it "differs between two aliases of the same function under the same field, so they don't collapse into one computation" do + computation = computation_of("foo", :min, computed_field_name: "exact_min", leaf_alias: "exactMin") + aliased_computation = computation_of("foo", :min, computed_field_name: "exact_min", leaf_alias: "myMin") + + expect(computation).not_to eq(aliased_computation) + expect(computation.key(aggregation_name: "my_aggs")).not_to eq(aliased_computation.key(aggregation_name: "my_aggs")) + end end describe "#clause" do @@ -47,6 +61,12 @@ module Aggregation expect(computation.clause).to eq({"avg" => {"field" => "foo.c.bar.d"}}) end + + it "is unaffected by an alias on the leaf" do + computation = computation_of("foo", "bar", :avg, computed_field_name: "approximate_avg", leaf_alias: "myAvg") + + expect(computation.clause).to eq({"avg" => {"field" => "foo.bar"}}) + end end end end diff --git a/elasticgraph-graphql/spec/unit/elastic_graph/graphql/aggregation/query_adapter_spec.rb b/elasticgraph-graphql/spec/unit/elastic_graph/graphql/aggregation/query_adapter_spec.rb index b9e6627c5..b33d128c9 100644 --- a/elasticgraph-graphql/spec/unit/elastic_graph/graphql/aggregation/query_adapter_spec.rb +++ b/elasticgraph-graphql/spec/unit/elastic_graph/graphql/aggregation/query_adapter_spec.rb @@ -1440,7 +1440,11 @@ def build_sub_aggregation_query(args, default_page_size:, max_page_size:) name: "widget_aggregations", computations: [ computation_of("amount_cents", :avg, computed_field_name: "approximate_avg", field_names_in_graphql_query: ["ac1"]), - computation_of("amount_cents", :avg, computed_field_name: "approximate_avg", field_names_in_graphql_query: ["ac2"]), + # `aa1` and `aa2` are two aliases of the same function under the same field. Since the leaf key is + # derived from the alias, these produce two distinct computations (and, ultimately, two identical + # datastore aggregation clauses) rather than collapsing into one. + computation_of("amount_cents", :avg, computed_field_name: "approximate_avg", leaf_alias: "aa1", field_names_in_graphql_query: ["ac2"]), + computation_of("amount_cents", :avg, computed_field_name: "approximate_avg", leaf_alias: "aa2", field_names_in_graphql_query: ["ac2"]), computation_of("amount_cents", :sum, computed_field_name: "exact_sum", field_names_in_graphql_query: ["ac2"]) ] )]) @@ -1793,8 +1797,8 @@ def build_sub_aggregation_query(args, default_page_size:, max_page_size:) expect(aggregations).to eq([aggregation_query_of( name: "wa", computations: [ - computation_of("amount_cents", :avg, computed_field_name: "approximate_avg", field_names_in_graphql_query: ["ac"]), - computation_of("amount_cents", :max, computed_field_name: "exact_max", field_names_in_graphql_query: ["ac"]) + computation_of("amount_cents", :avg, computed_field_name: "approximate_avg", leaf_alias: "aa", field_names_in_graphql_query: ["ac"]), + computation_of("amount_cents", :max, computed_field_name: "exact_max", leaf_alias: "em", field_names_in_graphql_query: ["ac"]) ], groupings: [ field_term_grouping_of("size", field_names_in_graphql_query: ["s"]), @@ -1900,8 +1904,8 @@ def build_sub_aggregation_query(args, default_page_size:, max_page_size:) expect(aggregations).to eq([aggregation_query_of( name: "wa", computations: [ - computation_of("amount_cents", :avg, computed_field_name: "approximate_avg", field_names_in_graphql_query: ["ac"]), - computation_of("amount_cents", :max, computed_field_name: "exact_max", field_names_in_graphql_query: ["ac"]) + computation_of("amount_cents", :avg, computed_field_name: "approximate_avg", leaf_alias: "aa", field_names_in_graphql_query: ["ac"]), + computation_of("amount_cents", :max, computed_field_name: "exact_max", leaf_alias: "em", field_names_in_graphql_query: ["ac"]) ], groupings: [ field_term_grouping_of("size", field_names_in_graphql_query: ["s"]), 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 8c6b731b1..59040b0b4 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 @@ -284,6 +284,39 @@ module Aggregation ] end + it "uses GraphQL field aliases when resolving the aggregation function leaf fields, allowing the same function to be requested multiple times under one field" do + aggs = { + aggregated_value_key_of("amount_cents", "es") => {"value" => 900.0}, + aggregated_value_key_of("amount_cents", "exact_sum") => {"value" => 900.0}, + aggregated_value_key_of("cost", "amount_cents", "mx") => {"value" => 400.0} + } + + response = resolve_target_nodes(<<~QUERY, aggs: aggs) + target: widget_aggregations { + nodes { + aggregated_values { + amount_cents { + es: exact_sum + exact_sum + } + cost { + amount_cents { mx: exact_max } + } + } + } + } + QUERY + + expect(response).to eq [ + { + "aggregated_values" => { + "amount_cents" => {"es" => 900, "exact_sum" => 900}, + "cost" => {"amount_cents" => {"mx" => 400}} + } + } + ] + end + it "resolves aggregated Date/DateTime/LocalTime values" do aggs = { aggregated_value_key_of("created_at", "exact_min") => {"value" => 1696854612000.0, "value_as_string" => "2023-10-09T12:30:12.000Z"},