diff --git a/config/schema/artifacts/runtime_metadata.yaml b/config/schema/artifacts/runtime_metadata.yaml index 784ef9c9c..cc617562d 100644 --- a/config/schema/artifacts/runtime_metadata.yaml +++ b/config/schema/artifacts/runtime_metadata.yaml @@ -7714,6 +7714,7 @@ scalar_types_by_name: coercion_adapter: name: ElasticGraph::GraphQL::ScalarCoercionAdapters::Cursor require_path: elastic_graph/graphql/scalar_coercion_adapters/cursor + grouping_missing_value_placeholder: "$SECURE_RANDOM_VALUE" indexing_preparer: name: ElasticGraph::Indexer::IndexingPreparers::NoOp require_path: elastic_graph/indexer/indexing_preparers/no_op @@ -7735,6 +7736,7 @@ scalar_types_by_name: coercion_adapter: name: ElasticGraph::GraphQL::ScalarCoercionAdapters::NoOp require_path: elastic_graph/graphql/scalar_coercion_adapters/no_op + grouping_missing_value_placeholder: NaN indexing_preparer: name: ElasticGraph::Indexer::IndexingPreparers::NoOp require_path: elastic_graph/indexer/indexing_preparers/no_op @@ -7742,6 +7744,7 @@ scalar_types_by_name: coercion_adapter: name: ElasticGraph::GraphQL::ScalarCoercionAdapters::NoOp require_path: elastic_graph/graphql/scalar_coercion_adapters/no_op + grouping_missing_value_placeholder: "$SECURE_RANDOM_VALUE" indexing_preparer: name: ElasticGraph::Indexer::IndexingPreparers::NoOp require_path: elastic_graph/indexer/indexing_preparers/no_op @@ -7749,6 +7752,7 @@ scalar_types_by_name: coercion_adapter: name: ElasticGraph::GraphQL::ScalarCoercionAdapters::NoOp require_path: elastic_graph/graphql/scalar_coercion_adapters/no_op + grouping_missing_value_placeholder: NaN indexing_preparer: name: ElasticGraph::Indexer::IndexingPreparers::Integer require_path: elastic_graph/indexer/indexing_preparers/integer @@ -7756,6 +7760,7 @@ scalar_types_by_name: coercion_adapter: name: ElasticGraph::GraphQL::ScalarCoercionAdapters::JsonSafeLong require_path: elastic_graph/graphql/scalar_coercion_adapters/longs + grouping_missing_value_placeholder: NaN indexing_preparer: name: ElasticGraph::Indexer::IndexingPreparers::Integer require_path: elastic_graph/indexer/indexing_preparers/integer @@ -7777,6 +7782,7 @@ scalar_types_by_name: coercion_adapter: name: ElasticGraph::GraphQL::ScalarCoercionAdapters::NoOp require_path: elastic_graph/graphql/scalar_coercion_adapters/no_op + grouping_missing_value_placeholder: "$SECURE_RANDOM_VALUE" indexing_preparer: name: ElasticGraph::Indexer::IndexingPreparers::NoOp require_path: elastic_graph/indexer/indexing_preparers/no_op @@ -7784,6 +7790,7 @@ scalar_types_by_name: coercion_adapter: name: ElasticGraph::GraphQL::ScalarCoercionAdapters::TimeZone require_path: elastic_graph/graphql/scalar_coercion_adapters/time_zone + grouping_missing_value_placeholder: "$SECURE_RANDOM_VALUE" indexing_preparer: name: ElasticGraph::Indexer::IndexingPreparers::NoOp require_path: elastic_graph/indexer/indexing_preparers/no_op @@ -7791,6 +7798,7 @@ scalar_types_by_name: coercion_adapter: name: ElasticGraph::GraphQL::ScalarCoercionAdapters::Untyped require_path: elastic_graph/graphql/scalar_coercion_adapters/untyped + grouping_missing_value_placeholder: "$SECURE_RANDOM_VALUE" indexing_preparer: name: ElasticGraph::Indexer::IndexingPreparers::Untyped require_path: elastic_graph/indexer/indexing_preparers/untyped diff --git a/config/schema/artifacts_with_apollo/runtime_metadata.yaml b/config/schema/artifacts_with_apollo/runtime_metadata.yaml index 0b86e827f..5a0157a49 100644 --- a/config/schema/artifacts_with_apollo/runtime_metadata.yaml +++ b/config/schema/artifacts_with_apollo/runtime_metadata.yaml @@ -7851,6 +7851,7 @@ scalar_types_by_name: coercion_adapter: name: ElasticGraph::GraphQL::ScalarCoercionAdapters::Cursor require_path: elastic_graph/graphql/scalar_coercion_adapters/cursor + grouping_missing_value_placeholder: "$SECURE_RANDOM_VALUE" indexing_preparer: name: ElasticGraph::Indexer::IndexingPreparers::NoOp require_path: elastic_graph/indexer/indexing_preparers/no_op @@ -7879,6 +7880,7 @@ scalar_types_by_name: coercion_adapter: name: ElasticGraph::GraphQL::ScalarCoercionAdapters::NoOp require_path: elastic_graph/graphql/scalar_coercion_adapters/no_op + grouping_missing_value_placeholder: NaN indexing_preparer: name: ElasticGraph::Indexer::IndexingPreparers::NoOp require_path: elastic_graph/indexer/indexing_preparers/no_op @@ -7886,6 +7888,7 @@ scalar_types_by_name: coercion_adapter: name: ElasticGraph::GraphQL::ScalarCoercionAdapters::NoOp require_path: elastic_graph/graphql/scalar_coercion_adapters/no_op + grouping_missing_value_placeholder: "$SECURE_RANDOM_VALUE" indexing_preparer: name: ElasticGraph::Indexer::IndexingPreparers::NoOp require_path: elastic_graph/indexer/indexing_preparers/no_op @@ -7893,6 +7896,7 @@ scalar_types_by_name: coercion_adapter: name: ElasticGraph::GraphQL::ScalarCoercionAdapters::NoOp require_path: elastic_graph/graphql/scalar_coercion_adapters/no_op + grouping_missing_value_placeholder: NaN indexing_preparer: name: ElasticGraph::Indexer::IndexingPreparers::Integer require_path: elastic_graph/indexer/indexing_preparers/integer @@ -7900,6 +7904,7 @@ scalar_types_by_name: coercion_adapter: name: ElasticGraph::GraphQL::ScalarCoercionAdapters::JsonSafeLong require_path: elastic_graph/graphql/scalar_coercion_adapters/longs + grouping_missing_value_placeholder: NaN indexing_preparer: name: ElasticGraph::Indexer::IndexingPreparers::Integer require_path: elastic_graph/indexer/indexing_preparers/integer @@ -7921,6 +7926,7 @@ scalar_types_by_name: coercion_adapter: name: ElasticGraph::GraphQL::ScalarCoercionAdapters::NoOp require_path: elastic_graph/graphql/scalar_coercion_adapters/no_op + grouping_missing_value_placeholder: "$SECURE_RANDOM_VALUE" indexing_preparer: name: ElasticGraph::Indexer::IndexingPreparers::NoOp require_path: elastic_graph/indexer/indexing_preparers/no_op @@ -7928,6 +7934,7 @@ scalar_types_by_name: coercion_adapter: name: ElasticGraph::GraphQL::ScalarCoercionAdapters::TimeZone require_path: elastic_graph/graphql/scalar_coercion_adapters/time_zone + grouping_missing_value_placeholder: "$SECURE_RANDOM_VALUE" indexing_preparer: name: ElasticGraph::Indexer::IndexingPreparers::NoOp require_path: elastic_graph/indexer/indexing_preparers/no_op @@ -7935,6 +7942,7 @@ scalar_types_by_name: coercion_adapter: name: ElasticGraph::GraphQL::ScalarCoercionAdapters::Untyped require_path: elastic_graph/graphql/scalar_coercion_adapters/untyped + grouping_missing_value_placeholder: "$SECURE_RANDOM_VALUE" indexing_preparer: name: ElasticGraph::Indexer::IndexingPreparers::Untyped require_path: elastic_graph/indexer/indexing_preparers/untyped diff --git a/elasticgraph-schema_artifacts/sig/elastic_graph/schema_artifacts/runtime_metadata/scalar_type.rbs b/elasticgraph-schema_artifacts/sig/elastic_graph/schema_artifacts/runtime_metadata/scalar_type.rbs index b236f500a..5246fae4d 100644 --- a/elasticgraph-schema_artifacts/sig/elastic_graph/schema_artifacts/runtime_metadata/scalar_type.rbs +++ b/elasticgraph-schema_artifacts/sig/elastic_graph/schema_artifacts/runtime_metadata/scalar_type.rbs @@ -4,18 +4,18 @@ module ElasticGraph class ScalarTypeSupertype attr_reader coercion_adapter_ref: ::Hash[::String, ::String] attr_reader indexing_preparer_ref: ::Hash[::String, ::String] - attr_reader grouping_missing_value_placeholder: ::String? + attr_reader grouping_missing_value_placeholder: (::String | ::Numeric)? def initialize: ( coercion_adapter_ref: ::Hash[::String, ::String], indexing_preparer_ref: ::Hash[::String, ::String], - grouping_missing_value_placeholder: ::String? + grouping_missing_value_placeholder: (::String | ::Numeric)? ) -> void def with: ( ?coercion_adapter_ref: ::Hash[::String, ::String], ?indexing_preparer_ref: ::Hash[::String, ::String], - ?grouping_missing_value_placeholder: ::String? + ?grouping_missing_value_placeholder: (::String | ::Numeric)? ) -> ScalarType end diff --git a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/scalar_type.rb b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/scalar_type.rb index 69db7c60f..e4095a2cf 100644 --- a/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/scalar_type.rb +++ b/elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/scalar_type.rb @@ -44,6 +44,7 @@ module SchemaElements class ScalarType < Struct.new( :schema_def_state, :type_ref, + :grouping_missing_value_placeholder_overridden, :mapping_type, :runtime_metadata, :aggregated_values_customizations, @@ -66,7 +67,7 @@ class ScalarType < Struct.new( # @private def initialize(schema_def_state, name) - super(schema_def_state, schema_def_state.type_ref(name).to_final_form) + super(schema_def_state, schema_def_state.type_ref(name).to_final_form, false) # Default the runtime metadata before yielding, so it can be overridden as needed. self.runtime_metadata = SchemaArtifacts::RuntimeMetadata::ScalarType.new( @@ -85,6 +86,10 @@ def initialize(schema_def_state, name) if missing.any? raise Errors::SchemaError, "Scalar types require `mapping` and `json_schema` to be configured, but `#{name}` lacks #{missing.join(" and ")}." end + + if (placeholder = inferred_grouping_missing_value_placeholder) + self.runtime_metadata = runtime_metadata.with(grouping_missing_value_placeholder: placeholder) + end end # @return [String] name of the scalar type @@ -155,6 +160,31 @@ def prepare_for_indexing_with(preparer_name, defined_at:) }).tap(&:load_indexing_preparer) # verify the preparer is valid. end + # Specifies a placeholder value to use for missing values when grouping by this scalar type. + # This optimization allows ElasticGraph to use a single terms aggregation instead of separate + # terms and missing aggregations, reducing the exponential explosion of subaggregations when + # grouping by multiple fields. + # + # @param placeholder [String, Numeric] the placeholder value to use for missing/null values + # @return [void] + # + # @example Define a grouping missing value placeholder + # ElasticGraph.define_schema do |schema| + # schema.scalar_type "BigInt" do |t| + # t.mapping type: "long" + # t.json_schema type: "integer", minimum: -(2**53) + 1, maximum: (2**53) - 1 + # t.grouping_missing_value_placeholder "NaN" + # end + # end + def grouping_missing_value_placeholder(placeholder) + unless placeholder.nil? || placeholder.is_a?(String) || placeholder.is_a?(Numeric) + raise Errors::SchemaError, "grouping_missing_value_placeholder must be a String or Numeric value, but got #{placeholder.class}: #{placeholder.inspect}" + end + + self.grouping_missing_value_placeholder_overridden = true + self.runtime_metadata = runtime_metadata.with(grouping_missing_value_placeholder: placeholder) + end + # @return [String] the GraphQL SDL form of this scalar def to_sdl "#{formatted_documentation}scalar #{name} #{directives_sdl}" @@ -311,9 +341,45 @@ def to_aggregated_values_type schema_def_state.factory.new_aggregated_values_type_for_index_leaf_type(name, &customization_block) end + def inferred_grouping_missing_value_placeholder + return nil if grouping_missing_value_placeholder_overridden || mapping_type.nil? + + if STRING_TYPES.include?(mapping_type) + MISSING_STRING_PLACEHOLDER + elsif FLOAT_TYPES.include?(mapping_type) + MISSING_NUMERIC_PLACEHOLDER + elsif mapping_type == "long" + # It is only safe to use NaN for a long when the long's range is safe to coerce to a float + # without loss of precision. This is because using NaN as the missing value will cause + # the datastore to coerce the other bucket keys to float. + # JSON schema min/max only constrains newly indexed values, not existing data that may fall outside the range before the constraints were added. + # This is an edge case where the long range may exceed safe float precision. + # In this case, users can set grouping_missing_value_placeholder to nil. + if (json_schema_options[:minimum] || LONG_STRING_MIN) >= JSON_SAFE_LONG_MIN && + (json_schema_options[:maximum] || LONG_STRING_MAX) <= JSON_SAFE_LONG_MAX + MISSING_NUMERIC_PLACEHOLDER + end + elsif mapping_type == "unsigned_long" + # Similar to the checks above for long except we only need to check the max + # (since the min is zero even if not specified) + if (json_schema_options[:maximum] || LONG_STRING_MAX) <= JSON_SAFE_LONG_MAX + MISSING_NUMERIC_PLACEHOLDER + end + elsif INTEGER_TYPES.include?(mapping_type) + # All other integer types can safely be coerced to float without loss of precision + MISSING_NUMERIC_PLACEHOLDER + end + end + # https://www.elastic.co/guide/en/elasticsearch/reference/current/mapping-types.html # https://www.elastic.co/guide/en/elasticsearch/reference/7.13/number.html#number - NUMERIC_TYPES = %w[long integer short byte double float half_float scaled_float unsigned_long].to_set + FLOAT_TYPES = %w[double float half_float scaled_float].to_set + INTEGER_TYPES = %w[long integer short byte unsigned_long].to_set + NUMERIC_TYPES = FLOAT_TYPES | INTEGER_TYPES + # https://www.elastic.co/docs/reference/elasticsearch/mapping-reference/keyword + # https://www.elastic.co/docs/reference/elasticsearch/mapping-reference/text-type-family + # https://docs.opensearch.org/latest/mappings/supported-field-types/index/#string-based-field-types + STRING_TYPES = %w[keyword constant_keyword wildcard text match_only_text pattern_text semantic_text].to_set DATE_TYPES = %w[date date_nanos].to_set # The Elasticsearch/OpenSearch docs do not exhaustively give a list of types on which range queries are efficient, # but the docs are clear that it is efficient on numeric and date types, and is inefficient on string diff --git a/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/schema_elements/scalar_type.rbs b/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/schema_elements/scalar_type.rbs index ff5089f5d..d3539080f 100644 --- a/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/schema_elements/scalar_type.rbs +++ b/elasticgraph-schema_definition/sig/elastic_graph/schema_definition/schema_elements/scalar_type.rbs @@ -4,12 +4,13 @@ module ElasticGraph class ScalarTypeSuperClass attr_accessor schema_def_state: State attr_accessor type_ref: TypeReference + attr_accessor grouping_missing_value_placeholder_overridden: bool attr_accessor mapping_type: ::String attr_accessor runtime_metadata: SchemaArtifacts::RuntimeMetadata::ScalarType attr_accessor aggregated_values_customizations: (^(ObjectType) -> void)? attr_accessor filter_input_customizations: (^(InputType) -> void)? - def initialize: (State, TypeReference) -> void + def initialize: (State, TypeReference, bool) -> void end class ScalarType < ScalarTypeSuperClass @@ -24,6 +25,7 @@ module ElasticGraph def aggregated_values_type: () -> TypeReference def coerce_with: (::String, defined_at: ::String) -> void def prepare_for_indexing_with: (::String, defined_at: ::String) -> void + def grouping_missing_value_placeholder: (::String | ::Numeric) -> void def customize_aggregated_values_type: () { (ObjectType) -> void } -> void def customize_filter_input_type: () { (InputType) -> void } -> void @@ -41,8 +43,12 @@ module ElasticGraph def to_input_filters: () -> ::Array[SchemaElements::InputType] def to_aggregated_values_type: () -> SchemaElements::ObjectType? + def inferred_grouping_missing_value_placeholder: () -> ::String? + FLOAT_TYPES: ::Set[::String] + INTEGER_TYPES: ::Set[::String] NUMERIC_TYPES: ::Set[::String] + STRING_TYPES: ::Set[::String] DATE_TYPES: ::Set[::String] COMPARABLE_TYPES: ::Set[::String] diff --git a/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/graphql_schema/enum_type_spec.rb b/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/graphql_schema/enum_type_spec.rb index 415ae7c50..4154091c6 100644 --- a/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/graphql_schema/enum_type_spec.rb +++ b/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/graphql_schema/enum_type_spec.rb @@ -240,6 +240,19 @@ module SchemaDefinition }.to raise_invalid_graphql_name_error_for("INVALID.NAME") end + it "doesn't allow an enum value that conflicts with MISSING_ENUM_PLACEHOLDER" do + expect { + # The placeholder "**missing**" contains special characters that are not allowed + # in GraphQL enums, so it can never conflict with a valid enum value. + # This test documents that behavior. + define_schema do |api| + api.enum_type "Color" do |e| + e.value MISSING_ENUM_PLACEHOLDER + end + end + }.to raise_error(Errors::InvalidGraphQLNameError) + end + it "raises a clear error when the type name has the type wrapping characters" do expect { define_schema do |api| diff --git a/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/scalar_types_by_name_spec.rb b/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/scalar_types_by_name_spec.rb index 8cdf05d2b..0fe8ccb0b 100644 --- a/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/scalar_types_by_name_spec.rb +++ b/elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/runtime_metadata/scalar_types_by_name_spec.rb @@ -10,6 +10,10 @@ module ElasticGraph module SchemaDefinition + float_types = ElasticGraph::SchemaDefinition::SchemaElements::ScalarType::FLOAT_TYPES + string_types = ElasticGraph::SchemaDefinition::SchemaElements::ScalarType::STRING_TYPES + integer_types = ElasticGraph::SchemaDefinition::SchemaElements::ScalarType::INTEGER_TYPES + RSpec.describe "RuntimeMetadata #scalar_types_by_name" do include_context "RuntimeMetadata support" @@ -90,6 +94,217 @@ module SchemaDefinition }) end + describe "`grouping_missing_value_placeholder`" do + it "can be set to a number" do + grouping_missing_value_placeholder = grouping_missing_value_placeholder_for("long", type: "integer") do |t| + t.grouping_missing_value_placeholder(-1) + end + + expect(grouping_missing_value_placeholder).to eq(-1) + end + + it "can be set to a string" do + grouping_missing_value_placeholder = grouping_missing_value_placeholder_for("keyword", type: "string") do |t| + t.grouping_missing_value_placeholder "missing" + end + + expect(grouping_missing_value_placeholder).to eq("missing") + end + + it "does not infer placeholder when placeholder is set to nil" do + grouping_missing_value_placeholder = grouping_missing_value_placeholder_for("keyword", type: "string") do |t| + t.grouping_missing_value_placeholder nil + end + + expect(grouping_missing_value_placeholder).to be_nil + end + + it "raises an error when placeholder is not a String, Numeric, or nil" do + expect { + grouping_missing_value_placeholder_for("keyword", type: "string") do |t| + t.grouping_missing_value_placeholder :symbol + end + }.to raise_error Errors::SchemaError, a_string_including( + "grouping_missing_value_placeholder must be a String or Numeric value", + "got Symbol: :symbol" + ) + end + + it "raises an error when placeholder is an array" do + expect { + grouping_missing_value_placeholder_for("keyword", type: "string") do |t| + t.grouping_missing_value_placeholder ["invalid"] + end + }.to raise_error Errors::SchemaError, a_string_including( + "grouping_missing_value_placeholder must be a String or Numeric value", + "got Array: [\"invalid\"]" + ) + end + + it "raises an error when placeholder is a hash" do + expect { + grouping_missing_value_placeholder_for("keyword", type: "string") do |t| + t.grouping_missing_value_placeholder({key: "value"}) + end + }.to raise_error Errors::SchemaError, a_string_including( + "grouping_missing_value_placeholder must be a String or Numeric value", + "got Hash" + ) + end + + it "accepts integer values" do + grouping_missing_value_placeholder = grouping_missing_value_placeholder_for("long", type: "integer") do |t| + t.grouping_missing_value_placeholder 42 + end + + expect(grouping_missing_value_placeholder).to eq(42) + end + + it "accepts float values" do + grouping_missing_value_placeholder = grouping_missing_value_placeholder_for("double", type: "number") do |t| + t.grouping_missing_value_placeholder 3.14 + end + + expect(grouping_missing_value_placeholder).to eq(3.14) + end + + float_types.each do |float_type| + it "infers 'NaN' for float type #{float_type}" do + grouping_missing_value_placeholder = grouping_missing_value_placeholder_for(float_type, type: "number") + + expect(grouping_missing_value_placeholder).to eq(MISSING_NUMERIC_PLACEHOLDER) + end + end + + string_types.each do |string_type| + it "infers secure random string for string type #{string_type}" do + grouping_missing_value_placeholder = grouping_missing_value_placeholder_for(string_type, type: "string") + + expect(grouping_missing_value_placeholder).to eq(MISSING_STRING_PLACEHOLDER) + end + end + + integer_types.grep_v(/long/).each do |int_type| + it "infers 'NaN' for safe integer type #{int_type}" do + grouping_missing_value_placeholder = grouping_missing_value_placeholder_for(int_type, type: "integer") + + expect(grouping_missing_value_placeholder).to eq(MISSING_NUMERIC_PLACEHOLDER) + end + end + + it "infers 'NaN' for long types with JSON-safe min/max range" do + grouping_missing_value_placeholder = grouping_missing_value_placeholder_for("long", type: "integer", minimum: JSON_SAFE_LONG_MIN, maximum: JSON_SAFE_LONG_MAX) + + expect(grouping_missing_value_placeholder).to eq(MISSING_NUMERIC_PLACEHOLDER) + end + + it "does not infer a value for long types with max too large" do + grouping_missing_value_placeholder = grouping_missing_value_placeholder_for("long", type: "integer", minimum: -(2**53) + 1, maximum: (2**60) - 1) + + expect(grouping_missing_value_placeholder).to be_nil + end + + it "does not infer placeholder for long types with min too small" do + grouping_missing_value_placeholder = grouping_missing_value_placeholder_for("long", type: "integer", minimum: -(2**60), maximum: (2**53) - 1) + + expect(grouping_missing_value_placeholder).to be_nil + end + + it "does not infer placeholder for long types with only minimum specified" do + grouping_missing_value_placeholder = grouping_missing_value_placeholder_for("long", type: "integer", minimum: 0) + + expect(grouping_missing_value_placeholder).to be_nil + end + + it "does not infer placeholder for long types with only maximum specified" do + grouping_missing_value_placeholder = grouping_missing_value_placeholder_for("long", type: "integer", maximum: 1000) + + expect(grouping_missing_value_placeholder).to be_nil + end + + it "does not infer placeholder for long types without min/max specified" do + grouping_missing_value_placeholder = grouping_missing_value_placeholder_for("long", type: "integer") + + expect(grouping_missing_value_placeholder).to be_nil + end + + it "infers 'NaN' for unsigned_long types with safe maximum" do + grouping_missing_value_placeholder = grouping_missing_value_placeholder_for("unsigned_long", type: "integer", maximum: (2**53) - 1) + + expect(grouping_missing_value_placeholder).to eq(MISSING_NUMERIC_PLACEHOLDER) + end + + it "does not infer placeholder for unsigned_long types with unsafe maximum" do + grouping_missing_value_placeholder = grouping_missing_value_placeholder_for("unsigned_long", type: "integer", maximum: (2**60) - 1) + + expect(grouping_missing_value_placeholder).to be_nil + end + + it "does not infer placeholder for unsigned_long types without maximum specified" do + grouping_missing_value_placeholder = grouping_missing_value_placeholder_for("unsigned_long", type: "integer") + + expect(grouping_missing_value_placeholder).to be_nil + end + + describe "boundary conditions for JSON-safe long ranges" do + it "infers 'NaN' when exactly at safe boundaries" do + grouping_missing_value_placeholder = grouping_missing_value_placeholder_for("long", type: "integer", minimum: JSON_SAFE_LONG_MIN, maximum: JSON_SAFE_LONG_MAX) + + expect(grouping_missing_value_placeholder).to eq(MISSING_NUMERIC_PLACEHOLDER) + end + + it "does not infer placeholder when minimum is one below safe range" do + grouping_missing_value_placeholder = grouping_missing_value_placeholder_for("long", type: "integer", minimum: JSON_SAFE_LONG_MIN - 1, maximum: JSON_SAFE_LONG_MAX) + + expect(grouping_missing_value_placeholder).to be_nil + end + + it "does not infer placeholder when maximum is one above safe range" do + grouping_missing_value_placeholder = grouping_missing_value_placeholder_for("long", type: "integer", minimum: JSON_SAFE_LONG_MIN, maximum: JSON_SAFE_LONG_MAX + 1) + + expect(grouping_missing_value_placeholder).to be_nil + end + end + + it "has expected value for all built-in scalar types" do + results = define_schema + built_in_scalars = results.state.scalar_types_by_name.keys + scalar_types_by_name = results.runtime_metadata.scalar_types_by_name + + placeholders_by_scalar_type = built_in_scalars.to_h do |scalar_type| + [scalar_type, scalar_types_by_name.fetch(scalar_type).grouping_missing_value_placeholder] + end + + expect(placeholders_by_scalar_type).to eq({ + "Boolean" => nil, + "Cursor" => MISSING_STRING_PLACEHOLDER, + "Date" => nil, + "DateTime" => nil, + "Float" => MISSING_NUMERIC_PLACEHOLDER, + "ID" => MISSING_STRING_PLACEHOLDER, + "Int" => MISSING_NUMERIC_PLACEHOLDER, + "JsonSafeLong" => MISSING_NUMERIC_PLACEHOLDER, + "LocalTime" => nil, + "LongString" => nil, # outside of the JSON safe range. + "String" => MISSING_STRING_PLACEHOLDER, + "TimeZone" => MISSING_STRING_PLACEHOLDER, + "Untyped" => MISSING_STRING_PLACEHOLDER + }) + end + + def grouping_missing_value_placeholder_for(mapping_type, **json_schema) + metadata = scalar_type_metadata_for "CustomScalar" do |s| + s.scalar_type "CustomScalar" do |t| + t.mapping type: mapping_type + t.json_schema(**json_schema) + yield t if block_given? + end + end + + metadata.grouping_missing_value_placeholder + end + end + def scalar_type_metadata_for(name, &block) define_schema(&block) .runtime_metadata diff --git a/elasticgraph-support/lib/elastic_graph/constants.rb b/elasticgraph-support/lib/elastic_graph/constants.rb index 5d09bb9ed..79deb1d18 100644 --- a/elasticgraph-support/lib/elastic_graph/constants.rb +++ b/elasticgraph-support/lib/elastic_graph/constants.rb @@ -6,6 +6,8 @@ # # frozen_string_literal: true +require "securerandom" + # Root namespace for all ElasticGraph code. module ElasticGraph # Here we enumerate constants that are used from multiple places in the code. @@ -249,6 +251,33 @@ module ElasticGraph # @private VISIBILITY_PROFILE = :main + # Value used as the missing value placeholder for subaggregation on enum fields. + # This never collides with any real enum value since GraphQL enum values cannot contain `*` characters. + # @private + MISSING_ENUM_PLACEHOLDER = "**missing**" + + # This is a placeholder for the actual placeholder value used for string types (keyword, text). + # When a type is configured with this placeholder value then in the graphql query layer + # we use the random valued MISSING_STRING_PLACEHOLDER_VALUE instead. + # @private + MISSING_STRING_PLACEHOLDER = "$SECURE_RANDOM_VALUE" + + # A random 18 byte (24 character) base64 string used as the missing value placeholder + # for subaggregation on string fields. UUID are generally consider safe to use without + # risk of collision and since this has more random bits than a UUID it should have even + # less risk of collision. This uses a value generated randomly at runtime instead + # of using a hard-coded value in order to avoid intentional collision attacks. + # @private + MISSING_STRING_PLACEHOLDER_VALUE = SecureRandom.urlsafe_base64(18) + + # Value used as the missing value placeholder for subaggregation on numeric fields. + # This never collides with any numeric values in the datastore because JSON, ElasticSearch, + # and OpenSearch aren't capable of storing NaN. + # NaN works as a placeholder for missing numeric values, but it does have the side-effect of coercing + # integers to floats so we only use it on types limited to the the JsonSafeLong range. + # @private + MISSING_NUMERIC_PLACEHOLDER = "NaN" + # TODO(steep): it complains about `define_schema` not being defined but it is defined # in another file; I shouldn't have to say it's dynamic here. For now this works though. # @dynamic self.define_schema diff --git a/elasticgraph-support/sig/elastic_graph/constants.rbs b/elasticgraph-support/sig/elastic_graph/constants.rbs index 5ed04d784..77096bfde 100644 --- a/elasticgraph-support/sig/elastic_graph/constants.rbs +++ b/elasticgraph-support/sig/elastic_graph/constants.rbs @@ -36,4 +36,8 @@ module ElasticGraph DATASTORE_BULK_FILTER_PATH: ::String GRAPHQL_LAMBDA_AWS_ARN_HEADER: ::String VISIBILITY_PROFILE: ::Symbol + MISSING_ENUM_PLACEHOLDER: ::String + MISSING_STRING_PLACEHOLDER: ::String + MISSING_STRING_PLACEHOLDER_VALUE: ::String + MISSING_NUMERIC_PLACEHOLDER: ::String end diff --git a/elasticgraph-support/spec/unit/elastic_graph/constants_spec.rb b/elasticgraph-support/spec/unit/elastic_graph/constants_spec.rb index de2a1f5f5..87fcae686 100644 --- a/elasticgraph-support/spec/unit/elastic_graph/constants_spec.rb +++ b/elasticgraph-support/spec/unit/elastic_graph/constants_spec.rb @@ -17,5 +17,14 @@ module ElasticGraph expect(encoded_data).to eq({"uuid" => "dca02d20-baee-4ee9-a027-feece0a6de3a"}) end + + specify "MISSING_STRING_PLACEHOLDER_VALUE is a 24-character string (18 bytes encoded as base64)" do + # Should be a 24-character string (18 bytes encoded as base64) + expect(MISSING_STRING_PLACEHOLDER_VALUE).to be_a(String) + expect(MISSING_STRING_PLACEHOLDER_VALUE.length).to eq(24) + + # Should be URL-safe base64 (no +, /, or = characters) + expect(MISSING_STRING_PLACEHOLDER_VALUE).to match(/\A[A-Za-z0-9_-]+\z/) + end end end