Skip to content

Phase 2: Infer grouping_missing_value_placeholder based on mapping_type #893

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
myronmarston merged 1 commit into from
Oct 31, 2025
Merged

Phase 2: Infer grouping_missing_value_placeholder based on mapping_type #893

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions config/schema/artifacts/runtime_metadata.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -7735,27 +7736,31 @@ 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
ID:
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
Int:
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
JsonSafeLong:
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
Expand All @@ -7777,20 +7782,23 @@ 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
TimeZone:
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
Untyped:
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
Expand Down
8 changes: 8 additions & 0 deletions config/schema/artifacts_with_apollo/runtime_metadata.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -7879,27 +7880,31 @@ 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
ID:
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
Int:
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
JsonSafeLong:
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
Expand All @@ -7921,20 +7926,23 @@ 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
TimeZone:
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
Untyped:
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
Expand Down
6 changes: 3 additions & 3 deletions ...raph-schema_artifacts/sig/elastic_graph/schema_artifacts/runtime_metadata/scalar_type.rbs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
70 changes: 68 additions & 2 deletions ...raph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/scalar_type.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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(
Expand All @@ -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
Expand Down Expand Up @@ -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}"
Expand Down Expand Up @@ -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.

@myronmarston myronmarston Oct 30, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two things here:

  • How did you this out? Is it documented somewhere? (If so, can you link to those docs here?)
  • It's worth noting that the presence of the JSON schema min/max do not guarantee that all indexed values are within the specified range. It guarantees that newly indexed values will be in that range, but it's possible that the JSON schema min/max were added after data started being indexed, and some values could already be in the index which are outside the specified range.
    • However, this is very much an edge case. My position is that if someone runs into this case, that's a time when they can override the grouping_missing_value_placeholder (maybe the the first concrete use case I've thought of for doing so).
    • In practice, I don't expect ElasticGraph users to be defining custom long types--EG already offers JsonSafeLong and LongString and most users are likely to use those, which already have and enforce the JSON schema min/max.
    • Regardless, maybe worth acknowledging via a comment that this can happen and that grouping_missing_value_placeholder can be forced to nil in that case? That way there's a record that we considered it.
    • ...I am curious what you think about this.

@mmarston mmarston Oct 31, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How did you this out? Is it documented somewhere? (If so, can you link to those docs here?)

Unfortunately I'm not aware of any ES/OS documentation for the behavior. I know that using "NaN" works for integer types and I've tested that end-to-end. I have examples documented here in the phase 4 PR.

I'm now not certain about the bucket keys being coerced to floats, so it is possible that the range checks are unnecessary. However, I'd rather keep the range checks in for now to be safe. The alternative is that I completely remove missing value placeholder support for all integer mapping types (including long).

I first came to the inference that ints were getting coerced to floats when I was working on phase 4 and found that to get some tests to pass I needed to change some bucket keys to add .0, e.g. from 2022 to 2022.0. See this comment.

But when I've tested directly calling OpenSearch/ElasticSearch and when I've queried using GraphQL (in phase 4) I see integer output, not floats. So I'm not sure what is going on with the integration tests I noted above.

It's worth noting that the presence of the JSON schema min/max do not guarantee that all indexed values are within the specified range....
... maybe worth acknowledging via a comment that this can happen

I've noted this in the code comment.

@myronmarston myronmarston Oct 31, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, that's helpful context.

One suggestion: to help see what's happening, you can run tests with DEBUG_QUERY=1. When that ENV var is present it'll print the ES/OS request and response so you can more easily see what's going on.

# 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
Expand Down
8 changes: 7 additions & 1 deletion ...aph-schema_definition/sig/elastic_graph/schema_definition/schema_elements/scalar_type.rbs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand All @@ -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]

Expand Down
13 changes: 13 additions & 0 deletions ...ema_definition/spec/unit/elastic_graph/schema_definition/graphql_schema/enum_type_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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|
Expand Down
Loading