Fix infinite loop with multiple includes by LTA-Thinking · Pull Request #5297 · microsoft/fhir-server

LTA-Thinking · 2025-12-30T19:57:31Z

Description

When using multiple _include/_revinclude parameters in a search with _sort it is possible for the includes continuation tokens to get into an infinite loop if there are more than two pages of results. This happens because the included results are being sorted by surrogate id, when our logic is assuming it is being sorted by resource type id and then surrogate id.

Related issues

Addresses Bug 180045

Testing

New and existing E2E tests.

FHIR Team Checklist

Update the title of the PR to be succinct and less than 65 characters
Add a milestone to the PR for the sprint that it is merged (i.e. add S47)
Tag the PR with the type of update: Bug, Build, Dependencies, Enhancement, New-Feature or Documentation
Tag the PR with Open source, Azure API for FHIR (CosmosDB or common code) or Azure Healthcare APIs (SQL or common code) to specify where this change is intended to be released.
Tag the PR with Schema Version backward compatible or Schema Version backward incompatible or Schema Version unchanged if this adds or updates Sql script which is/is not backward compatible with the code.
When changing or adding behavior, if your code modifies the system design or changes design assumptions, please create and include an ADR.
CI is green before merge
Review squash-merge requirements

Semver Change (docs)

Patch|Skip|Feature|Breaking (reason)

...lth.Fhir.SqlServer/Features/Search/Expressions/Visitors/QueryGenerators/SqlQueryGenerator.cs

LTA-Thinking · 2025-12-31T17:23:49Z

/azp run

azure-pipelines · 2025-12-31T17:24:06Z

Azure Pipelines successfully started running 1 pipeline(s).

test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Search/SortTests.cs

        {
            var tag = Guid.NewGuid().ToString();
-            var resources = await CreatePatientsWithLinkedObservations(tag);
+            var resources = await CreatePatientsWithLinkedObservationAndEncounter(tag);


In general, to fix a useless assignment where the right-hand side has important side effects, you keep the call but remove the unused local variable. If the result is truly not needed, you should either discard it with _ or simply await the method without assigning the result. This preserves behavior while eliminating dead code and any confusion about the variable’s purpose.

For this specific case in test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Search/SortTests.cs, in the method GivenPatientsWithIncludedResourcesGreaterThanOnePage_WhenSearchedWithSortAndInclude_ThenSearchResultsContainIncludedResources, change line 1156 from assigning to resources to just awaiting the method call. You do not need any new imports or additional methods. The minimal change is:

Replace var resources = await CreatePatientsWithLinkedObservationAndEncounter(tag); with await CreatePatientsWithLinkedObservationAndEncounter(tag);.

This keeps the creation of patients, observations, and encounters (the side effect) while removing the unused local variable, without affecting existing test behavior.

test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Search/SortTests.cs

        {
            var tag = Guid.NewGuid().ToString();
-            var resources = await CreatePatientsWithLinkedObservations(tag);
+            var resources = await CreatePatientsWithLinkedObservationAndEncounter(tag);


To fix this, we should keep calling CreatePatientsWithLinkedObservationAndEncounter(tag) (to preserve its side effects of creating linked resources) but remove the unnecessary assignment to the unused local variable resources. In general, when a method is called purely for its side effects and the return value is never used, we should either not capture the return value or use the discard _ if we want to emphasize that it is intentionally unused.

Specifically in test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Search/SortTests.cs, inside the method GivenPatientsWithIncludedResources_WhenSearchedWithSortAndIncludeOnDataThatOnlyPartiallyContainsTheSortField_ThenTheRightIncludedResourcesAreReturned, replace the line:

var resources = await CreatePatientsWithLinkedObservationAndEncounter(tag);

with:

await CreatePatientsWithLinkedObservationAndEncounter(tag);

No new imports or helper methods are required. This preserves all existing functionality while eliminating the useless assignment that CodeQL flagged.

test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Search/SortTests.cs

        {
            var tag = Guid.NewGuid().ToString();
-            var resources = await CreatePatientsWithLinkedObservations(tag);
+            var resources = await CreatePatientsWithLinkedObservationAndEncounter(tag);


To fix this, we should keep the call to CreatePatientsWithLinkedObservationAndEncounter(tag) (for its side effects of creating test data) but remove the unused local variable resources. In C#, the simplest way is to either call the method without capturing its return value, or assign it to the discard _. Both approaches eliminate the useless assignment while preserving behavior.

The most direct change, with minimal impact, is in test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/Search/SortTests.cs inside GivenPatientsWithIncludedResources_WhenSearchedWithSortAndInclude_ThenTheSecondPhaseContinuationTokenIsReturned. Replace:

var resources = await CreatePatientsWithLinkedObservationAndEncounter(tag);

with:

await CreatePatientsWithLinkedObservationAndEncounter(tag);

No new imports, methods, or additional definitions are required; we are only changing how the existing method call is made so that the unused variable is removed.

LTA-Thinking · 2026-01-09T22:37:25Z

/azp run

azure-pipelines · 2026-01-09T22:37:32Z

Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command.

LTA-Thinking · 2026-01-09T22:38:22Z

/azp run

azure-pipelines · 2026-01-09T22:38:38Z

Azure Pipelines successfully started running 1 pipeline(s).

Fix infinite loop with multiple includes

45bca79

LTA-Thinking requested a review from a team as a code owner December 30, 2025 19:57

LTA-Thinking added this to the FY26\Q2\2Wk\2Wk13 milestone Dec 30, 2025

LTA-Thinking added Bug Bug bug bug. Azure Healthcare APIs Label denotes that the issue or PR is relevant to the FHIR service in the Azure Healthcare APIs labels Dec 30, 2025

Fix R5

248bcba

SergeyGaluzo reviewed Dec 30, 2025

View reviewed changes

...lth.Fhir.SqlServer/Features/Search/Expressions/Visitors/QueryGenerators/SqlQueryGenerator.cs Show resolved Hide resolved

SergeyGaluzo previously approved these changes Dec 30, 2025

View reviewed changes

LTA-Thinking added the No-PaaS-breaking-change label Dec 31, 2025

Skip tests

02ac7d3

LTA-Thinking dismissed SergeyGaluzo’s stale review via 02ac7d3 January 5, 2026 16:23

RB Johnson (He/Him) added 2 commits January 5, 2026 11:07

Reenable some tests

725c79d

Try with encounters

226638e

github-advanced-security bot found potential problems Jan 6, 2026

View reviewed changes

Reenable last test

84ee0cb

Address comments

26b1f58

rbans96 previously approved these changes Jan 12, 2026

View reviewed changes

LTA-Thinking enabled auto-merge (squash) January 12, 2026 23:05

Fix test

67c3a4e

LTA-Thinking dismissed rbans96’s stale review via 67c3a4e January 13, 2026 15:26

jestradaMS approved these changes Jan 13, 2026

View reviewed changes

LTA-Thinking merged commit 6dd540c into main Jan 13, 2026
60 of 61 checks passed

LTA-Thinking deleted the personal/rojo/fix-sort-include-2 branch January 13, 2026 19:48

jestradaMS mentioned this pull request Jan 30, 2026

Fix SearchParameter pagination bug - move TOP to outer query #5362

Merged

@@ -1204,7 +1204,7 @@
                     public async Task GivenPatientsWithIncludedResources_WhenSearchedWithSortAndInclude_ThenTheSecondPhaseContinuationTokenIsReturned(int includesCount, string sort)
                     {
                         var tag = Guid.NewGuid().ToString();
-                        var resources = await CreatePatientsWithLinkedObservationAndEncounter(tag);
+                        await CreatePatientsWithLinkedObservationAndEncounter(tag);
                         var response = await Client.SearchAsync($"Patient?_tag={tag}&_sort={sort}&_revinclude=Observation:subject&_revinclude=Encounter:subject&_count=12&_includesCount={includesCount}");
                         var relatedLink = response.Resource.Link.FirstOrDefault(link => link.Relation.Equals("related", StringComparison.OrdinalIgnoreCase));

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix infinite loop with multiple includes#5297

Fix infinite loop with multiple includes#5297
LTA-Thinking merged 8 commits intomainfrom
personal/rojo/fix-sort-include-2

LTA-Thinking commented Dec 30, 2025 •

edited

Loading

Uh oh!

Uh oh!

LTA-Thinking commented Dec 31, 2025

Uh oh!

azure-pipelines bot commented Dec 31, 2025

Uh oh!

Check warning

Copilot Autofix

Uh oh!

Check warning

Copilot Autofix

Check warning

Copilot Autofix

LTA-Thinking commented Jan 9, 2026

Uh oh!

azure-pipelines bot commented Jan 9, 2026

Uh oh!

LTA-Thinking commented Jan 9, 2026

Uh oh!

azure-pipelines bot commented Jan 9, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

LTA-Thinking commented Dec 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related issues

Testing

FHIR Team Checklist

Semver Change (docs)

Uh oh!

Uh oh!

LTA-Thinking commented Dec 31, 2025

Uh oh!

azure-pipelines bot commented Dec 31, 2025

Uh oh!

Check warning

Copilot Autofix

Uh oh!

Check warning

Copilot Autofix

Check warning

Copilot Autofix

LTA-Thinking commented Jan 9, 2026

Uh oh!

azure-pipelines bot commented Jan 9, 2026

Uh oh!

LTA-Thinking commented Jan 9, 2026

Uh oh!

azure-pipelines bot commented Jan 9, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

LTA-Thinking commented Dec 30, 2025 •

edited

Loading