Fix null type handling to skip comparison for unresolvable types

claude · claude · commit b125184f47dc · 2026-03-04T15:47:42.000Z
The previous fix only handled the both-null case, but the actual benchmarkDeepAbstractConcrete scenario has mixed null and non-null types: fragments on "Whatever" (unresolvable) produce null-typed fields alongside Abstract-typed fields from interface inline fragments. A null type means the parent was unresolvable, so we should skip the comparison entirely rather than report a spurious conflict. This changes the null check from reporting an error to simply continuing, which matches the semantic that null = unknown/unresolvable = skip. The test now reproduces the exact benchmark scenario that was crashing. https://claude.ai/code/session_01MZkhqChmheW6d46H4p8T7Y
diff --git a/src/main/java/graphql/validation/OperationValidator.java b/src/main/java/graphql/validation/OperationValidator.java
@@ -1242,9 +1242,6 @@ private boolean sameArguments(List<Argument> arguments1, @Nullable List<Argument
             GraphQLType typeA = typeAOriginal;
             GraphQLType typeB = fieldAndType.graphQLType;
             if (typeA == null || typeB == null) {
-                if (typeA != typeB) {
-                    return mkNotSameTypeError(path, fields, typeA, typeB);
-                }
                 continue;
             }
             while (true) {
diff --git a/src/test/groovy/graphql/validation/OverlappingFieldsCanBeMergedTest.groovy b/src/test/groovy/graphql/validation/OverlappingFieldsCanBeMergedTest.groovy
@@ -968,22 +968,36 @@ class OverlappingFieldsCanBeMergedTest extends Specification {
         errorCollector.getErrors().size() == 0
     }
 
-    def "fields with null types from unresolvable parent should not produce spurious conflicts"() {
+    def "mixed null and non-null field types from unresolvable fragments should not conflict"() {
         given:
         // Regression test for commit 072165b which changed the null-handling in requireSameOutputTypeShape.
-        // When multiple fields have null graphQLType (e.g. from fragments on unknown types), both typeA
-        // and typeB are null. The old code (072165b) unconditionally reported a conflict when typeB was
-        // null, even if typeA was also null. The fix correctly treats both-null as compatible.
+        // This reproduces the benchmarkDeepAbstractConcrete JMH benchmark scenario: the fragment spreads
+        // on "Whatever" (which doesn't exist in the schema), so the outer "field" has null graphQLType.
+        // After mergeSubSelections, the inner field set contains both null-typed entries (from the
+        // unresolvable parent) and Abstract-typed entries (from the interface inline fragments).
+        // A null type means the parent was unresolvable, so we should skip the comparison rather than
+        // report a spurious conflict.
         def schema = SchemaGenerator.createdMockedSchema('''
-            type Query { field: String }
+            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! }
         ''')
-        // Both fragments spread on "Unknown" which doesn't exist in the schema, so all field
-        // types resolve to null. The overlapping "id" fields should be treated as compatible
-        // since both have null type.
         def query = '''
-        query Test {
-            ... on Unknown { id }
-            ... on Unknown { id }
+        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:
