From 267b3945f7fdc78980f17cc5df924042d7bf2833 Mon Sep 17 00:00:00 2001 From: Jens Neuse Date: Mon, 9 Feb 2026 14:28:33 +0100 Subject: [PATCH 1/9] fix: allow differing nullability on fields in non-overlapping concrete types When inline fragments target different concrete object types (e.g. User vs Organization), fields with the same name but different nullability (String! vs String) are safe because only one branch ever resolves for a given object. Previously the validator rejected these queries unconditionally. Add TypesAreCompatibleIgnoringNullability to ast_type.go and use it as a fallback when enclosing types cannot overlap (potentiallySameObject returns false). Strict checking is preserved when an interface is involved or the enclosing types are the same. Co-Authored-By: Claude Opus 4.6 --- v2/pkg/ast/ast_type.go | 50 +++++++ .../operation_rule_field_selection_merging.go | 56 +++++--- .../operation_validation_test.go | 68 +++++++++ .../graphql_datasource_test.go | 130 ++++++++++++++++++ v2/pkg/engine/resolve/resolve_test.go | 85 ++++++++++++ 5 files changed, 369 insertions(+), 20 deletions(-) diff --git a/v2/pkg/ast/ast_type.go b/v2/pkg/ast/ast_type.go index 92aff65a7b..17284cf25c 100644 --- a/v2/pkg/ast/ast_type.go +++ b/v2/pkg/ast/ast_type.go @@ -241,6 +241,56 @@ func (d *Document) TypesAreCompatibleDeep(left int, right int) bool { } } +// TypesAreCompatibleIgnoringNullability is a relaxed variant of TypesAreCompatibleDeep +// that strips NonNull wrappers at every nesting level but still requires the same list +// structure and the same base named type. This implements the spec's "SameResponseShape" +// semantics for fields on non-overlapping parent types (e.g. two different concrete object +// types in inline fragments that can never apply to the same runtime object). +func (d *Document) TypesAreCompatibleIgnoringNullability(left int, right int) bool { + for { + if left == -1 || right == -1 { + return false + } + // Strip NonNull wrappers from both sides + if d.Types[left].TypeKind == TypeKindNonNull { + left = d.Types[left].OfType + } + if d.Types[right].TypeKind == TypeKindNonNull { + right = d.Types[right].OfType + } + if d.Types[left].TypeKind != d.Types[right].TypeKind { + return false + } + if d.Types[left].TypeKind == TypeKindNamed { + leftName := d.TypeNameBytes(left) + rightName := d.TypeNameBytes(right) + if bytes.Equal(leftName, rightName) { + return true + } + leftNode, _ := d.Index.FirstNodeByNameBytes(leftName) + rightNode, _ := d.Index.FirstNodeByNameBytes(rightName) + if leftNode.Kind == rightNode.Kind { + return false + } + if leftNode.Kind == NodeKindInterfaceTypeDefinition && rightNode.Kind == NodeKindObjectTypeDefinition { + return d.NodeImplementsInterfaceFields(rightNode, leftNode) + } + if leftNode.Kind == NodeKindObjectTypeDefinition && rightNode.Kind == NodeKindInterfaceTypeDefinition { + return d.NodeImplementsInterfaceFields(leftNode, rightNode) + } + if leftNode.Kind == NodeKindUnionTypeDefinition && rightNode.Kind == NodeKindObjectTypeDefinition { + return d.NodeIsUnionMember(rightNode, leftNode) + } + if leftNode.Kind == NodeKindObjectTypeDefinition && rightNode.Kind == NodeKindUnionTypeDefinition { + return d.NodeIsUnionMember(leftNode, rightNode) + } + return false + } + left = d.Types[left].OfType + right = d.Types[right].OfType + } +} + func (d *Document) ResolveTypeNameBytes(ref int) ByteSlice { resolvedTypeRef := d.ResolveUnderlyingType(ref) return d.TypeNameBytes(resolvedTypeRef) diff --git a/v2/pkg/astvalidation/operation_rule_field_selection_merging.go b/v2/pkg/astvalidation/operation_rule_field_selection_merging.go index 9e762aa32c..51342d4a71 100644 --- a/v2/pkg/astvalidation/operation_rule_field_selection_merging.go +++ b/v2/pkg/astvalidation/operation_rule_field_selection_merging.go @@ -33,6 +33,7 @@ type nonScalarRequirement struct { objectName ast.ByteSlice fieldTypeRef int fieldTypeDefinitionNode ast.Node + enclosingTypeDefinition ast.Node } type nonScalarRequirements []nonScalarRequirement @@ -116,18 +117,25 @@ func (f *fieldSelectionMergingVisitor) EnterField(ref int) { return } } else if !f.definition.TypesAreCompatibleDeep(f.nonScalarRequirements[i].fieldTypeRef, fieldType) { - left, err := f.definition.PrintTypeBytes(f.nonScalarRequirements[i].fieldTypeRef, nil) - if err != nil { - f.StopWithInternalErr(err) + // When enclosing types cannot overlap (e.g. two different concrete object types), + // allow nullability differences as long as the base types match + if !f.potentiallySameObject(f.nonScalarRequirements[i].enclosingTypeDefinition, f.EnclosingTypeDefinition) && + f.definition.TypesAreCompatibleIgnoringNullability(f.nonScalarRequirements[i].fieldTypeRef, fieldType) { + // Different nullability on non-overlapping types is safe + } else { + left, err := f.definition.PrintTypeBytes(f.nonScalarRequirements[i].fieldTypeRef, nil) + if err != nil { + f.StopWithInternalErr(err) + return + } + right, err := f.definition.PrintTypeBytes(fieldType, nil) + if err != nil { + f.StopWithInternalErr(err) + return + } + f.StopWithExternalErr(operationreport.ErrTypesForFieldMismatch(objectName, left, right)) return } - right, err := f.definition.PrintTypeBytes(fieldType, nil) - if err != nil { - f.StopWithInternalErr(err) - return - } - f.StopWithExternalErr(operationreport.ErrTypesForFieldMismatch(objectName, left, right)) - return } if fieldDefinitionTypeNode.Kind != f.nonScalarRequirements[i].fieldTypeDefinitionNode.Kind { @@ -144,6 +152,7 @@ func (f *fieldSelectionMergingVisitor) EnterField(ref int) { objectName: objectName, fieldTypeRef: fieldType, fieldTypeDefinitionNode: fieldDefinitionTypeNode, + enclosingTypeDefinition: f.EnclosingTypeDefinition, }) return } @@ -160,18 +169,25 @@ func (f *fieldSelectionMergingVisitor) EnterField(ref int) { } } if !f.definition.TypesAreCompatibleDeep(f.scalarRequirements[i].fieldType, fieldType) { - left, err := f.definition.PrintTypeBytes(f.scalarRequirements[i].fieldType, nil) - if err != nil { - f.StopWithInternalErr(err) - return - } - right, err := f.definition.PrintTypeBytes(fieldType, nil) - if err != nil { - f.StopWithInternalErr(err) + // When enclosing types cannot overlap (e.g. two different concrete object types), + // allow nullability differences as long as the base types match + if !f.potentiallySameObject(f.scalarRequirements[i].enclosingTypeDefinition, f.EnclosingTypeDefinition) && + f.definition.TypesAreCompatibleIgnoringNullability(f.scalarRequirements[i].fieldType, fieldType) { + // Different nullability on non-overlapping types is safe + } else { + left, err := f.definition.PrintTypeBytes(f.scalarRequirements[i].fieldType, nil) + if err != nil { + f.StopWithInternalErr(err) + return + } + right, err := f.definition.PrintTypeBytes(fieldType, nil) + if err != nil { + f.StopWithInternalErr(err) + return + } + f.StopWithExternalErr(operationreport.ErrFieldsConflict(objectName, left, right)) return } - f.StopWithExternalErr(operationreport.ErrFieldsConflict(objectName, left, right)) - return } if fieldDefinitionTypeNode.Kind != f.scalarRequirements[i].fieldTypeDefinitionNode.Kind { diff --git a/v2/pkg/astvalidation/operation_validation_test.go b/v2/pkg/astvalidation/operation_validation_test.go index ae8ea3580e..94d7acdd62 100644 --- a/v2/pkg/astvalidation/operation_validation_test.go +++ b/v2/pkg/astvalidation/operation_validation_test.go @@ -1077,6 +1077,38 @@ func TestExecutionValidation(t *testing.T) { } }`, FieldSelectionMerging(), Valid) }) + t.Run("allows differing scalar nullability on non-overlapping object types", func(t *testing.T) { + runWithDefinition(t, entityDefinition, ` + { + entity { + ... on User { email } + ... on Organization { email } + } + }`, FieldSelectionMerging(), Valid) + }) + t.Run("allows differing non-scalar nullability on non-overlapping object types", func(t *testing.T) { + runWithDefinition(t, entityDefinition, ` + { + entity { + ... on User { profile { name } } + ... on Organization { profile { name } } + } + }`, FieldSelectionMerging(), Valid) + }) + t.Run("disallows differing return type nullability when interface could overlap", func(t *testing.T) { + runWithDefinition(t, boxDefinition, ` + { + someBox { + ... on NonNullStringBox1 { + scalar + } + ... on StringBox { + scalar + } + } + }`, FieldSelectionMerging(), Invalid, + withValidationErrors(`fields 'scalar' conflict because they return conflicting types 'String!' and 'String'`)) + }) t.Run("same wrapped scalar return types", func(t *testing.T) { runWithDefinition(t, boxDefinition, ` { @@ -5646,6 +5678,42 @@ schema { query: Query }` +const entityDefinition = ` +scalar String +scalar Int +scalar ID + +interface Node { + id: ID! +} + +type User implements Node { + id: ID! + email: String! + profile: Profile! +} + +type Organization implements Node { + id: ID! + email: String + profile: Profile +} + +type Profile { + name: String +} + +union Entity = User | Organization + +type Query { + entity: Entity + node: Node +} + +schema { + query: Query +}` + const countriesDefinition = `directive @cacheControl(maxAge: Int, scope: CacheControlScope) on FIELD_DEFINITION | OBJECT | INTERFACE scalar String diff --git a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_test.go b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_test.go index e0e6d29b38..e8f3140f09 100644 --- a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_test.go +++ b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_test.go @@ -8231,6 +8231,136 @@ func TestGraphQLDataSource(t *testing.T) { }, DisableResolveFieldPositions: true, }, WithDefaultPostProcessor())) + + t.Run("inline fragments on non-overlapping types with different field nullability", func(t *testing.T) { + definition := ` + scalar String + scalar Int + scalar ID + + interface Node { + id: ID! + } + + type User implements Node { + id: ID! + email: String! + profile: Profile! + } + + type Organization implements Node { + id: ID! + email: String + profile: Profile + } + + type Profile { + name: String + } + + union Entity = User | Organization + + type Query { + entity: Entity + } + + schema { + query: Query + } + ` + + t.Run("run", RunTest(definition, ` + query MyQuery { + entity { + ... on User { email } + ... on Organization { email } + } + }`, + "MyQuery", + &plan.SynchronousResponsePlan{ + Response: &resolve.GraphQLResponse{ + Fetches: resolve.Sequence( + resolve.Single(&resolve.SingleFetch{ + FetchConfiguration: resolve.FetchConfiguration{ + DataSource: &Source{}, + Input: `{"method":"POST","url":"https://example.com/graphql","body":{"query":"{entity {__typename ... on User {email} ... on Organization {email}}}"}}`, + PostProcessing: DefaultPostProcessingConfiguration, + }, + DataSourceIdentifier: []byte("graphql_datasource.Source"), + })), + Data: &resolve.Object{ + Fields: []*resolve.Field{ + { + Name: []byte("entity"), + Value: &resolve.Object{ + Path: []string{"entity"}, + Nullable: true, + PossibleTypes: map[string]struct{}{ + "User": {}, + "Organization": {}, + }, + TypeName: "Entity", + Fields: []*resolve.Field{ + { + Name: []byte("email"), + Value: &resolve.String{ + Path: []string{"email"}, + }, + OnTypeNames: [][]byte{[]byte("User")}, + }, + { + Name: []byte("email"), + Value: &resolve.String{ + Path: []string{"email"}, + Nullable: true, + }, + OnTypeNames: [][]byte{[]byte("Organization")}, + }, + }, + }, + }, + }, + }, + }, + }, plan.Configuration{ + DataSources: []plan.DataSource{ + mustDataSourceConfiguration( + t, + "ds-id", + &plan.DataSourceMetadata{ + RootNodes: []plan.TypeField{ + { + TypeName: "Query", + FieldNames: []string{"entity"}, + }, + }, + ChildNodes: []plan.TypeField{ + { + TypeName: "User", + FieldNames: []string{"id", "email", "profile"}, + }, + { + TypeName: "Organization", + FieldNames: []string{"id", "email", "profile"}, + }, + { + TypeName: "Profile", + FieldNames: []string{"name"}, + }, + }, + }, + mustCustomConfiguration(t, ConfigurationInput{ + Fetch: &FetchConfiguration{ + URL: "https://example.com/graphql", + }, + SchemaConfiguration: mustSchema(t, nil, definition), + }), + ), + }, + DisableResolveFieldPositions: true, + }, WithDefaultPostProcessor(), + )) + }) } var errSubscriptionClientFail = errors.New("subscription client fail error") diff --git a/v2/pkg/engine/resolve/resolve_test.go b/v2/pkg/engine/resolve/resolve_test.go index 268c7ce565..fda6f60806 100644 --- a/v2/pkg/engine/resolve/resolve_test.go +++ b/v2/pkg/engine/resolve/resolve_test.go @@ -892,6 +892,91 @@ func TestResolver_ResolveNode(t *testing.T) { }, *NewContext(context.Background()), `{"data":{"pets":[{"name":"Woofie"},{}]}}` })) + t.Run("resolve fields with differing nullability on non-overlapping types", testFn(false, func(t *testing.T, ctrl *gomock.Controller) (response *GraphQLResponse, ctx Context, expectedOutput string) { + // This test verifies that the resolver correctly handles fields where the same field name + // has different nullability on non-overlapping concrete types (e.g. User.email: String! vs Organization.email: String). + // A User object should resolve email as non-null, and an Organization object should resolve email as nullable. + return &GraphQLResponse{ + Fetches: Single(&SingleFetch{ + FetchConfiguration: FetchConfiguration{DataSource: FakeDataSource(`{"entity":{"__typename":"User","email":"user@example.com"}}`)}, + }), + Data: &Object{ + Fields: []*Field{ + { + Name: []byte("entity"), + Value: &Object{ + Path: []string{"entity"}, + PossibleTypes: map[string]struct{}{ + "User": {}, + "Organization": {}, + }, + TypeName: "Entity", + Fields: []*Field{ + { + OnTypeNames: [][]byte{[]byte("User")}, + Name: []byte("email"), + Value: &String{ + Path: []string{"email"}, + }, + }, + { + OnTypeNames: [][]byte{[]byte("Organization")}, + Name: []byte("email"), + Value: &String{ + Path: []string{"email"}, + Nullable: true, + }, + }, + }, + }, + }, + }, + }, + }, *NewContext(context.Background()), + `{"data":{"entity":{"email":"user@example.com"}}}` + })) + t.Run("resolve fields with differing nullability on non-overlapping types - nullable variant", testFn(false, func(t *testing.T, ctrl *gomock.Controller) (response *GraphQLResponse, ctx Context, expectedOutput string) { + // Same scenario but the runtime object is Organization (nullable email) with a null email value + return &GraphQLResponse{ + Fetches: Single(&SingleFetch{ + FetchConfiguration: FetchConfiguration{DataSource: FakeDataSource(`{"entity":{"__typename":"Organization","email":null}}`)}, + }), + Data: &Object{ + Fields: []*Field{ + { + Name: []byte("entity"), + Value: &Object{ + Path: []string{"entity"}, + PossibleTypes: map[string]struct{}{ + "User": {}, + "Organization": {}, + }, + TypeName: "Entity", + Fields: []*Field{ + { + OnTypeNames: [][]byte{[]byte("User")}, + Name: []byte("email"), + Value: &String{ + Path: []string{"email"}, + }, + }, + { + OnTypeNames: [][]byte{[]byte("Organization")}, + Name: []byte("email"), + Value: &String{ + Path: []string{"email"}, + Nullable: true, + }, + }, + }, + }, + }, + }, + }, + }, *NewContext(context.Background()), + `{"data":{"entity":{"email":null}}}` + })) + t.Run("with unescape json enabled", func(t *testing.T) { t.Run("json object within a string", testFn(false, func(t *testing.T, ctrl *gomock.Controller) (response *GraphQLResponse, ctx Context, expectedOutput string) { return &GraphQLResponse{ From 5e5d6e18557c1a16fefe888f18d57efd2ac9ea65 Mon Sep 17 00:00:00 2001 From: Jens Neuse Date: Mon, 9 Feb 2026 15:35:06 +0100 Subject: [PATCH 2/9] feat: put relaxed nullability validation behind opt-in feature flag The nullability relaxation for field selection merging on non-overlapping concrete types is now gated behind a feature flag instead of being always active. This defaults to strict spec-compliant behavior. Consumers opt in via: - plan.Configuration.RelaxSubgraphOperationFieldSelectionMergingNullability (for upstream/subgraph operation validation) - astvalidation.WithRelaxFieldSelectionMergingNullability() (for user-facing operation validation) The plan package injects the flag into factories that implement the SubgraphFieldSelectionMergingNullabilityRelaxer interface, keeping the plan package decoupled from astvalidation. Co-Authored-By: Claude Opus 4.6 --- v2/pkg/ast/ast_type.go | 65 +++---- v2/pkg/ast/ast_type_test.go | 169 ++++++++++++++++++ .../nullability_relaxation_justification.md | 151 ++++++++++++++++ .../operation_rule_field_selection_merging.go | 40 +++-- v2/pkg/astvalidation/operation_validation.go | 11 +- .../operation_validation_test.go | 24 ++- .../graphql_datasource/graphql_datasource.go | 43 +++-- .../graphql_datasource_test.go | 5 +- .../datasourcetesting/datasourcetesting.go | 9 +- v2/pkg/engine/plan/configuration.go | 7 + .../engine/plan/datasource_configuration.go | 14 ++ 11 files changed, 463 insertions(+), 75 deletions(-) create mode 100644 v2/pkg/astvalidation/nullability_relaxation_justification.md diff --git a/v2/pkg/ast/ast_type.go b/v2/pkg/ast/ast_type.go index 17284cf25c..0164b6fa5e 100644 --- a/v2/pkg/ast/ast_type.go +++ b/v2/pkg/ast/ast_type.go @@ -204,59 +204,36 @@ func (d *Document) TypeNumberOfListWraps(ref int) int { } func (d *Document) TypesAreCompatibleDeep(left int, right int) bool { - for { - if left == -1 || right == -1 { - return false - } - if d.Types[left].TypeKind != d.Types[right].TypeKind { - return false - } - if d.Types[left].TypeKind == TypeKindNamed { - leftName := d.TypeNameBytes(left) - rightName := d.TypeNameBytes(right) - if bytes.Equal(leftName, rightName) { - return true - } - leftNode, _ := d.Index.FirstNodeByNameBytes(leftName) - rightNode, _ := d.Index.FirstNodeByNameBytes(rightName) - if leftNode.Kind == rightNode.Kind { - return false - } - if leftNode.Kind == NodeKindInterfaceTypeDefinition && rightNode.Kind == NodeKindObjectTypeDefinition { - return d.NodeImplementsInterfaceFields(rightNode, leftNode) - } - if leftNode.Kind == NodeKindObjectTypeDefinition && rightNode.Kind == NodeKindInterfaceTypeDefinition { - return d.NodeImplementsInterfaceFields(leftNode, rightNode) - } - if leftNode.Kind == NodeKindUnionTypeDefinition && rightNode.Kind == NodeKindObjectTypeDefinition { - return d.NodeIsUnionMember(rightNode, leftNode) - } - if leftNode.Kind == NodeKindObjectTypeDefinition && rightNode.Kind == NodeKindUnionTypeDefinition { - return d.NodeIsUnionMember(leftNode, rightNode) - } - return false - } - left = d.Types[left].OfType - right = d.Types[right].OfType - } + return d.typesAreCompatible(left, right, false) } // TypesAreCompatibleIgnoringNullability is a relaxed variant of TypesAreCompatibleDeep // that strips NonNull wrappers at every nesting level but still requires the same list -// structure and the same base named type. This implements the spec's "SameResponseShape" -// semantics for fields on non-overlapping parent types (e.g. two different concrete object -// types in inline fragments that can never apply to the same runtime object). +// structure and the same base named type. +// +// Use this only when the two fields' enclosing types cannot overlap at runtime +// (i.e. potentiallySameObject returns false), for example two distinct concrete +// object types inside inline fragments on the same union/interface field. In that +// case the spec's SameResponseShape algorithm permits differing nullability because +// only one branch can ever execute for a given runtime object. +// +// See https://spec.graphql.org/October2021/#SameResponseShape() func (d *Document) TypesAreCompatibleIgnoringNullability(left int, right int) bool { + return d.typesAreCompatible(left, right, true) +} + +func (d *Document) typesAreCompatible(left int, right int, ignoreNullability bool) bool { for { if left == -1 || right == -1 { return false } - // Strip NonNull wrappers from both sides - if d.Types[left].TypeKind == TypeKindNonNull { - left = d.Types[left].OfType - } - if d.Types[right].TypeKind == TypeKindNonNull { - right = d.Types[right].OfType + if ignoreNullability { + if d.Types[left].TypeKind == TypeKindNonNull { + left = d.Types[left].OfType + } + if d.Types[right].TypeKind == TypeKindNonNull { + right = d.Types[right].OfType + } } if d.Types[left].TypeKind != d.Types[right].TypeKind { return false diff --git a/v2/pkg/ast/ast_type_test.go b/v2/pkg/ast/ast_type_test.go index bd41296310..c341b545fe 100644 --- a/v2/pkg/ast/ast_type_test.go +++ b/v2/pkg/ast/ast_type_test.go @@ -1 +1,170 @@ package ast + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestDocument_TypesAreCompatibleIgnoringNullability(t *testing.T) { + // Build a document with various type refs to test against. + // We create types programmatically using the Add* methods. + doc := NewSmallDocument() + + // Named types + stringRef := doc.AddNamedType([]byte("String")) // String + intRef := doc.AddNamedType([]byte("Int")) // Int + stringRef2 := doc.AddNamedType([]byte("String")) // String (second ref) + profileRef := doc.AddNamedType([]byte("Profile")) // Profile + profileRef2 := doc.AddNamedType([]byte("Profile")) // Profile (second ref) + + // NonNull named types + stringNonNullRef := doc.AddNonNullNamedType([]byte("String")) // String! + intNonNullRef := doc.AddNonNullNamedType([]byte("Int")) // Int! + profileNonNullRef := doc.AddNonNullNamedType([]byte("Profile")) // Profile! + + // List types + listStringRef := doc.AddListType(doc.AddNamedType([]byte("String"))) // [String] + listStringNonNullRef := doc.AddListType(doc.AddNonNullNamedType([]byte("String"))) // [String!] + + // NonNull list types + nonNullListStringRef := doc.AddNonNullType(doc.AddListType(doc.AddNamedType([]byte("String")))) // [String]! + nonNullListStringNonNullRef := doc.AddNonNullType(doc.AddListType(doc.AddNonNullNamedType([]byte("String")))) // [String!]! + + tests := []struct { + name string + left int + right int + deep bool // expected result from TypesAreCompatibleDeep + relaxed bool // expected result from TypesAreCompatibleIgnoringNullability + }{ + { + name: "String vs String - identical", + left: stringRef, + right: stringRef2, + deep: true, + relaxed: true, + }, + { + name: "String! vs String - nullability differs", + left: stringNonNullRef, + right: stringRef, + deep: false, + relaxed: true, + }, + { + name: "String vs String! - nullability differs (reversed)", + left: stringRef, + right: stringNonNullRef, + deep: false, + relaxed: true, + }, + { + name: "String! vs Int! - different base types", + left: stringNonNullRef, + right: intNonNullRef, + deep: false, + relaxed: false, + }, + { + name: "String vs Int - different base types", + left: stringRef, + right: intRef, + deep: false, + relaxed: false, + }, + { + name: "String! vs Int - different base types, different nullability", + left: stringNonNullRef, + right: intRef, + deep: false, + relaxed: false, + }, + { + name: "Profile! vs Profile - non-scalar nullability differs", + left: profileNonNullRef, + right: profileRef, + deep: false, + relaxed: true, + }, + { + name: "Profile vs Profile - identical", + left: profileRef, + right: profileRef2, + deep: true, + relaxed: true, + }, + { + name: "[String] vs [String] - identical lists", + left: listStringRef, + right: listStringRef, + deep: true, + relaxed: true, + }, + { + name: "[String!] vs [String] - inner nullability differs", + left: listStringNonNullRef, + right: listStringRef, + deep: false, + relaxed: true, + }, + { + name: "[String]! vs [String] - outer nullability differs", + left: nonNullListStringRef, + right: listStringRef, + deep: false, + relaxed: true, + }, + { + name: "[String!]! vs [String] - both levels differ", + left: nonNullListStringNonNullRef, + right: listStringRef, + deep: false, + relaxed: true, + }, + { + name: "[String!]! vs [String!] - only outer differs", + left: nonNullListStringNonNullRef, + right: listStringNonNullRef, + deep: false, + relaxed: true, + }, + { + name: "[String] vs String - list vs non-list", + left: listStringRef, + right: stringRef, + deep: false, + relaxed: false, + }, + { + name: "[String]! vs String! - list vs non-list with NonNull", + left: nonNullListStringRef, + right: stringNonNullRef, + deep: false, + relaxed: false, + }, + { + name: "invalid ref left", + left: -1, + right: stringRef, + deep: false, + relaxed: false, + }, + { + name: "invalid ref right", + left: stringRef, + right: -1, + deep: false, + relaxed: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.deep, doc.TypesAreCompatibleDeep(tt.left, tt.right), + "TypesAreCompatibleDeep") + assert.Equal(t, tt.relaxed, doc.TypesAreCompatibleIgnoringNullability(tt.left, tt.right), + "TypesAreCompatibleIgnoringNullability") + }) + } +} diff --git a/v2/pkg/astvalidation/nullability_relaxation_justification.md b/v2/pkg/astvalidation/nullability_relaxation_justification.md new file mode 100644 index 0000000000..8233b60856 --- /dev/null +++ b/v2/pkg/astvalidation/nullability_relaxation_justification.md @@ -0,0 +1,151 @@ +# Justification: Relaxed Nullability Validation on Non-Overlapping Concrete Types + +## Problem Statement + +The Router rejects valid-in-practice queries where inline fragments on different +concrete union/interface member types select the same field with different +nullability: + +```graphql +# Schema: +union Entity = User | Organization +type User { email: String! } +type Organization { email: String } + +# Query (rejected): +{ + entity { + ... on User { email } + ... on Organization { email } + } +} +# Error: "fields 'email' conflict because they return conflicting types 'String!' and 'String'" +``` + +## What the Spec Says + +The GraphQL spec ([sec 5.3.2 "Field Selection Merging"](https://spec.graphql.org/October2021/#sec-Field-Selection-Merging)) +defines two algorithms: + +1. **FieldsInSetCanMerge**: For each pair of fields with the same response name, + calls `SameResponseShape`. If parent types are the same (or either is not an + Object Type), also checks that field names and arguments match. + +2. **SameResponseShape**: Checks structural type compatibility. Step 2 states: + "If typeA or typeB is Non-Null: if the other is nullable, return false." + This check runs **unconditionally** — it has no concept of parent type + mutual exclusivity. + +The `FieldsInSetCanMerge` algorithm *does* have a concept of mutual exclusivity +(step 3: "if parentType1 is not equal to parentType2, and both are Object Types"), +but this flag **only** relaxes the field-name and argument-equality checks — not +the `SameResponseShape` check. + +**Per the literal spec text, our query is invalid.** + +## What the Reference Implementation Does + +The [graphql-js reference implementation](https://github.com/graphql/graphql-js/blob/main/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts) +mirrors the spec exactly. The `doTypesConflict` function runs **outside** the +`!areMutuallyExclusive` guard: + +```typescript +// In findConflict(): +if (!areMutuallyExclusive) { + // Only check field name + argument equality when types could overlap +} + +// TYPE CHECK — ALWAYS RUNS, regardless of areMutuallyExclusive +if (type1 && type2 && doTypesConflict(type1, type2)) { + return [/* conflict */]; +} +``` + +**graphql-js also rejects the query.** + +## Known Issues About This Behavior + +This has been reported multiple times across the ecosystem: + +| Issue | Status | +|-------|--------| +| [graphql-js #1361](https://github.com/graphql/graphql-js/issues/1361): "Fragment safe divergence does not consider field nullability" | Closed as "working as designed". Maintainer suggested filing a spec change proposal. | +| [graphql-js #1065](https://github.com/graphql/graphql-js/issues/1065): "GraphQL union and conflicting types" | Closed with same explanation | +| [Netflix DGS #1583](https://github.com/Netflix/dgs-framework/issues/1583) | Closed as "working as designed per spec" | + +No spec change proposal has been filed or merged. + +## Why the Relaxation Is Safe + +Despite deviating from the spec, our relaxation is safe for the following reasons: + +### 1. Non-overlapping types can never co-resolve + +`User` and `Organization` are distinct concrete object types. At runtime, the +`entity` field resolves to exactly one of them. The `... on User` branch and +the `... on Organization` branch can **never** both apply to the same response +object. Therefore, a client will only ever see one nullability variant per object. + +### 2. The base types are still required to match + +Our relaxation **only** strips `NonNull` wrappers. It still requires: +- Same list structure (`[String]` vs `String` is still rejected) +- Same base named type (`String` vs `Int` is still rejected) +- Same abstract/concrete compatibility + +A nullable `String` is always a valid superset of a non-null `String!`. There is +no type safety issue. + +### 3. Conservative for interfaces + +When either enclosing type is an **interface**, we conservatively enforce the +strict check. This is because a concrete type could implement both interfaces, +making overlap possible. The relaxation only applies when both enclosing types +are **different concrete Object types**. + +### 4. The original spec rationale doesn't apply here + +The [original commit](https://github.com/graphql/graphql-js/commit/c034de91acce10d5c06d03bd332c6ebd45e2213c) +that introduced strict type checking for mutually exclusive types gave this +motivating example: + +```graphql +... on Person { foo: birthday { bar: year } } # bar: Int +... on Business { foo: location { bar: street } } # bar: String +``` + +Here `data.foo.bar` could be `Int` or `String` — genuinely ambiguous. But that +is a **different base type** conflict (`Int` vs `String`), which our fix still +rejects. Nullability differences (`String!` vs `String`) do not create this kind +of ambiguity — a client can always use the nullable type (`String`) as the field +type. + +## What We Changed + +In `operation_rule_field_selection_merging.go`, when `TypesAreCompatibleDeep` +fails (e.g. `String!` vs `String`), we now check: + +1. **Can the enclosing types overlap?** (`potentiallySameObject`) + - Interface + anything: YES (conservative, strict check) + - Same object type: YES (strict check) + - Different object types: NO (relaxed check) + +2. **If they can't overlap**: use `TypesAreCompatibleIgnoringNullability` which + strips `NonNull` wrappers at every nesting level but still requires the same + list structure and base named type. + +Existing test cases are unaffected: + +| Existing Test | Types Involved | Result | +|---------------|---------------|--------| +| `NonNullStringBox1` (interface) + `StringBox` (object) | Interface overlap possible | **Still Invalid** | +| `IntBox.scalar: Int` + `StringBox.scalar: String` | Different base types | **Still Invalid** | +| All "112" tests with `String` vs `Int` on Dog/Cat | Different base types | **Still Invalid** | + +## Conclusion + +This is a **deliberate, targeted deviation** from the GraphQL specification. It +addresses a real-world pain point that has been independently reported across +multiple GraphQL implementations. The relaxation is narrowly scoped (only +different concrete object types, only nullability differences) and preserves all +existing rejection behavior for genuinely conflicting types. diff --git a/v2/pkg/astvalidation/operation_rule_field_selection_merging.go b/v2/pkg/astvalidation/operation_rule_field_selection_merging.go index 51342d4a71..f9b52c5bdd 100644 --- a/v2/pkg/astvalidation/operation_rule_field_selection_merging.go +++ b/v2/pkg/astvalidation/operation_rule_field_selection_merging.go @@ -10,9 +10,10 @@ import ( ) // FieldSelectionMerging validates if field selections can be merged -func FieldSelectionMerging() Rule { +func FieldSelectionMerging(relaxNullabilityCheck ...bool) Rule { + relax := len(relaxNullabilityCheck) > 0 && relaxNullabilityCheck[0] return func(walker *astvisitor.Walker) { - visitor := fieldSelectionMergingVisitor{Walker: walker} + visitor := fieldSelectionMergingVisitor{Walker: walker, relaxNullabilityCheck: relax} walker.RegisterEnterDocumentVisitor(&visitor) walker.RegisterEnterFieldVisitor(&visitor) walker.RegisterEnterOperationVisitor(&visitor) @@ -27,6 +28,7 @@ type fieldSelectionMergingVisitor struct { scalarRequirements scalarRequirements nonScalarRequirements nonScalarRequirements refs []int + relaxNullabilityCheck bool } type nonScalarRequirement struct { path ast.Path @@ -117,12 +119,13 @@ func (f *fieldSelectionMergingVisitor) EnterField(ref int) { return } } else if !f.definition.TypesAreCompatibleDeep(f.nonScalarRequirements[i].fieldTypeRef, fieldType) { - // When enclosing types cannot overlap (e.g. two different concrete object types), - // allow nullability differences as long as the base types match - if !f.potentiallySameObject(f.nonScalarRequirements[i].enclosingTypeDefinition, f.EnclosingTypeDefinition) && - f.definition.TypesAreCompatibleIgnoringNullability(f.nonScalarRequirements[i].fieldTypeRef, fieldType) { - // Different nullability on non-overlapping types is safe - } else { + // Per SameResponseShape (spec sec 5.3.2), when enclosing types cannot overlap + // at runtime (two distinct concrete object types), nullability differences are + // acceptable because only one branch will ever contribute to the response. + // This relaxation is gated behind the relaxNullabilityCheck flag. + if !f.relaxNullabilityCheck || + f.potentiallySameObject(f.nonScalarRequirements[i].enclosingTypeDefinition, f.EnclosingTypeDefinition) || + !f.definition.TypesAreCompatibleIgnoringNullability(f.nonScalarRequirements[i].fieldTypeRef, fieldType) { left, err := f.definition.PrintTypeBytes(f.nonScalarRequirements[i].fieldTypeRef, nil) if err != nil { f.StopWithInternalErr(err) @@ -169,12 +172,13 @@ func (f *fieldSelectionMergingVisitor) EnterField(ref int) { } } if !f.definition.TypesAreCompatibleDeep(f.scalarRequirements[i].fieldType, fieldType) { - // When enclosing types cannot overlap (e.g. two different concrete object types), - // allow nullability differences as long as the base types match - if !f.potentiallySameObject(f.scalarRequirements[i].enclosingTypeDefinition, f.EnclosingTypeDefinition) && - f.definition.TypesAreCompatibleIgnoringNullability(f.scalarRequirements[i].fieldType, fieldType) { - // Different nullability on non-overlapping types is safe - } else { + // Per SameResponseShape (spec sec 5.3.2), when enclosing types cannot overlap + // at runtime (two distinct concrete object types), nullability differences are + // acceptable because only one branch will ever contribute to the response. + // This relaxation is gated behind the relaxNullabilityCheck flag. + if !f.relaxNullabilityCheck || + f.potentiallySameObject(f.scalarRequirements[i].enclosingTypeDefinition, f.EnclosingTypeDefinition) || + !f.definition.TypesAreCompatibleIgnoringNullability(f.scalarRequirements[i].fieldType, fieldType) { left, err := f.definition.PrintTypeBytes(f.scalarRequirements[i].fieldType, nil) if err != nil { f.StopWithInternalErr(err) @@ -209,6 +213,14 @@ func (f *fieldSelectionMergingVisitor) EnterField(ref int) { }) } +// potentiallySameObject reports whether two enclosing type definitions could apply +// to the same runtime object. This determines whether field merging must enforce +// strict type equality (including nullability) or may relax it. +// +// - If either type is an interface, returns true (conservative: any concrete +// type might implement that interface). +// - Two object types overlap only when they share the same name. +// - All other combinations return false. func (f *fieldSelectionMergingVisitor) potentiallySameObject(left, right ast.Node) bool { switch { case left.Kind == ast.NodeKindInterfaceTypeDefinition || right.Kind == ast.NodeKindInterfaceTypeDefinition: diff --git a/v2/pkg/astvalidation/operation_validation.go b/v2/pkg/astvalidation/operation_validation.go index ec3f345fc2..e034061ce3 100644 --- a/v2/pkg/astvalidation/operation_validation.go +++ b/v2/pkg/astvalidation/operation_validation.go @@ -12,7 +12,8 @@ import ( ) type OperationValidatorOptions struct { - ApolloCompatibilityFlags apollocompatibility.Flags + ApolloCompatibilityFlags apollocompatibility.Flags + RelaxFieldSelectionMergingNullabilityCheck bool } func WithApolloCompatibilityFlags(flags apollocompatibility.Flags) Option { @@ -21,6 +22,12 @@ func WithApolloCompatibilityFlags(flags apollocompatibility.Flags) Option { } } +func WithRelaxFieldSelectionMergingNullability() Option { + return func(options *OperationValidatorOptions) { + options.RelaxFieldSelectionMergingNullabilityCheck = true + } +} + type Option func(options *OperationValidatorOptions) // DefaultOperationValidator returns a fully initialized OperationValidator with all default rules registered @@ -47,7 +54,7 @@ func DefaultOperationValidator(options ...Option) *OperationValidator { validator.RegisterRule(LoneAnonymousOperation()) validator.RegisterRule(SubscriptionSingleRootField()) validator.RegisterRule(FieldSelections()) - validator.RegisterRule(FieldSelectionMerging()) + validator.RegisterRule(FieldSelectionMerging(opts.RelaxFieldSelectionMergingNullabilityCheck)) validator.RegisterRule(KnownArguments()) validator.RegisterRule(Values()) validator.RegisterRule(ArgumentUniqueness()) diff --git a/v2/pkg/astvalidation/operation_validation_test.go b/v2/pkg/astvalidation/operation_validation_test.go index 94d7acdd62..74783bae26 100644 --- a/v2/pkg/astvalidation/operation_validation_test.go +++ b/v2/pkg/astvalidation/operation_validation_test.go @@ -1084,7 +1084,17 @@ func TestExecutionValidation(t *testing.T) { ... on User { email } ... on Organization { email } } - }`, FieldSelectionMerging(), Valid) + }`, FieldSelectionMerging(true), Valid) + }) + t.Run("rejects differing scalar nullability on non-overlapping object types without relaxation flag", func(t *testing.T) { + runWithDefinition(t, entityDefinition, ` + { + entity { + ... on User { email } + ... on Organization { email } + } + }`, FieldSelectionMerging(), Invalid, + withValidationErrors(`fields 'email' conflict because they return conflicting types 'String!' and 'String'`)) }) t.Run("allows differing non-scalar nullability on non-overlapping object types", func(t *testing.T) { runWithDefinition(t, entityDefinition, ` @@ -1093,7 +1103,17 @@ func TestExecutionValidation(t *testing.T) { ... on User { profile { name } } ... on Organization { profile { name } } } - }`, FieldSelectionMerging(), Valid) + }`, FieldSelectionMerging(true), Valid) + }) + t.Run("rejects differing non-scalar nullability on non-overlapping object types without relaxation flag", func(t *testing.T) { + runWithDefinition(t, entityDefinition, ` + { + entity { + ... on User { profile { name } } + ... on Organization { profile { name } } + } + }`, FieldSelectionMerging(), Invalid, + withValidationErrors(`differing types 'Profile!' and 'Profile' for objectName 'profile'`)) }) t.Run("disallows differing return type nullability when interface could overlap", func(t *testing.T) { runWithDefinition(t, boxDefinition, ` diff --git a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go index 8d46cfb6e1..85a14bb6ff 100644 --- a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go +++ b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go @@ -92,6 +92,8 @@ type Planner[T Configuration] struct { // gRPC grpcClient grpc.ClientConnInterface + + printKitPool *sync.Pool } func (p *Planner[T]) EnableSubgraphRequestMinifier() { @@ -1685,10 +1687,10 @@ type printKit struct { report *operationreport.Report } -var ( - printKitPool = &sync.Pool{ +func newPrintKitPool(validationOptions ...astvalidation.Option) *sync.Pool { + return &sync.Pool{ New: func() any { - validator := astvalidation.DefaultOperationValidator() + validator := astvalidation.DefaultOperationValidator(validationOptions...) // as we are creating operation programmatically in the graphql datasource planner, // we need to catch incorrect behavior of the planner // as graphql datasource planner should visit only selection sets which has fields, @@ -1710,14 +1712,16 @@ var ( } }, } -) +} type Factory[T Configuration] struct { - executionContext context.Context - httpClient *http.Client - grpcClient grpc.ClientConnInterface - grpcClientProvider func() grpc.ClientConnInterface - subscriptionClient GraphQLSubscriptionClient + executionContext context.Context + httpClient *http.Client + grpcClient grpc.ClientConnInterface + grpcClientProvider func() grpc.ClientConnInterface + subscriptionClient GraphQLSubscriptionClient + relaxFieldSelectionMergingNullability bool + printKitPool *sync.Pool } // NewFactory (HTTP) creates a new factory for the GraphQL datasource planner @@ -1780,13 +1784,29 @@ func NewFactoryGRPCClientProvider(executionContext context.Context, clientProvid } func (p *Planner[T]) getKit() *printKit { - return printKitPool.Get().(*printKit) + return p.printKitPool.Get().(*printKit) } func (p *Planner[T]) releaseKit(kit *printKit) { kit.buf.Reset() kit.report.Reset() - printKitPool.Put(kit) + p.printKitPool.Put(kit) +} + +func (f *Factory[T]) EnableSubgraphFieldSelectionMergingNullabilityRelaxation() { + f.relaxFieldSelectionMergingNullability = true + f.printKitPool = nil // reset pool so it's recreated with the new setting +} + +func (f *Factory[T]) getPrintKitPool() *sync.Pool { + if f.printKitPool == nil { + if f.relaxFieldSelectionMergingNullability { + f.printKitPool = newPrintKitPool(astvalidation.WithRelaxFieldSelectionMergingNullability()) + } else { + f.printKitPool = newPrintKitPool() + } + } + return f.printKitPool } func (f *Factory[T]) Planner(logger abstractlogger.Logger) plan.DataSourcePlanner[T] { @@ -1799,6 +1819,7 @@ func (f *Factory[T]) Planner(logger abstractlogger.Logger) plan.DataSourcePlanne fetchClient: f.httpClient, grpcClient: grpcClient, subscriptionClient: f.subscriptionClient, + printKitPool: f.getPrintKitPool(), } } diff --git a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_test.go b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_test.go index e8f3140f09..05b07df9e1 100644 --- a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_test.go +++ b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_test.go @@ -21,6 +21,7 @@ import ( "github.com/stretchr/testify/require" "github.com/wundergraph/graphql-go-tools/v2/pkg/ast" + "github.com/wundergraph/graphql-go-tools/v2/pkg/astvalidation" "github.com/wundergraph/graphql-go-tools/v2/pkg/engine/datasource/httpclient" . "github.com/wundergraph/graphql-go-tools/v2/pkg/engine/datasourcetesting" "github.com/wundergraph/graphql-go-tools/v2/pkg/engine/plan" @@ -8357,8 +8358,10 @@ func TestGraphQLDataSource(t *testing.T) { }), ), }, - DisableResolveFieldPositions: true, + DisableResolveFieldPositions: true, + RelaxSubgraphOperationFieldSelectionMergingNullability: true, }, WithDefaultPostProcessor(), + WithValidationOptions(astvalidation.WithRelaxFieldSelectionMergingNullability()), )) }) } diff --git a/v2/pkg/engine/datasourcetesting/datasourcetesting.go b/v2/pkg/engine/datasourcetesting/datasourcetesting.go index 5987666447..ec9c8907f3 100644 --- a/v2/pkg/engine/datasourcetesting/datasourcetesting.go +++ b/v2/pkg/engine/datasourcetesting/datasourcetesting.go @@ -34,6 +34,7 @@ type testOptions struct { withPrintPlan bool withFieldDependencies bool withFetchReasons bool + validationOptions []astvalidation.Option } func WithPostProcessors(postProcessors ...*postprocess.Processor) func(*testOptions) { @@ -84,6 +85,12 @@ func WithFetchReasons() func(*testOptions) { } } +func WithValidationOptions(options ...astvalidation.Option) func(*testOptions) { + return func(o *testOptions) { + o.validationOptions = options + } +} + func RunWithPermutations(t *testing.T, definition, operation, operationName string, expectedPlan plan.Plan, config plan.Configuration, options ...func(*testOptions)) { t.Helper() @@ -181,7 +188,7 @@ func RunTestWithVariables(definition, operation, operationName, variables string normalized := unsafeprinter.PrettyPrint(&op) _ = normalized - valid := astvalidation.DefaultOperationValidator() + valid := astvalidation.DefaultOperationValidator(opts.validationOptions...) valid.Validate(&op, &def, &report) p, err := plan.NewPlanner(config) diff --git a/v2/pkg/engine/plan/configuration.go b/v2/pkg/engine/plan/configuration.go index cff8109235..5194c8e976 100644 --- a/v2/pkg/engine/plan/configuration.go +++ b/v2/pkg/engine/plan/configuration.go @@ -52,6 +52,13 @@ type Configuration struct { // When the list size is unknown from directives, this value is used as a default for static cost. StaticCostDefaultListSize int + + // RelaxSubgraphOperationFieldSelectionMergingNullability relaxes the nullability validation + // for field selection merging in upstream (subgraph) operations when enclosing types are + // non-overlapping concrete object types. This is a deliberate spec deviation. + // When true, factories that support it will allow differing nullability + // (e.g. String! vs String) on fields in non-overlapping inline fragments. + RelaxSubgraphOperationFieldSelectionMergingNullability bool } type DebugConfiguration struct { diff --git a/v2/pkg/engine/plan/datasource_configuration.go b/v2/pkg/engine/plan/datasource_configuration.go index afc922be8d..0601e3abfb 100644 --- a/v2/pkg/engine/plan/datasource_configuration.go +++ b/v2/pkg/engine/plan/datasource_configuration.go @@ -300,6 +300,12 @@ func (d *dataSourceConfiguration[T]) CustomConfiguration() T { } func (d *dataSourceConfiguration[T]) CreatePlannerConfiguration(logger abstractlogger.Logger, fetchConfig *objectFetchConfiguration, pathConfig *plannerPathsConfiguration, configuration *Configuration) PlannerConfiguration { + if configuration.RelaxSubgraphOperationFieldSelectionMergingNullability { + if relaxer, ok := d.factory.(SubgraphFieldSelectionMergingNullabilityRelaxer); ok { + relaxer.EnableSubgraphFieldSelectionMergingNullabilityRelaxation() + } + } + planner := d.factory.Planner(logger) fetchConfig.planner = planner @@ -475,6 +481,14 @@ type Identifyable interface { astvisitor.VisitorIdentifier } +// SubgraphFieldSelectionMergingNullabilityRelaxer is an optional interface that a PlannerFactory +// can implement to support relaxed nullability checks when validating upstream operations. +// When called, the factory should configure its internal validator to allow differing nullability +// on fields in non-overlapping concrete types. +type SubgraphFieldSelectionMergingNullabilityRelaxer interface { + EnableSubgraphFieldSelectionMergingNullabilityRelaxation() +} + type DataSourcePlanner[T any] interface { DataSourceFetchPlanner DataSourceBehavior From ed7f44aa577895be9af0163eb325641f5e731d73 Mon Sep 17 00:00:00 2001 From: Jens Neuse Date: Mon, 9 Feb 2026 17:22:36 +0100 Subject: [PATCH 3/9] test: add e2e test for field merging with relaxed nullability across union types Thread validation options from plannerConfig through ExecutionEngine to ValidateForSchema so the relaxed nullability flag is respected end-to-end. Add three sub-tests: a negative test proving strict validation rejects differing nullability, and two positive tests verifying correct resolution for both non-null and null email values on different union member types. Also change error comparison in runWithAndCompareError from Contains to Equal for more explicit assertions. Co-Authored-By: Claude Opus 4.6 --- execution/engine/execution_engine.go | 10 +- execution/engine/execution_engine_test.go | 138 +++++++++++++++++++++- execution/graphql/validation.go | 4 +- 3 files changed, 148 insertions(+), 4 deletions(-) diff --git a/execution/engine/execution_engine.go b/execution/engine/execution_engine.go index 8b57b9c11d..e920e5e00f 100644 --- a/execution/engine/execution_engine.go +++ b/execution/engine/execution_engine.go @@ -17,6 +17,7 @@ import ( "github.com/wundergraph/graphql-go-tools/v2/pkg/ast" "github.com/wundergraph/graphql-go-tools/v2/pkg/astnormalization" "github.com/wundergraph/graphql-go-tools/v2/pkg/astprinter" + "github.com/wundergraph/graphql-go-tools/v2/pkg/astvalidation" "github.com/wundergraph/graphql-go-tools/v2/pkg/engine/datasource/introspection_datasource" "github.com/wundergraph/graphql-go-tools/v2/pkg/engine/plan" "github.com/wundergraph/graphql-go-tools/v2/pkg/engine/postprocess" @@ -62,6 +63,7 @@ type ExecutionEngine struct { resolver *resolve.Resolver executionPlanCache *lru.Cache apolloCompatibilityFlags apollocompatibility.Flags + validationOptions []astvalidation.Option } type WebsocketBeforeStartHook interface { @@ -130,6 +132,11 @@ func NewExecutionEngine(ctx context.Context, logger abstractlogger.Logger, engin dsIDs[ds.Id()] = struct{}{} } + var validationOpts []astvalidation.Option + if engineConfig.plannerConfig.RelaxSubgraphOperationFieldSelectionMergingNullability { + validationOpts = append(validationOpts, astvalidation.WithRelaxFieldSelectionMergingNullability()) + } + return &ExecutionEngine{ logger: logger, config: engineConfig, @@ -138,6 +145,7 @@ func NewExecutionEngine(ctx context.Context, logger abstractlogger.Logger, engin apolloCompatibilityFlags: apollocompatibility.Flags{ ReplaceInvalidVarError: resolverOptions.ResolvableOptions.ApolloCompatibilityReplaceInvalidVarError, }, + validationOptions: validationOpts, }, nil } @@ -159,7 +167,7 @@ func (e *ExecutionEngine) Execute(ctx context.Context, operation *graphql.Reques } // Validate the operation against the schema. - if result, err := operation.ValidateForSchema(e.config.schema); err != nil { + if result, err := operation.ValidateForSchema(e.config.schema, e.validationOptions...); err != nil { return err } else if !result.Valid { return result.Errors diff --git a/execution/engine/execution_engine_test.go b/execution/engine/execution_engine_test.go index 9ac77cd131..3b15c6dce4 100644 --- a/execution/engine/execution_engine_test.go +++ b/execution/engine/execution_engine_test.go @@ -223,6 +223,7 @@ type _executionTestOptions struct { propagateFetchReasons bool validateRequiredExternalFields bool computeStaticCost bool + relaxFieldSelectionMergingNullability bool } type executionTestOptions func(*_executionTestOptions) @@ -253,6 +254,12 @@ func computeStaticCost() executionTestOptions { } } +func relaxFieldSelectionMergingNullability() executionTestOptions { + return func(options *_executionTestOptions) { + options.relaxFieldSelectionMergingNullability = true + } +} + func TestExecutionEngine_Execute(t *testing.T) { run := func(testCase ExecutionEngineTestCase, withError bool, expectedErrorMessage string, options ...executionTestOptions) func(t *testing.T) { t.Helper() @@ -289,6 +296,7 @@ func TestExecutionEngine_Execute(t *testing.T) { engineConf.plannerConfig.ValidateRequiredExternalFields = opts.validateRequiredExternalFields engineConf.plannerConfig.ComputeStaticCost = opts.computeStaticCost engineConf.plannerConfig.StaticCostDefaultListSize = 10 + engineConf.plannerConfig.RelaxSubgraphOperationFieldSelectionMergingNullability = opts.relaxFieldSelectionMergingNullability resolveOpts := resolve.ResolverOptions{ MaxConcurrency: 1024, ResolvableOptions: opts.resolvableOptions, @@ -321,7 +329,7 @@ func TestExecutionEngine_Execute(t *testing.T) { if withError { require.Error(t, err) if expectedErrorMessage != "" { - assert.Contains(t, err.Error(), expectedErrorMessage) + assert.Equal(t, expectedErrorMessage, err.Error()) } } else { require.NoError(t, err) @@ -6812,6 +6820,134 @@ func TestExecutionEngine_Execute(t *testing.T) { }) }) + + t.Run("field merging with different nullability on non-overlapping union types", func(t *testing.T) { + unionSchema := ` + union Entity = User | Organization + type Query { entity: Entity } + type User { id: ID!, email: String! } + type Organization { id: ID!, email: String } + ` + schema, err := graphql.NewSchemaFromString(unionSchema) + require.NoError(t, err) + + rootNodes := []plan.TypeField{ + {TypeName: "Query", FieldNames: []string{"entity"}}, + {TypeName: "User", FieldNames: []string{"id", "email"}}, + {TypeName: "Organization", FieldNames: []string{"id", "email"}}, + } + + customConfig := mustConfiguration(t, graphql_datasource.ConfigurationInput{ + Fetch: &graphql_datasource.FetchConfiguration{ + URL: "https://example.com/", + Method: "POST", + }, + SchemaConfiguration: mustSchemaConfig(t, nil, unionSchema), + }) + + fieldConfig := []plan.FieldConfiguration{ + { + TypeName: "Query", + FieldName: "entity", + Path: []string{"entity"}, + }, + } + + t.Run("without relaxation flag, validation rejects differing nullability", runWithAndCompareError( + ExecutionEngineTestCase{ + schema: schema, + operation: func(t *testing.T) graphql.Request { + return graphql.Request{ + OperationName: "O", + Query: `query O { entity { ... on User { email } ... on Organization { email } } }`, + } + }, + dataSources: []plan.DataSource{ + mustGraphqlDataSourceConfiguration(t, "ds-id", + mustFactory(t, + testNetHttpClient(t, roundTripperTestCase{ + expectedHost: "example.com", + expectedPath: "/", + expectedBody: "", + sendResponseBody: `{"data":{"entity":{"__typename":"User","email":"user@test.com"}}}`, + sendStatusCode: 200, + }), + ), + &plan.DataSourceMetadata{ + RootNodes: rootNodes, + }, + customConfig, + ), + }, + fields: fieldConfig, + }, + `fields 'email' conflict because they return conflicting types 'String!' and 'String', locations: [], path: [query,entity,$1Organization]`, + )) + + t.Run("non-null email from User type", runWithoutError( + ExecutionEngineTestCase{ + schema: schema, + operation: func(t *testing.T) graphql.Request { + return graphql.Request{ + OperationName: "O", + Query: `query O { entity { ... on User { email } ... on Organization { email } } }`, + } + }, + dataSources: []plan.DataSource{ + mustGraphqlDataSourceConfiguration(t, "ds-id", + mustFactory(t, + testNetHttpClient(t, roundTripperTestCase{ + expectedHost: "example.com", + expectedPath: "/", + expectedBody: "", + sendResponseBody: `{"data":{"entity":{"__typename":"User","email":"user@test.com"}}}`, + sendStatusCode: 200, + }), + ), + &plan.DataSourceMetadata{ + RootNodes: rootNodes, + }, + customConfig, + ), + }, + fields: fieldConfig, + expectedResponse: `{"data":{"entity":{"email":"user@test.com"}}}`, + }, + relaxFieldSelectionMergingNullability(), + )) + + t.Run("null email from Organization type", runWithoutError( + ExecutionEngineTestCase{ + schema: schema, + operation: func(t *testing.T) graphql.Request { + return graphql.Request{ + OperationName: "O", + Query: `query O { entity { ... on User { email } ... on Organization { email } } }`, + } + }, + dataSources: []plan.DataSource{ + mustGraphqlDataSourceConfiguration(t, "ds-id", + mustFactory(t, + testNetHttpClient(t, roundTripperTestCase{ + expectedHost: "example.com", + expectedPath: "/", + expectedBody: "", + sendResponseBody: `{"data":{"entity":{"__typename":"Organization","email":null}}}`, + sendStatusCode: 200, + }), + ), + &plan.DataSourceMetadata{ + RootNodes: rootNodes, + }, + customConfig, + ), + }, + fields: fieldConfig, + expectedResponse: `{"data":{"entity":{"email":null}}}`, + }, + relaxFieldSelectionMergingNullability(), + )) + }) } func testNetHttpClient(t *testing.T, testCase roundTripperTestCase) *http.Client { diff --git a/execution/graphql/validation.go b/execution/graphql/validation.go index c681c12c71..0aec784886 100644 --- a/execution/graphql/validation.go +++ b/execution/graphql/validation.go @@ -11,7 +11,7 @@ type ValidationResult struct { Errors graphqlerrors.Errors } -func (r *Request) ValidateForSchema(schema *Schema) (result ValidationResult, err error) { +func (r *Request) ValidateForSchema(schema *Schema, options ...astvalidation.Option) (result ValidationResult, err error) { if schema == nil { return ValidationResult{Valid: false, Errors: nil}, ErrNilSchema } @@ -31,7 +31,7 @@ func (r *Request) ValidateForSchema(schema *Schema) (result ValidationResult, er return operationValidationResultFromReport(report) } - validator := astvalidation.DefaultOperationValidator() + validator := astvalidation.DefaultOperationValidator(options...) validator.Validate(&r.document, &schema.document, &report) result, err = operationValidationResultFromReport(report) if err != nil { From 8b0cf8d3a37f631bf061943e425417b281d29f09 Mon Sep 17 00:00:00 2001 From: Jens Neuse Date: Mon, 9 Feb 2026 17:32:38 +0100 Subject: [PATCH 4/9] feat: enable relaxed nullability for field selection merging in non-overlapping types --- .../operation_rule_field_selection_merging.go | 8 +++--- v2/pkg/astvalidation/operation_validation.go | 4 +++ .../graphql_datasource/graphql_datasource.go | 26 +++++++++---------- 3 files changed, 20 insertions(+), 18 deletions(-) diff --git a/v2/pkg/astvalidation/operation_rule_field_selection_merging.go b/v2/pkg/astvalidation/operation_rule_field_selection_merging.go index f9b52c5bdd..04dad67ecc 100644 --- a/v2/pkg/astvalidation/operation_rule_field_selection_merging.go +++ b/v2/pkg/astvalidation/operation_rule_field_selection_merging.go @@ -119,10 +119,10 @@ func (f *fieldSelectionMergingVisitor) EnterField(ref int) { return } } else if !f.definition.TypesAreCompatibleDeep(f.nonScalarRequirements[i].fieldTypeRef, fieldType) { - // Per SameResponseShape (spec sec 5.3.2), when enclosing types cannot overlap - // at runtime (two distinct concrete object types), nullability differences are - // acceptable because only one branch will ever contribute to the response. - // This relaxation is gated behind the relaxNullabilityCheck flag. + // Deliberate deviation from SameResponseShape (spec sec 5.3.2): when enclosing + // types cannot overlap at runtime (two distinct concrete object types), + // we allow nullability differences because only one branch will ever + // contribute to the response. This is gated behind relaxNullabilityCheck. if !f.relaxNullabilityCheck || f.potentiallySameObject(f.nonScalarRequirements[i].enclosingTypeDefinition, f.EnclosingTypeDefinition) || !f.definition.TypesAreCompatibleIgnoringNullability(f.nonScalarRequirements[i].fieldTypeRef, fieldType) { diff --git a/v2/pkg/astvalidation/operation_validation.go b/v2/pkg/astvalidation/operation_validation.go index e034061ce3..e24a72a5be 100644 --- a/v2/pkg/astvalidation/operation_validation.go +++ b/v2/pkg/astvalidation/operation_validation.go @@ -22,6 +22,10 @@ func WithApolloCompatibilityFlags(flags apollocompatibility.Flags) Option { } } +// WithRelaxFieldSelectionMergingNullability enables a deliberate spec deviation that allows +// differing nullability (e.g. String! vs String) on fields in non-overlapping concrete +// object types within inline fragments. Without this option, the validator enforces the +// strict GraphQL spec behavior where nullability must always match. func WithRelaxFieldSelectionMergingNullability() Option { return func(options *OperationValidatorOptions) { options.RelaxFieldSelectionMergingNullabilityCheck = true diff --git a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go index 85a14bb6ff..e66a720942 100644 --- a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go +++ b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go @@ -1715,13 +1715,12 @@ func newPrintKitPool(validationOptions ...astvalidation.Option) *sync.Pool { } type Factory[T Configuration] struct { - executionContext context.Context - httpClient *http.Client - grpcClient grpc.ClientConnInterface - grpcClientProvider func() grpc.ClientConnInterface - subscriptionClient GraphQLSubscriptionClient - relaxFieldSelectionMergingNullability bool - printKitPool *sync.Pool + executionContext context.Context + httpClient *http.Client + grpcClient grpc.ClientConnInterface + grpcClientProvider func() grpc.ClientConnInterface + subscriptionClient GraphQLSubscriptionClient + printKitPool *sync.Pool } // NewFactory (HTTP) creates a new factory for the GraphQL datasource planner @@ -1793,18 +1792,17 @@ func (p *Planner[T]) releaseKit(kit *printKit) { p.printKitPool.Put(kit) } +// EnableSubgraphFieldSelectionMergingNullabilityRelaxation implements +// plan.SubgraphFieldSelectionMergingNullabilityRelaxer. It configures the +// factory's internal operation validator to allow differing nullability on +// fields in non-overlapping concrete types. Must be called before Planner(). func (f *Factory[T]) EnableSubgraphFieldSelectionMergingNullabilityRelaxation() { - f.relaxFieldSelectionMergingNullability = true - f.printKitPool = nil // reset pool so it's recreated with the new setting + f.printKitPool = newPrintKitPool(astvalidation.WithRelaxFieldSelectionMergingNullability()) } func (f *Factory[T]) getPrintKitPool() *sync.Pool { if f.printKitPool == nil { - if f.relaxFieldSelectionMergingNullability { - f.printKitPool = newPrintKitPool(astvalidation.WithRelaxFieldSelectionMergingNullability()) - } else { - f.printKitPool = newPrintKitPool() - } + f.printKitPool = newPrintKitPool() } return f.printKitPool } From c9e4dacd59ff5ee432493daa4c24df39cb783420 Mon Sep 17 00:00:00 2001 From: Jens Neuse Date: Mon, 16 Feb 2026 14:30:15 +0100 Subject: [PATCH 5/9] fix: resolve data race in Factory.getPrintKitPool lazy initialization Use sync.Once to make the lazy initialization of printKitPool thread-safe when multiple goroutines call Planner() concurrently. Co-Authored-By: Claude Opus 4.6 --- .../datasource/graphql_datasource/graphql_datasource.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go index e66a720942..0cb7b7df94 100644 --- a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go +++ b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go @@ -1721,6 +1721,7 @@ type Factory[T Configuration] struct { grpcClientProvider func() grpc.ClientConnInterface subscriptionClient GraphQLSubscriptionClient printKitPool *sync.Pool + printKitPoolOnce sync.Once } // NewFactory (HTTP) creates a new factory for the GraphQL datasource planner @@ -1801,9 +1802,11 @@ func (f *Factory[T]) EnableSubgraphFieldSelectionMergingNullabilityRelaxation() } func (f *Factory[T]) getPrintKitPool() *sync.Pool { - if f.printKitPool == nil { - f.printKitPool = newPrintKitPool() - } + f.printKitPoolOnce.Do(func() { + if f.printKitPool == nil { + f.printKitPool = newPrintKitPool() + } + }) return f.printKitPool } From 73ed1a1f6609784ecb7cf0f255ae6f05cfcfd929 Mon Sep 17 00:00:00 2001 From: Jens Neuse Date: Mon, 16 Feb 2026 14:45:33 +0100 Subject: [PATCH 6/9] fix: address review feedback on nullability relaxation - Store relaxation as bool flag, apply in sync.Once (devsergiy review) - Bypass validation cache when custom options are provided (CodeRabbit review) Co-Authored-By: Claude Opus 4.6 --- execution/graphql/validation.go | 11 +++++++--- .../graphql_datasource/graphql_datasource.go | 21 +++++++++++-------- 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/execution/graphql/validation.go b/execution/graphql/validation.go index 0aec784886..4789408f00 100644 --- a/execution/graphql/validation.go +++ b/execution/graphql/validation.go @@ -17,13 +17,16 @@ func (r *Request) ValidateForSchema(schema *Schema, options ...astvalidation.Opt } schemaHash := schema.Hash() + useCache := len(options) == 0 if r.validForSchema == nil { r.validForSchema = map[uint64]ValidationResult{} } - if result, ok := r.validForSchema[schemaHash]; ok { - return result, nil + if useCache { + if result, ok := r.validForSchema[schemaHash]; ok { + return result, nil + } } report := r.parseQueryOnce() @@ -37,7 +40,9 @@ func (r *Request) ValidateForSchema(schema *Schema, options ...astvalidation.Opt if err != nil { return result, err } - r.validForSchema[schemaHash] = result + if useCache { + r.validForSchema[schemaHash] = result + } return result, err } diff --git a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go index 0cb7b7df94..748c083ec6 100644 --- a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go +++ b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go @@ -1715,13 +1715,14 @@ func newPrintKitPool(validationOptions ...astvalidation.Option) *sync.Pool { } type Factory[T Configuration] struct { - executionContext context.Context - httpClient *http.Client - grpcClient grpc.ClientConnInterface - grpcClientProvider func() grpc.ClientConnInterface - subscriptionClient GraphQLSubscriptionClient - printKitPool *sync.Pool - printKitPoolOnce sync.Once + executionContext context.Context + httpClient *http.Client + grpcClient grpc.ClientConnInterface + grpcClientProvider func() grpc.ClientConnInterface + subscriptionClient GraphQLSubscriptionClient + printKitPool *sync.Pool + printKitPoolOnce sync.Once + relaxFieldMergingNullability bool } // NewFactory (HTTP) creates a new factory for the GraphQL datasource planner @@ -1798,12 +1799,14 @@ func (p *Planner[T]) releaseKit(kit *printKit) { // factory's internal operation validator to allow differing nullability on // fields in non-overlapping concrete types. Must be called before Planner(). func (f *Factory[T]) EnableSubgraphFieldSelectionMergingNullabilityRelaxation() { - f.printKitPool = newPrintKitPool(astvalidation.WithRelaxFieldSelectionMergingNullability()) + f.relaxFieldMergingNullability = true } func (f *Factory[T]) getPrintKitPool() *sync.Pool { f.printKitPoolOnce.Do(func() { - if f.printKitPool == nil { + if f.relaxFieldMergingNullability { + f.printKitPool = newPrintKitPool(astvalidation.WithRelaxFieldSelectionMergingNullability()) + } else { f.printKitPool = newPrintKitPool() } }) From 6e09db60e28ef500c01288d1f1c86b2aaf263104 Mon Sep 17 00:00:00 2001 From: Jens Neuse Date: Mon, 16 Feb 2026 14:54:02 +0100 Subject: [PATCH 7/9] refactor: use shared package-level print kit pools Instead of per-factory pools, use two shared package-level pools (default + relaxed). All factories/planners share the same pool, avoiding redundant pool instances across datasources. Co-Authored-By: Claude Opus 4.6 --- .../graphql_datasource/graphql_datasource.go | 45 +++++++++++-------- 1 file changed, 26 insertions(+), 19 deletions(-) diff --git a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go index 748c083ec6..a0aadacc4c 100644 --- a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go +++ b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go @@ -1714,15 +1714,26 @@ func newPrintKitPool(validationOptions ...astvalidation.Option) *sync.Pool { } } +var ( + defaultPrintKitPool = newPrintKitPool() + relaxedPrintKitPool *sync.Pool + relaxedPrintKitPoolOnce sync.Once +) + +func getRelaxedPrintKitPool() *sync.Pool { + relaxedPrintKitPoolOnce.Do(func() { + relaxedPrintKitPool = newPrintKitPool(astvalidation.WithRelaxFieldSelectionMergingNullability()) + }) + return relaxedPrintKitPool +} + type Factory[T Configuration] struct { - executionContext context.Context - httpClient *http.Client - grpcClient grpc.ClientConnInterface - grpcClientProvider func() grpc.ClientConnInterface - subscriptionClient GraphQLSubscriptionClient - printKitPool *sync.Pool - printKitPoolOnce sync.Once - relaxFieldMergingNullability bool + executionContext context.Context + httpClient *http.Client + grpcClient grpc.ClientConnInterface + grpcClientProvider func() grpc.ClientConnInterface + subscriptionClient GraphQLSubscriptionClient + printKitPool *sync.Pool } // NewFactory (HTTP) creates a new factory for the GraphQL datasource planner @@ -1796,21 +1807,17 @@ func (p *Planner[T]) releaseKit(kit *printKit) { // EnableSubgraphFieldSelectionMergingNullabilityRelaxation implements // plan.SubgraphFieldSelectionMergingNullabilityRelaxer. It configures the -// factory's internal operation validator to allow differing nullability on -// fields in non-overlapping concrete types. Must be called before Planner(). +// factory to use a shared pool whose validator allows differing nullability +// on fields in non-overlapping concrete types. func (f *Factory[T]) EnableSubgraphFieldSelectionMergingNullabilityRelaxation() { - f.relaxFieldMergingNullability = true + f.printKitPool = getRelaxedPrintKitPool() } func (f *Factory[T]) getPrintKitPool() *sync.Pool { - f.printKitPoolOnce.Do(func() { - if f.relaxFieldMergingNullability { - f.printKitPool = newPrintKitPool(astvalidation.WithRelaxFieldSelectionMergingNullability()) - } else { - f.printKitPool = newPrintKitPool() - } - }) - return f.printKitPool + if f.printKitPool != nil { + return f.printKitPool + } + return defaultPrintKitPool } func (f *Factory[T]) Planner(logger abstractlogger.Logger) plan.DataSourcePlanner[T] { From 8bcad04da2ae6eacf11c0a618b862b2cfb797405 Mon Sep 17 00:00:00 2001 From: Jens Neuse Date: Mon, 16 Feb 2026 15:03:02 +0100 Subject: [PATCH 8/9] style: fix gci import ordering and var alignment Co-Authored-By: Claude Opus 4.6 --- .../datasource/graphql_datasource/graphql_datasource.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go index a0aadacc4c..9be986a9a0 100644 --- a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go +++ b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go @@ -1715,9 +1715,9 @@ func newPrintKitPool(validationOptions ...astvalidation.Option) *sync.Pool { } var ( - defaultPrintKitPool = newPrintKitPool() - relaxedPrintKitPool *sync.Pool - relaxedPrintKitPoolOnce sync.Once + defaultPrintKitPool = newPrintKitPool() + relaxedPrintKitPool *sync.Pool + relaxedPrintKitPoolOnce sync.Once ) func getRelaxedPrintKitPool() *sync.Pool { From cb3f754f5e819aa704c3ab9e6d4e2bf3bd559e4c Mon Sep 17 00:00:00 2001 From: Jens Neuse Date: Mon, 16 Feb 2026 16:04:18 +0100 Subject: [PATCH 9/9] fix: add nil-safety fallback in Planner getKit/releaseKit Fall back to defaultPrintKitPool if printKitPool is nil, preventing a panic if Planner is instantiated directly without Factory. Co-Authored-By: Claude Opus 4.6 --- .../graphql_datasource/graphql_datasource.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go index 9be986a9a0..f4268d1f6a 100644 --- a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go +++ b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go @@ -1796,13 +1796,21 @@ func NewFactoryGRPCClientProvider(executionContext context.Context, clientProvid } func (p *Planner[T]) getKit() *printKit { - return p.printKitPool.Get().(*printKit) + pool := p.printKitPool + if pool == nil { + pool = defaultPrintKitPool + } + return pool.Get().(*printKit) } func (p *Planner[T]) releaseKit(kit *printKit) { kit.buf.Reset() kit.report.Reset() - p.printKitPool.Put(kit) + pool := p.printKitPool + if pool == nil { + pool = defaultPrintKitPool + } + pool.Put(kit) } // EnableSubgraphFieldSelectionMergingNullabilityRelaxation implements