[Joel Town Road (hippi345)]

Why make this change?

• Fixes [Bug]: GraphQL fields listed after a fragment spread are silently dropped #3861
  • In SqlQueryStructure.AddGraphQLFields, expanding a fragment spread or inline fragment used return;, which exits the selection foreach instead of moving to the next node.
  • HTTP GraphQL queries on current main do not reproduce this in our testing (Hot Chocolate 16 appears to merge fragment selections before AddGraphQLFields runs). The issue was reported on 2.1.5, which uses the same Hot Chocolate version, so the HTTP reproduction may depend on conditions we couldn't recreate. The code bug is real, and the unit test covers the loop.

What is this change?

• After expanding a FragmentSpread or InlineFragment, AddGraphQLFields continues to the next selection instead of returning from the method.
• Unit test AddGraphQLFields_ContinuesAfterInlineFragment calls AddGraphQLFields directly with an inline fragment followed by another field, covering the selection loop.

How was this tested?

• Integration Tests
• Unit Tests

dotnet test src/Service.Tests/Azure.DataApiBuilder.Service.Tests.csproj \
  --filter "FullyQualifiedName~AddGraphQLFields_ContinuesAfterInlineFragment" \
  --configuration Debug

dotnet test src/Service.Tests/Azure.DataApiBuilder.Service.Tests.csproj \
  --filter "FullyQualifiedName~SqlQueryStructureHelperTests" \
  --configuration Debug

With the fix reverted (return; in both fragment branches), the unit test failed because the trailing field was dropped:

Failed AddGraphQLFields_ContinuesAfterInlineFragment [103 ms]
Error Message:
 CollectionAssert.AreEquivalent failed. The number of elements in the collections do not match. Expected:<2>. Actual:<1>.

Test Run Failed.
Total tests: 1
     Failed: 1

With continue; applied:

Passed AddGraphQLFields_ContinuesAfterInlineFragment [5 ms]
...
Test Run Successful.
Total tests: 34
     Passed: 34

(The 34-test run is FullyQualifiedName~SqlQueryStructureHelperTests, which includes SqlQueryStructureHelperTests and nearby BaseSqlQueryStructureHelperTests.)

Sample Request(s)

GraphQL query that would drop trailing fields when AddGraphQLFields receives unexpanded fragments (exercised directly by the unit test):

query {
  book_by_pk(id: 1) {
    ...bookFields
    title
    publishers { name }
  }
}

fragment bookFields on book { id }

On current main with Hot Chocolate 16, the engine merges those fragment selections before AddGraphQLFields, so this HTTP query is not a reproduction. The unit test is the coverage for the loop.

This change was prepared with AI assistance. I remain responsible for the contribution and have reviewed it.

[azure-pipelines]

Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

[Joel Town Road (hippi345)]

@microsoft-github-policy-service agree

[Copilot]

🟢 Approval recommended

The focused control-flow fix preserves trailing selections, and the regression test directly covers the failure.

No open issues 🎉

What changed in this PR

Fixes a latent SQL GraphQL selection bug from #3861 that drops fields following fragments.

Changes:

• Continues processing selections after named and inline fragments.
• Adds a direct regression test for fields following an inline fragment, bypassing Hot Chocolate 16’s fragment normalization.

File	Description
src/​Service.Tests/​UnitTests/​SqlQueryStructureHelperTests.cs	Adds regression coverage and supporting metadata setup.
src/​Core/​Resolvers/​Sql Query Structures/​SqlQueryStructure.cs	Replaces early returns with loop continuation after fragment expansion.

_{Review effort: Balanced}

[Mohit G (gmohit21)]

Hi Joel Town Road (@hippi345) , The changes LGTM—the return → continue fix makes sense for both fragment branches. Could you please address the following as well?

1. CI validation: I only see the CLA and reviewer-assignment checks on this PR. Could we get the required build and test pipelines running and confirm they pass before merging?
2. Named-fragment coverage: Could you add a regression test for a named fragment spread followed by another field, matching the example in #3861? The new test covers inline fragments, but not named spreads. Please verify that the test fails with the original return and passes with continue, asserting that both the fragment’s fields and the trailing field are included.

Thanks!

[Joel Town Road (hippi345)]

The new commit is 8c83517. Fork PRs here don't start the build and test pipelines until a project member comments /azp run. If you can comment /azp run, I'll watch them.

Added AddGraphQLFields_ContinuesAfterNamedFragmentSpread for the #3861 shape: ...bookFields selecting id, then title. Both columns have to be present. With the old return the test fails (Expected:<2>. Actual:<1>). With continue it passes.