Skip to content
3 changes: 3 additions & 0 deletions execution/engine/execution_engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,9 @@ func NewExecutionEngine(ctx context.Context, logger abstractlogger.Logger, engin
if engineConfig.plannerConfig.RelaxSubgraphOperationFieldSelectionMergingNullability {
validationOpts = append(validationOpts, astvalidation.WithRelaxFieldSelectionMergingNullability())
}
if engineConfig.plannerConfig.RelaxSubgraphOperationFieldSelectionMergingTypeMismatch {
validationOpts = append(validationOpts, astvalidation.WithRelaxFieldSelectionMergingTypeMismatch())
}

return &ExecutionEngine{
logger: logger,
Expand Down
203 changes: 203 additions & 0 deletions execution/engine/execution_engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,7 @@ type _executionTestOptions struct {
validateRequiredExternalFields bool
computeCosts bool
relaxFieldSelectionMergingNullability bool
relaxFieldSelectionMergingTypeMismatch bool
}

type executionTestOptions func(*_executionTestOptions)
Expand Down Expand Up @@ -261,6 +262,12 @@ func relaxFieldSelectionMergingNullability() executionTestOptions {
}
}

func relaxFieldSelectionMergingTypeMismatch() executionTestOptions {
return func(options *_executionTestOptions) {
options.relaxFieldSelectionMergingTypeMismatch = true
}
}

func TestExecutionEngine_Execute(t *testing.T) {
run := func(testCase ExecutionEngineTestCase, withError bool, expectedErrorMessage string, options ...executionTestOptions) func(t *testing.T) {
t.Helper()
Expand Down Expand Up @@ -298,6 +305,7 @@ func TestExecutionEngine_Execute(t *testing.T) {
engineConf.plannerConfig.ComputeCosts = opts.computeCosts
engineConf.plannerConfig.StaticCostDefaultListSize = 10
engineConf.plannerConfig.RelaxSubgraphOperationFieldSelectionMergingNullability = opts.relaxFieldSelectionMergingNullability
engineConf.plannerConfig.RelaxSubgraphOperationFieldSelectionMergingTypeMismatch = opts.relaxFieldSelectionMergingTypeMismatch
resolveOpts := resolve.ResolverOptions{
MaxConcurrency: 1024,
ResolvableOptions: opts.resolvableOptions,
Expand Down Expand Up @@ -7715,6 +7723,201 @@ func TestExecutionEngine_Execute(t *testing.T) {
relaxFieldSelectionMergingNullability(),
))
})

t.Run("field merging with different types on non-overlapping union types", func(t *testing.T) {
unionSchema := `
enum IssueState { OPEN CLOSED }
enum PullRequestReviewState { PENDING APPROVED }
union Updatable = Issue | PullRequestReview
type Query { updatable: Updatable }
type Issue { id: ID!, state: IssueState, title: String!, priority: Int }
type PullRequestReview { id: ID!, state: PullRequestReviewState, title: String, priority: String }
`
schema, err := graphql.NewSchemaFromString(unionSchema)
require.NoError(t, err)

rootNodes := []plan.TypeField{
{TypeName: "Query", FieldNames: []string{"updatable"}},
{TypeName: "Issue", FieldNames: []string{"id", "state", "title", "priority"}},
{TypeName: "PullRequestReview", FieldNames: []string{"id", "state", "title", "priority"}},
}

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: "updatable",
Path: []string{"updatable"},
},
}

// Strict-mode failure test: use scalar type mismatch (Int vs String) to
// verify that validation rejects different types without the relaxation flag.
scalarMismatchSchema := `
type User { id: ID!, priority: Int }
type Organization { id: ID!, priority: String }
union Entity = User | Organization
type Query { entity: Entity }
`
scalarMismatchParsedSchema, err2 := graphql.NewSchemaFromString(scalarMismatchSchema)
require.NoError(t, err2)

scalarMismatchRootNodes := []plan.TypeField{
{TypeName: "Query", FieldNames: []string{"entity"}},
{TypeName: "User", FieldNames: []string{"id", "priority"}},
{TypeName: "Organization", FieldNames: []string{"id", "priority"}},
}

scalarMismatchConfig := mustConfiguration(t, graphql_datasource.ConfigurationInput{
Fetch: &graphql_datasource.FetchConfiguration{
URL: "https://example.com/",
Method: "POST",
},
SchemaConfiguration: mustSchemaConfig(t, nil, scalarMismatchSchema),
})

scalarMismatchFieldConfig := []plan.FieldConfiguration{
{
TypeName: "Query",
FieldName: "entity",
Path: []string{"entity"},
},
}

t.Run("without relaxation flag, validation rejects different scalar types", runWithAndCompareError(
ExecutionEngineTestCase{
schema: scalarMismatchParsedSchema,
operation: func(t *testing.T) graphql.Request {
return graphql.Request{
OperationName: "O",
Query: `query O { entity { ... on User { priority } ... on Organization { priority } } }`,
}
},
dataSources: []plan.DataSource{
mustGraphqlDataSourceConfiguration(t, "ds-id",
mustFactory(t,
testNetHttpClient(t, roundTripperTestCase{
expectedHost: "example.com",
expectedPath: "/",
expectedBody: "",
sendResponseBody: `{"data":{"entity":{"__typename":"User","priority":1}}}`,
sendStatusCode: 200,
}),
),
&plan.DataSourceMetadata{
RootNodes: scalarMismatchRootNodes,
},
scalarMismatchConfig,
),
},
fields: scalarMismatchFieldConfig,
},
`fields 'priority' conflict because they return conflicting types 'Int' and 'String', locations: [], path: [query,entity,$1Organization]`,
))

t.Run("without relaxation flag, validation rejects different scalar types on union members", runWithAndCompareError(
ExecutionEngineTestCase{
schema: schema,
operation: func(t *testing.T) graphql.Request {
return graphql.Request{
OperationName: "O",
Query: `query O { updatable { ... on Issue { priority } ... on PullRequestReview { priority } } }`,
}
},
dataSources: []plan.DataSource{
mustGraphqlDataSourceConfiguration(t, "ds-id",
mustFactory(t,
testNetHttpClient(t, roundTripperTestCase{
expectedHost: "example.com",
expectedPath: "/",
expectedBody: "",
sendResponseBody: `{"data":{"updatable":{"__typename":"Issue","priority":1}}}`,
sendStatusCode: 200,
}),
),
&plan.DataSourceMetadata{
RootNodes: rootNodes,
},
customConfig,
),
},
fields: fieldConfig,
},
`fields 'priority' conflict because they return conflicting types 'Int' and 'String', locations: [], path: [query,updatable,$1PullRequestReview]`,
))

t.Run("with relaxation flag, different enum types work", runWithoutError(
Comment thread
jensneuse marked this conversation as resolved.
ExecutionEngineTestCase{
schema: schema,
operation: func(t *testing.T) graphql.Request {
return graphql.Request{
OperationName: "O",
Query: `query O { updatable { ... on Issue { state } ... on PullRequestReview { state } } }`,
}
},
dataSources: []plan.DataSource{
mustGraphqlDataSourceConfiguration(t, "ds-id",
mustFactory(t,
testNetHttpClient(t, roundTripperTestCase{
expectedHost: "example.com",
expectedPath: "/",
expectedBody: "",
sendResponseBody: `{"data":{"updatable":{"__typename":"Issue","state":"OPEN"}}}`,
sendStatusCode: 200,
}),
),
&plan.DataSourceMetadata{
RootNodes: rootNodes,
},
customConfig,
),
},
fields: fieldConfig,
expectedResponse: `{"data":{"updatable":{"state":"OPEN"}}}`,
},
relaxFieldSelectionMergingTypeMismatch(),
))

t.Run("with both relaxation flags, different enum types and nullability work", runWithoutError(
ExecutionEngineTestCase{
schema: schema,
operation: func(t *testing.T) graphql.Request {
return graphql.Request{
OperationName: "O",
Query: `query O { updatable { ... on Issue { state title } ... on PullRequestReview { state title } } }`,
}
},
dataSources: []plan.DataSource{
mustGraphqlDataSourceConfiguration(t, "ds-id",
mustFactory(t,
testNetHttpClient(t, roundTripperTestCase{
expectedHost: "example.com",
expectedPath: "/",
expectedBody: "",
sendResponseBody: `{"data":{"updatable":{"__typename":"Issue","state":"OPEN","title":"Bug report"}}}`,
sendStatusCode: 200,
}),
),
&plan.DataSourceMetadata{
RootNodes: rootNodes,
},
customConfig,
),
},
fields: fieldConfig,
expectedResponse: `{"data":{"updatable":{"state":"OPEN","title":"Bug report"}}}`,
},
relaxFieldSelectionMergingNullability(),
relaxFieldSelectionMergingTypeMismatch(),
))
})
}

func testNetHttpClient(t *testing.T, testCase roundTripperTestCase) *http.Client {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Justification: Relaxed Nullability Validation on Non-Overlapping Concrete Types
# Justification: Relaxed Field Selection Merging on Non-Overlapping Concrete Types

## Problem Statement

Expand Down Expand Up @@ -130,9 +130,12 @@ fails (e.g. `String!` vs `String`), we now 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.
2. **If they can't overlap** (nullability relaxation only): use
`TypesAreCompatibleIgnoringNullability` which strips `NonNull` wrappers at
every nesting level but still requires the same list structure and base named
type. Note: this "base types must match" guarantee applies only to
`RelaxNullabilityCheck`. The broader `RelaxTypeMismatchCheck` permits
entirely different base named types on non-overlapping concrete types.

Existing test cases are unaffected:

Expand All @@ -142,10 +145,47 @@ Existing test cases are unaffected:
| `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** |

## Type Mismatch Relaxation (RelaxTypeMismatchCheck)

In addition to the nullability relaxation above, a broader relaxation is
available via `RelaxTypeMismatchCheck`. This allows **completely different types**
(e.g. `IssueState` vs `PullRequestReviewState`) on fields in non-overlapping
concrete object types.

### Problem

Schemas like GitHub's define interfaces (e.g. `Updatable`) whose implementing
types have same-named fields returning different leaf types:

```graphql
interface Updatable { id: ID! }
type Issue implements Updatable { id: ID!, state: IssueState }
type PullRequestReview implements Updatable { id: ID!, state: PullRequestReviewState }
```

The spec's `SameResponseShape` algorithm rejects `state` across inline fragments
even though only one branch executes at runtime.

### Safety

The same `potentiallySameObject` guard applies: the relaxation only fires when
both enclosing types are different concrete Object types (never for interfaces).
At runtime, the response object can only match one concrete type, so different
field types cannot both appear for the same object.

### Relationship to nullability relaxation

`RelaxTypeMismatchCheck` is strictly broader than `RelaxNullabilityCheck`. When
the type mismatch flag is enabled on non-overlapping types, **all** type
differences (including nullability) are skipped because the code `continue`s
before reaching the nullability check. Both flags exist because callers may want
the narrower nullability-only relaxation without the broader type mismatch
relaxation.
Comment thread
coderabbitai[bot] marked this conversation as resolved.

## 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.
These are **deliberate, targeted deviations** from the GraphQL specification.
They address real-world pain points that have been independently reported across
multiple GraphQL implementations. Both relaxations are narrowly scoped (only
Comment thread
jensneuse marked this conversation as resolved.
different concrete object types) and preserve all existing rejection behavior
for genuinely conflicting types on overlapping or same types.
Loading