From 44e0700003956a665959966e7d47896b5a9c6ddb Mon Sep 17 00:00:00 2001 From: tobias-tengler <45513122+tobias-tengler@users.noreply.github.com> Date: Wed, 25 Jun 2025 10:59:40 +0200 Subject: [PATCH 1/2] Add reproduction --- .../test/Core.Tests/DemoIntegrationTests.cs | 177 +++++++++---- ...ultiple_Variable_Values_Some_Items_Null.md | 235 ++++++++++++++++++ 2 files changed, 368 insertions(+), 44 deletions(-) create mode 100644 src/HotChocolate/Fusion/test/Core.Tests/__snapshots__/DemoIntegrationTests.BatchExecutionState_With_Multiple_Variable_Values_Some_Items_Null.md diff --git a/src/HotChocolate/Fusion/test/Core.Tests/DemoIntegrationTests.cs b/src/HotChocolate/Fusion/test/Core.Tests/DemoIntegrationTests.cs index bba26390ebf..18c943b0358 100644 --- a/src/HotChocolate/Fusion/test/Core.Tests/DemoIntegrationTests.cs +++ b/src/HotChocolate/Fusion/test/Core.Tests/DemoIntegrationTests.cs @@ -134,21 +134,21 @@ type Product implements Node { using var subgraphs = new TestSubgraphCollection(output, [subgraphA, subgraphB]); var executor = await subgraphs.GetExecutorAsync(); var request = Parse(""" - query { - productsA { - id - name - price - reviewCount - } - productsB { - id - name - price - reviewCount - } - } - """); + query { + productsA { + id + name + price + reviewCount + } + productsB { + id + name + price + reviewCount + } + } + """); // act var result = await executor.ExecuteAsync( @@ -260,10 +260,10 @@ public async Task Authors_And_Reviews_Query_GetUserReviews_Report_Cost() // act var fusionGraph = await new FusionGraphComposer(logFactory: _logFactory) .ComposeAsync( - [ - demoProject.Reviews2.ToConfiguration(Reviews2ExtensionWithCostSdl), - demoProject.Accounts.ToConfiguration(AccountsExtensionWithCostSdl) - ]); + [ + demoProject.Reviews2.ToConfiguration(Reviews2ExtensionWithCostSdl), + demoProject.Accounts.ToConfiguration(AccountsExtensionWithCostSdl) + ]); var executor = await new ServiceCollection() .AddSingleton(demoProject.HttpClientFactory) @@ -1656,11 +1656,7 @@ public async Task Forward_Nested_Variables_No_OpName() .New() .SetDocument(request) .SetVariableValues( - new Dictionary - { - { "id", "UHJvZHVjdDox" }, - { "first", 1 }, - }) + new Dictionary { { "id", "UHJvZHVjdDox" }, { "first", 1 }, }) .Build()); // assert @@ -2028,10 +2024,7 @@ public async Task GetFirstPage_With_After_Null() // act var fusionGraph = await new FusionGraphComposer(logFactory: _logFactory).ComposeAsync( - new[] - { - demoProject.Appointment.ToConfiguration(), - }, + new[] { demoProject.Appointment.ToConfiguration(), }, new FusionFeatureCollection(FusionFeatures.NodeField)); var executor = await new ServiceCollection() @@ -2232,6 +2225,114 @@ type ResaleSurveyFeedback { await snapshot.MatchMarkdownAsync(); } + [Fact] + public async Task BatchExecutionState_With_Multiple_Variable_Values_Some_Items_Null() + { + // arrange + var subgraphA = await TestSubgraph.CreateAsync( + """ + type Query { + node(id: ID!): Node + nodes(ids: [ID!]!): [Node]! + } + + interface Node { + id: ID! + } + + type User implements Node { + id: ID! + displayName: String! + } + """); + var subgraphB = await TestSubgraph.CreateAsync( + """ + type Query { + node(id: ID!): Node + nodes(ids: [ID!]!): [Node]! @null(atIndex: 1) + userBySlug(slug: String!): User + } + + interface Node { + id: ID! + } + + type User implements Node { + relativeUrl: String! + id: ID! + } + """); + var subgraphC = await TestSubgraph.CreateAsync( + """ + type Query { + node(id: ID!): Node + nodes(ids: [ID!]!): [Node]! + } + + interface Node { + id: ID! + } + + type User implements Node { + id: ID! + feedbacks: FeedbacksConnection + } + + type FeedbacksConnection { + edges: [FeedbacksEdge!] + } + + type FeedbacksEdge { + node: ResaleFeedback! + } + + type ResaleFeedback implements Node { + feedback: ResaleSurveyFeedback + id: ID! + } + + type ResaleSurveyFeedback { + buyer: User + } + """); + + using var subgraphs = new TestSubgraphCollection(output, [subgraphA, subgraphB, subgraphC]); + var executor = await subgraphs.GetExecutorAsync(); + + var request = Parse( + """ + query { + userBySlug(slug: "me") { + feedbacks { + edges { + node { + feedback { + buyer { + relativeUrl + displayName + } + } + } + } + } + } + } + + """); + + // act + await using var result = await executor.ExecuteAsync( + OperationRequestBuilder + .New() + .SetDocument(request) + .Build()); + + // assert + var snapshot = new Snapshot(); + CollectSnapshotData(snapshot, request, result); + await snapshot.MatchMarkdownAsync(); + } + [Fact] public async Task BatchExecutionState_With_Multiple_Variable_Values_And_Forwarded_Variable() { @@ -2401,10 +2502,7 @@ type Query { } } """) - .SetVariableValues(new Dictionary - { - ["productId"] = "UHJvZHVjdAppMzg2MzE4NTk=" - }) + .SetVariableValues(new Dictionary { ["productId"] = "UHJvZHVjdAppMzg2MzE4NTk=" }) .Build(); // act @@ -2478,10 +2576,7 @@ type Query { } } """) - .SetVariableValues(new Dictionary - { - ["productId"] = "UHJvZHVjdAppMzg2MzE4NTk=" - }) + .SetVariableValues(new Dictionary { ["productId"] = "UHJvZHVjdAppMzg2MzE4NTk=" }) .Build(); // act @@ -2559,10 +2654,7 @@ type Query { } } """) - .SetVariableValues(new Dictionary - { - ["productId"] = "UHJvZHVjdAppMzg2MzE4NTk=" - }) + .SetVariableValues(new Dictionary { ["productId"] = "UHJvZHVjdAppMzg2MzE4NTk=" }) .Build(); // act @@ -2580,10 +2672,7 @@ public async Task Two_Arguments_Differing_Nullability_Does_Not_Duplicate_Forward // act var fusionGraph = await new FusionGraphComposer(logFactory: _logFactory).ComposeAsync( - new[] - { - demoProject.Accounts.ToConfiguration(AccountsExtensionSdl) - }); + new[] { demoProject.Accounts.ToConfiguration(AccountsExtensionSdl) }); var executor = await new ServiceCollection() .AddSingleton(demoProject.HttpClientFactory) diff --git a/src/HotChocolate/Fusion/test/Core.Tests/__snapshots__/DemoIntegrationTests.BatchExecutionState_With_Multiple_Variable_Values_Some_Items_Null.md b/src/HotChocolate/Fusion/test/Core.Tests/__snapshots__/DemoIntegrationTests.BatchExecutionState_With_Multiple_Variable_Values_Some_Items_Null.md new file mode 100644 index 00000000000..d9352dd0887 --- /dev/null +++ b/src/HotChocolate/Fusion/test/Core.Tests/__snapshots__/DemoIntegrationTests.BatchExecutionState_With_Multiple_Variable_Values_Some_Items_Null.md @@ -0,0 +1,235 @@ +# BatchExecutionState_With_Multiple_Variable_Values_Some_Items_Null + +## Result + +```json +{ + "errors": [ + { + "message": "Cannot return null for non-nullable field.", + "locations": [ + { + "line": 8, + "column": 15 + } + ], + "path": [ + "userBySlug", + "feedbacks", + "edges", + 1, + "node", + "feedback", + "buyer", + "relativeUrl" + ], + "extensions": { + "code": "HC0018" + } + }, + { + "message": "Cannot return null for non-nullable field.", + "locations": [ + { + "line": 9, + "column": 15 + } + ], + "path": [ + "userBySlug", + "feedbacks", + "edges", + 2, + "node", + "feedback", + "buyer", + "displayName" + ], + "extensions": { + "code": "HC0018" + } + }, + { + "message": "Cannot return null for non-nullable field.", + "locations": [ + { + "line": 9, + "column": 15 + } + ], + "path": [ + "userBySlug", + "feedbacks", + "edges", + 0, + "node", + "feedback", + "buyer", + "displayName" + ], + "extensions": { + "code": "HC0018" + } + }, + { + "message": "Unexpected Execution Error" + } + ], + "data": { + "userBySlug": { + "feedbacks": { + "edges": [ + { + "node": { + "feedback": { + "buyer": null + } + } + }, + { + "node": { + "feedback": { + "buyer": null + } + } + }, + { + "node": { + "feedback": { + "buyer": null + } + } + } + ] + } + } + } +} +``` + +## Request + +```graphql +{ + userBySlug(slug: "me") { + feedbacks { + edges { + node { + feedback { + buyer { + relativeUrl + displayName + } + } + } + } + } + } +} +``` + +## QueryPlan Hash + +```text +ECA485AE6695B779A8D5B148BC2D37D52EA81380 +``` + +## QueryPlan + +```json +{ + "document": "{ userBySlug(slug: \u0022me\u0022) { feedbacks { edges { node { feedback { buyer { relativeUrl displayName } } } } } } }", + "rootNode": { + "type": "Sequence", + "nodes": [ + { + "type": "Resolve", + "subgraph": "Subgraph_2", + "document": "query fetch_userBySlug_1 { userBySlug(slug: \u0022me\u0022) { __fusion_exports__1: id } }", + "selectionSetId": 0, + "provides": [ + { + "variable": "__fusion_exports__1" + } + ] + }, + { + "type": "Compose", + "selectionSetIds": [ + 0 + ] + }, + { + "type": "Resolve", + "subgraph": "Subgraph_3", + "document": "query fetch_userBySlug_2($__fusion_exports__1: ID!) { node(id: $__fusion_exports__1) { ... on User { feedbacks { edges { node { feedback { buyer { __fusion_exports__3: id } } } } } } } }", + "selectionSetId": 1, + "path": [ + "node" + ], + "requires": [ + { + "variable": "__fusion_exports__1" + } + ], + "provides": [ + { + "variable": "__fusion_exports__3" + } + ] + }, + { + "type": "Compose", + "selectionSetIds": [ + 1 + ] + }, + { + "type": "ResolveByKeyBatch", + "subgraph": "Subgraph_2", + "document": "query fetch_userBySlug_4($__fusion_exports__3: [ID!]!) { nodes(ids: $__fusion_exports__3) { ... on User { relativeUrl __fusion_exports__2: id __fusion_exports__3: id } } }", + "selectionSetId": 6, + "path": [ + "nodes" + ], + "requires": [ + { + "variable": "__fusion_exports__3" + } + ], + "provides": [ + { + "variable": "__fusion_exports__2" + } + ] + }, + { + "type": "ResolveByKeyBatch", + "subgraph": "Subgraph_1", + "document": "query fetch_userBySlug_3($__fusion_exports__2: [ID!]!) { nodes(ids: $__fusion_exports__2) { ... on User { displayName __fusion_exports__2: id } } }", + "selectionSetId": 6, + "path": [ + "nodes" + ], + "requires": [ + { + "variable": "__fusion_exports__2" + } + ] + }, + { + "type": "Compose", + "selectionSetIds": [ + 6 + ] + } + ] + }, + "state": { + "__fusion_exports__1": "User_id", + "__fusion_exports__2": "User_id", + "__fusion_exports__3": "User_id" + } +} +``` + From 7e240d840796948e66b54cf2ab1fb083dc4ac666 Mon Sep 17 00:00:00 2001 From: tobias-tengler <45513122+tobias-tengler@users.noreply.github.com> Date: Wed, 25 Jun 2025 12:02:09 +0200 Subject: [PATCH 2/2] Implement fix --- .../Core/Execution/Nodes/ResolveByKeyBatch.cs | 44 +++++++------- ...ultiple_Variable_Values_Some_Items_Null.md | 57 +++---------------- 2 files changed, 27 insertions(+), 74 deletions(-) diff --git a/src/HotChocolate/Fusion/src/Core/Execution/Nodes/ResolveByKeyBatch.cs b/src/HotChocolate/Fusion/src/Core/Execution/Nodes/ResolveByKeyBatch.cs index 387ac6b78e4..4ad8c480109 100644 --- a/src/HotChocolate/Fusion/src/Core/Execution/Nodes/ResolveByKeyBatch.cs +++ b/src/HotChocolate/Fusion/src/Core/Execution/Nodes/ResolveByKeyBatch.cs @@ -142,12 +142,12 @@ private static void InitializeRequests(FusionExecutionContext context, List batchExecutionState, string subgraphName) { var result = UnwrapResult(response, Requires); - ref var batchState = ref MemoryMarshal.GetArrayDataReference(batchExecutionState); - ref var end = ref Unsafe.Add(ref batchState, batchExecutionState.Length); + ref var batchState = ref MemoryMarshal.GetReference(CollectionsMarshal.AsSpan(batchExecutionState)); + ref var end = ref Unsafe.Add(ref batchState, batchExecutionState.Count); var pathLength = Path.Length; var first = true; @@ -195,13 +195,13 @@ private void ProcessResult( } private static Dictionary BuildVariables( - BatchExecutionState[] batchExecutionState, + List batchExecutionState, string[] requires, Dictionary argumentTypes) { var first = batchExecutionState[0]; - if (batchExecutionState.Length == 1) + if (batchExecutionState.Count == 1) { return first.VariableValues; } @@ -210,8 +210,8 @@ private static Dictionary BuildVariables( var uniqueState = new List(); var processed = new HashSet(); - ref var batchState = ref MemoryMarshal.GetArrayDataReference(batchExecutionState); - ref var end = ref Unsafe.Add(ref batchState, batchExecutionState.Length); + ref var batchState = ref MemoryMarshal.GetReference(CollectionsMarshal.AsSpan(batchExecutionState)); + ref var end = ref Unsafe.Add(ref batchState, batchExecutionState.Count); while (Unsafe.IsAddressLessThan(ref batchState, ref end)) { @@ -339,44 +339,38 @@ private static JsonElement LiftData(JsonElement data, string[] path) return current; } - private static BatchExecutionState[] CreateBatchBatchState(List executionState, string[] requires) + private static List CreateBatchBatchState(List executionState, string[] requires) { - var batchExecutionState = new BatchExecutionState[executionState.Count]; + var batchExecutionState = new List(executionState.Count); ref var state = ref MemoryMarshal.GetReference(CollectionsMarshal.AsSpan(executionState)); - ref var batchState = ref MemoryMarshal.GetArrayDataReference(batchExecutionState); ref var end = ref Unsafe.Add(ref state, executionState.Count); if (requires.Length == 1) { while (Unsafe.IsAddressLessThan(ref state, ref end)) { - var key = FormatKeyValue(state.VariableValues[requires[0]]); - batchState = new BatchExecutionState(key, state); + if (state.VariableValues.ContainsKey(requires[0])) + { + var key = FormatKeyValue(state.VariableValues[requires[0]]); + var batchState = new BatchExecutionState(key, state); + + batchExecutionState.Add(batchState); + } state = ref Unsafe.Add(ref state, 1)!; - batchState = ref Unsafe.Add(ref batchState, 1)!; } } else { - var keyBuilder = new StringBuilder(); - while (Unsafe.IsAddressLessThan(ref state, ref end)) { - ref var key = ref MemoryMarshal.GetArrayDataReference(requires); - ref var keyEnd = ref Unsafe.Add(ref key, requires.Length); - - while (Unsafe.IsAddressLessThan(ref key, ref keyEnd)) - { - keyBuilder.Append(FormatKeyValue(state.VariableValues[key])); - key = ref Unsafe.Add(ref key, 1)!; - } + var lastKey = requires[^1]; + var batchState = new BatchExecutionState(lastKey, state); - batchState = new BatchExecutionState(key, state); + batchExecutionState.Add(batchState); state = ref Unsafe.Add(ref state, 1)!; - batchState = ref Unsafe.Add(ref batchState, 1)!; } } diff --git a/src/HotChocolate/Fusion/test/Core.Tests/__snapshots__/DemoIntegrationTests.BatchExecutionState_With_Multiple_Variable_Values_Some_Items_Null.md b/src/HotChocolate/Fusion/test/Core.Tests/__snapshots__/DemoIntegrationTests.BatchExecutionState_With_Multiple_Variable_Values_Some_Items_Null.md index d9352dd0887..9bf5f92660a 100644 --- a/src/HotChocolate/Fusion/test/Core.Tests/__snapshots__/DemoIntegrationTests.BatchExecutionState_With_Multiple_Variable_Values_Some_Items_Null.md +++ b/src/HotChocolate/Fusion/test/Core.Tests/__snapshots__/DemoIntegrationTests.BatchExecutionState_With_Multiple_Variable_Values_Some_Items_Null.md @@ -26,53 +26,6 @@ "extensions": { "code": "HC0018" } - }, - { - "message": "Cannot return null for non-nullable field.", - "locations": [ - { - "line": 9, - "column": 15 - } - ], - "path": [ - "userBySlug", - "feedbacks", - "edges", - 2, - "node", - "feedback", - "buyer", - "displayName" - ], - "extensions": { - "code": "HC0018" - } - }, - { - "message": "Cannot return null for non-nullable field.", - "locations": [ - { - "line": 9, - "column": 15 - } - ], - "path": [ - "userBySlug", - "feedbacks", - "edges", - 0, - "node", - "feedback", - "buyer", - "displayName" - ], - "extensions": { - "code": "HC0018" - } - }, - { - "message": "Unexpected Execution Error" } ], "data": { @@ -82,7 +35,10 @@ { "node": { "feedback": { - "buyer": null + "buyer": { + "relativeUrl": "string", + "displayName": "string" + } } } }, @@ -96,7 +52,10 @@ { "node": { "feedback": { - "buyer": null + "buyer": { + "relativeUrl": "string", + "displayName": "string" + } } } }