Bug fix for pagination nested entities resulting key not found error.

[anushakolan]

Why make this change?

We are addressing 2 related issues in this PR:

Issue #2374 – Nested sibling relationships under books (websiteplacement, reviews, authors)

Problem: A nested query on books where a parent has multiple sibling relationships (for example, websiteplacement, reviews, and authors) could throw a KeyNotFoundException when RBAC or shape changes were involved. Pagination metadata was stored using only the root and the depth in the path, so different sibling relationships at the same depth could overwrite each other or look up the wrong entry.

Solution: We now key pagination metadata by both depth and the full relationship path (for example, “books → items → reviews” vs “books → items → authors”), so each sibling branch gets its own unique entry. Reads use the same full-path key, and if metadata for a branch is missing, we return an “empty” PaginationMetadata instead of throwing. This prevents collisions between sibling relationships and avoids runtime errors when a particular branch has no metadata.

Issue #3026 – Person's graph (AddressType / PhoneNumberType)

Problem: In the persons graph, a query selecting persons → addresses.items.AddressType and persons → phoneNumbers.items.PhoneNumberType could also throw a KeyNotFoundException. In some cases (for example, when RBAC removes a relationship or when that relationship is not paginated at all), there is legitimately no pagination metadata for that nested field, but the code assumed it always existed and indexed into the dictionary directly.

Solution: Metadata handling is now defensive in two places:
In the GraphQL execution helper, metadata lookups for object and list fields use safe TryGet-style access; if an entry isn’t present, we fall back to an empty PaginationMetadata instead of failing.
In the SQL query engine’s object resolver, we first check whether there is a subquery metadata entry for the field. If there isn’t, we treat the field as non‑paginated and return the JSON as-is rather than throwing.

Together, these changes fix both issues by
(a) using full path-based keys, so sibling branches don’t conflict,
(b) treating missing metadata as “no pagination here” rather than as a fatal error.

What is this change?

1. In SqlQueryEngine.ResolveObject, instead of always doing parentMetadata.Subqueries[fieldName] (which crashed when RBAC caused that entry to be missing), it now uses TryGetValue and:
   • If metadata exists and IsPaginated is true -> wrap the JSON as a pagination connection.
   • If metadata is missing -> just return the JSON as-is (no exception).
2. Introduced GetRelationshipPathSuffix(HotChocolate.Path path) to build a relationship path suffix like:
   • rel1 for /entity/items[0]/rel1
   • rel1::nested for /entity/items[0]/rel1/nested
3. SetNewMetadataChildren, now stores child metadata under keys of the form
   • root_PURE_RESOLVER_CTX::depth::relationshipPath, ensuring siblings at the same depth get distinct entries.
4. GetMetadata (used for list items fields):
   • For Selection.ResponseName == "items" and non-root paths, now looks up:
     a. GetMetadataKey(context.Path) + "::" + context.Path.Parent.Depth()
     plus the relationship suffix from GetRelationshipPathSuffix(context.Path.Parent).
     b. Uses ContextData.TryGetValue(...) and falls back to PaginationMetadata.MakeEmptyPaginationMetadata() when metadata is missing (e.g. Cosmos, pruned relationships).
5. GetMetadataObjectField (used for object fields like addresses, AddressType, PhoneNumberType):
   Updated all branches (indexer, nested non-root, root) to:
   • Append the relationship suffix to the base key (so keys align with SetNewMetadataChildren).
   • Use ContextData.TryGetValue(...) instead of direct indexing, return PaginationMetadata.MakeEmptyPaginationMetadata() when no metadata exists, instead of throwing.
6. Added a new test case in MsSqlGraphQLQueryTests, an integration test which queries books with multiple sibling nested relationships (websiteplacement, reviews, authors) under the authenticated role to:
   • Assert no KeyNotFoundException,
   • Verify all nested branches return data.

How was this tested?

Tested both manually and added an integration test (NestedReviewsConnection_WithSiblings_PaginatesMoreThanHundredItems).

Manually if we run this query without the bug fix:
query { persons { items { PersonID FirstName LastName addresses { items { AddressID City AddressType { AddressTypeID TypeName } } } phoneNumbers { items { PhoneNumberID PhoneNumber PhoneNumberType { PhoneNumberTypeID TypeName } } } } } }

We get the following response:

{ "errors": [ { "message": "The given key 'AddressType' was not present in the dictionary.", "locations": [ { "line": 11, "column": 11 } ], "path": [ "persons", "items", 0, "addresses", "items", 1, "AddressType" ] }, { "message": "The given key 'AddressType' was not present in the dictionary.", "locations": [ { "line": 11, "column": 11 } ], "path": [ "persons", "items", 0, "addresses", "items", 0, "AddressType" ] }, { "message": "The given key 'AddressType' was not present in the dictionary.", "locations": [ { "line": 11, "column": 11 } ], "path": [ "persons", "items", 1, "addresses", "items", 0, "AddressType" ] } ], "data": { "persons": { "items": [ { "PersonID": 1, "FirstName": "John", "LastName": "Doe", "addresses": { "items": [ { "AddressID": 1, "City": "New York", "AddressType": null }, { "AddressID": 2, "City": "New York", "AddressType": null } ] }, "phoneNumbers": { "items": [ { "PhoneNumberID": 1, "PhoneNumber": "123-456-7890", "PhoneNumberType": { "PhoneNumberTypeID": 1, "TypeName": "Mobile" } }, { "PhoneNumberID": 2, "PhoneNumber": "111-222-3333", "PhoneNumberType": { "PhoneNumberTypeID": 3, "TypeName": "Work" } } ] } }, { "PersonID": 2, "FirstName": "Jane", "LastName": "Smith", "addresses": { "items": [ { "AddressID": 3, "City": "Los Angeles", "AddressType": null } ] }, "phoneNumbers": { "items": [ { "PhoneNumberID": 3, "PhoneNumber": "987-654-3210", "PhoneNumberType": { "PhoneNumberTypeID": 2, "TypeName": "Home" } } ] } } ] } } }

After the bug fix, we get,

{ "data": { "persons": { "items": [ { "PersonID": 1, "FirstName": "John", "LastName": "Doe", "addresses": { "items": [ { "AddressID": 1, "City": "New York", "AddressType": { "AddressTypeID": 1, "TypeName": "Home" } }, { "AddressID": 2, "City": "New York", "AddressType": { "AddressTypeID": 2, "TypeName": "Work" } } ] }, "phoneNumbers": { "items": [ { "PhoneNumberID": 1, "PhoneNumber": "123-456-7890", "PhoneNumberType": { "PhoneNumberTypeID": 1, "TypeName": "Mobile" } }, { "PhoneNumberID": 2, "PhoneNumber": "111-222-3333", "PhoneNumberType": { "PhoneNumberTypeID": 3, "TypeName": "Work" } } ] } }, { "PersonID": 2, "FirstName": "Jane", "LastName": "Smith", "addresses": { "items": [ { "AddressID": 3, "City": "Los Angeles", "AddressType": { "AddressTypeID": 1, "TypeName": "Home" } } ] }, "phoneNumbers": { "items": [ { "PhoneNumberID": 3, "PhoneNumber": "987-654-3210", "PhoneNumberType": { "PhoneNumberTypeID": 2, "TypeName": "Home" } } ] } } ] } } }

Sample Request(s)

• Example REST and/or GraphQL request to demonstrate modifications
• Example of CLI usage to demonstrate modifications

[anushakolan]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 6 pipeline(s).

[Copilot]

Pull request overview

This PR fixes a KeyNotFoundException that occurred in GraphQL queries with multiple nested sibling relationships under RBAC (Role-Based Access Control). The fix changes the approach from direct dictionary access to using TryGetValue, allowing graceful handling of scenarios where pagination metadata may be missing due to authorization filtering.

Key Changes:

• Modified SqlQueryEngine.ResolveObject to use TryGetValue instead of direct dictionary indexing for accessing pagination metadata
• Added defensive handling to return elements as-is when metadata is unavailable
• Introduced an integration test to verify the fix and prevent regression

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
src/Core/Resolvers/SqlQueryEngine.cs	Replaced direct dictionary access with TryGetValue pattern to handle missing pagination metadata gracefully, preventing KeyNotFoundException when RBAC filters affect nested relationships
src/Service.Tests/SqlTests/GraphQLQueryTests/MsSqlGraphQLQueryTests.cs	Added integration test NestedSiblingRelationshipsWithRbac_DoNotThrowAndMaterialize to verify multiple nested sibling relationships work correctly under authenticated role

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

@anushakolan I've opened a new pull request, #3030, to work on those changes. Once the pull request is ready, I'll request review from you.

[Copilot]

@anushakolan I've opened a new pull request, #3031, to work on those changes. Once the pull request is ready, I'll request review from you.

[anushakolan]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 6 pipeline(s).

[Aniruddh25]

In the description, After the bug fix, we get, - we still see an error message. not the response after fixing the bug,

[anushakolan]

In the description, After the bug fix, we get, - we still see an error message. not the response after fixing the bug,

Fixed

[anushakolan]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 6 pipeline(s).

[Aniruddh25]

Waiting for a review of the new issue to better understand the root cause.

[anushakolan]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 6 pipeline(s).

[Aniruddh25]

Looks good to merge after resolving remaining comments.

#3029 (comment) - this is unresolved.

[anushakolan]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 6 pipeline(s).

[anushakolan]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 6 pipeline(s).