Phase 3: Wire up grouping_missing_value_placeholder in GraphQL layer - #894
Merged
Conversation
mmarston
requested review from
BrianSigafoos-SQ,
ayousufi,
bsorbo,
jwils,
jwondrusch,
lancethomps,
myronmarston,
nicolewoch,
thomasmahoney and
zachbutler-squareup
as code owners
October 30, 2025 15:46
This was referenced Oct 30, 2025
mmarston
force-pushed
the
mattmarston/phase-2-schema-definition
branch
2 times, most recently
from
October 31, 2025 04:47
e2d75f0 to
b38b7ef
Compare
mmarston
force-pushed
the
mattmarston/phase-3-graphql-wiring
branch
from
October 31, 2025 04:48
32ad6e5 to
4319ee7
Compare
myronmarston
requested changes
Oct 31, 2025
mmarston
force-pushed
the
mattmarston/phase-2-schema-definition
branch
from
October 31, 2025 15:16
b38b7ef to
50d7a79
Compare
Base automatically changed from
mattmarston/phase-2-schema-definition
to
main
October 31, 2025 16:26
mmarston
force-pushed
the
mattmarston/phase-3-graphql-wiring
branch
from
October 31, 2025 16:39
4319ee7 to
61eeb79
Compare
This commit makes the grouping_missing_value_placeholder attribute available from ElasticGraph::GraphQL::Schema::Type by wiring it through from runtime metadata for both scalar and enum types. Changes: - Add scalar_runtime_metadata as required positional parameter to Type.initialize - Add grouping_missing_value_placeholder method to Type that checks both scalar and enum runtime metadata - Update GraphQL::Schema to pass scalar runtime metadata when creating Type instances - Add tests for the new functionality
mmarston
force-pushed
the
mattmarston/phase-3-graphql-wiring
branch
from
October 31, 2025 18:44
61eeb79 to
f7ece77
Compare
Contributor
Author
|
All comments have been addressed. |
myronmarston
approved these changes
Oct 31, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is 3rd in a series of new pull requests that are working toward a new approach to handling missing values in subaggregations as described in #882. For an outline of the steps see the comment from @myronmarston here. For context see the first PR in the sequence,
#890. The 2nd PR is #1 (I need to move these PRs to block/elasticgraph).
This replaces the Phase 3, mattmarston/elasticgraph#2 PR that was in my forked repo. This PR addresses the comments there.
This commit makes the grouping_missing_value_placeholder attribute available from ElasticGraph::GraphQL::Schema::Type by wiring it through from runtime metadata for both scalar and enum types.
Changes:
Other PRs
Phase 1: Add grouping_missing_value_placeholder runtime metadata
Phase 2: Infer grouping_missing_value_placeholder based on mapping_type
Phase 3: Wire up grouping_missing_value_placeholder in GraphQL layer (this PR)
Phase 4: Use grouping_missing_value_placeholder in aggregation logic