Skip to content

Fix overlapping fields null type regression from 072165b #4289

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

Closed
andimarek wants to merge 2 commits into from
Closed

Fix overlapping fields null type regression from 072165b #4289

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
293b21b
Add failing test for overlapping fields null type regression (072165b)
claude Mar 3, 2026
e27544a
Fix null type handling regression in requireSameOutputTypeShape
claude Mar 3, 2026
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
7 changes: 5 additions & 2 deletions src/main/java/graphql/validation/OperationValidator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1241,8 +1241,11 @@ private boolean sameArguments(List<Argument> arguments1, @Nullable List<Argument
}
GraphQLType typeA = typeAOriginal;
GraphQLType typeB = fieldAndType.graphQLType;
if (typeB == null) {
return mkNotSameTypeError(path, fields, typeA, typeB);
if (typeA == null || typeB == null) {
if (typeA != typeB) {
return mkNotSameTypeError(path, fields, typeA, typeB);
}
continue;
}
while (true) {
if (isNonNull(typeA) || isNonNull(typeB)) {
Expand Down
44 changes: 44 additions & 0 deletions src/test/groovy/graphql/validation/OverlappingFieldsCanBeMergedTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import graphql.language.SourceLocation
import graphql.parser.Parser
import graphql.schema.GraphQLCodeRegistry
import graphql.schema.GraphQLSchema
import graphql.schema.idl.SchemaGenerator
import graphql.validation.LanguageTraversal
import graphql.validation.OperationValidationRule
import graphql.validation.OperationValidator
Expand Down Expand Up @@ -967,6 +968,49 @@ class OverlappingFieldsCanBeMergedTest extends Specification {
errorCollector.getErrors().size() == 0
}

def "mixed null and non-null field types from abstract and concrete inline fragments should not conflict"() {
given:
// Reproduces the benchmarkDeepAbstractConcrete scenario from OverlappingFieldValidationPerformance.
// The fragment spreads on interfaces (Abstract1, Abstract2) resolve "field" to type Abstract
// (because interfaces implement GraphQLFieldsContainer), but the fragment itself is on "Whatever"
// which doesn't exist, so the outer "field" selected directly from the fragment has null type.
// When mergeSubSelections collects sub-fields, the inner "field" selections end up as a set
// containing both null-typed entries (from the unresolvable parent) and Abstract-typed entries
// (from the interface inline fragments). The old code treated null as "unknown, skip comparison";
// commit 072165b regressed this by treating null typeB as an unconditional conflict.
def schema = SchemaGenerator.createdMockedSchema('''
type Query { viewer: Viewer }
interface Abstract { field: Abstract leaf: Int }
interface Abstract1 { field: Abstract leaf: Int }
interface Abstract2 { field: Abstract leaf: Int }
type Concrete1 implements Abstract1 { field: Abstract leaf: Int }
type Concrete2 implements Abstract2 { field: Abstract leaf: Int }
type Viewer { xingId: XingId }
type XingId { firstName: String! lastName: String! }
''')
// Nesting depth > 1 is required: the outer "field" on Query is unresolvable (null type),
// and the fragment's inline fragments resolve "field" on known interfaces (non-null type).
// After mergeSubSelections, the inner "field" set contains both null and non-null typed entries.
def query = '''
fragment multiply on Whatever {
field {
... on Abstract1 { field { leaf } }
... on Abstract2 { field { leaf } }
... on Concrete1 { field { leaf } }
... on Concrete2 { field { leaf } }
}
}
query DeepAbstractConcrete {
field { ...multiply field { ...multiply } }
}
'''
when:
traverse(query, schema)

then:
errorCollector.getErrors().isEmpty()
}

def "overlapping fields on lower level"() {
given:
def schema = schema('''
Expand Down
Loading