From 52ea1059f8ff1679dce570b9c5fa9e4254e3838c Mon Sep 17 00:00:00 2001 From: Paul Taladay Date: Sat, 13 Dec 2025 16:25:31 -0800 Subject: [PATCH 1/4] Added multiple id read using stored proc. --- ...SqlServerSearchServiceOptimizationTests.cs | 122 ------ .../Features/Search/SqlServerSearchService.cs | 204 +++++++++- ...th.Fhir.Shared.Tests.Integration.projitems | 1 + ...rchServiceResourceReadOptimizationTests.cs | 361 ++++++++++++++++++ 4 files changed, 565 insertions(+), 123 deletions(-) delete mode 100644 src/Microsoft.Health.Fhir.SqlServer.UnitTests/Features/Search/SqlServerSearchServiceOptimizationTests.cs create mode 100644 test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/SqlServerSearchServiceResourceReadOptimizationTests.cs diff --git a/src/Microsoft.Health.Fhir.SqlServer.UnitTests/Features/Search/SqlServerSearchServiceOptimizationTests.cs b/src/Microsoft.Health.Fhir.SqlServer.UnitTests/Features/Search/SqlServerSearchServiceOptimizationTests.cs deleted file mode 100644 index fc9da41ae2..0000000000 --- a/src/Microsoft.Health.Fhir.SqlServer.UnitTests/Features/Search/SqlServerSearchServiceOptimizationTests.cs +++ /dev/null @@ -1,122 +0,0 @@ -// ------------------------------------------------------------------------------------------------- -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License (MIT). See LICENSE in the repo root for license information. -// ------------------------------------------------------------------------------------------------- - -using System.Threading; -using System.Threading.Tasks; -using Microsoft.Health.Fhir.Core.Features.Search; -using Microsoft.Health.Fhir.Core.Models; -using Microsoft.Health.Fhir.Tests.Common; -using Microsoft.Health.Test.Utilities; -using Xunit; - -namespace Microsoft.Health.Fhir.SqlServer.UnitTests.Features.Search -{ - /// - /// Unit tests for SqlServerSearchService optimization logic. - /// These tests verify the early return conditions for the optimization method. - /// - [Trait(Traits.OwningTeam, OwningTeam.Fhir)] - [Trait(Traits.Category, Categories.DataSourceValidation)] - public class SqlServerSearchServiceOptimizationTests - { - [Fact] - public void SearchOptions_WithCountOnly_DisqualifiesOptimization() - { - // Arrange - var searchOptions = new SearchOptions - { - ResourceVersionTypes = ResourceVersionType.Latest, - CountOnly = true, - }; - - // Act & Assert - // When CountOnly is true, the optimization should be skipped - Assert.True(searchOptions.CountOnly); - Assert.Equal(ResourceVersionType.Latest, searchOptions.ResourceVersionTypes); - } - - [Fact] - public void SearchOptions_WithHistoryVersionType_QualifiesForOptimization() - { - // Arrange - var searchOptions = new SearchOptions - { - ResourceVersionTypes = ResourceVersionType.History, - CountOnly = false, - }; - - // Act & Assert - // History version type should now qualify for optimization (for versioned reads) - Assert.False(searchOptions.CountOnly); - Assert.True(searchOptions.ResourceVersionTypes.HasFlag(ResourceVersionType.History)); - } - - [Fact] - public void SearchOptions_WithSoftDeletedVersionType_DisqualifiesOptimization() - { - // Arrange - var searchOptions = new SearchOptions - { - ResourceVersionTypes = ResourceVersionType.SoftDeleted, - CountOnly = false, - }; - - // Act & Assert - // SoftDeleted version type alone should disqualify optimization - Assert.False(searchOptions.CountOnly); - Assert.Equal(ResourceVersionType.SoftDeleted, searchOptions.ResourceVersionTypes); - Assert.False(searchOptions.ResourceVersionTypes.HasFlag(ResourceVersionType.History)); - Assert.NotEqual(ResourceVersionType.Latest, searchOptions.ResourceVersionTypes); - } - - [Fact] - public void SearchOptions_WithOptimalConditions_QualifiesForOptimization() - { - // Arrange - var searchOptions = new SearchOptions - { - ResourceVersionTypes = ResourceVersionType.Latest, - CountOnly = false, - }; - - // Act & Assert - // When all conditions are met, the optimization should proceed - Assert.False(searchOptions.CountOnly); - Assert.Equal(ResourceVersionType.Latest, searchOptions.ResourceVersionTypes); - } - - [Fact] - public void ResourceVersionType_Latest_IsExpectedValue() - { - // Act & Assert - // Verify that ResourceVersionType.Latest has the expected flag value - Assert.True((ResourceVersionType.Latest & ResourceVersionType.Latest) == ResourceVersionType.Latest); - } - - [Fact] - public void ResourceVersionType_History_IsDifferentFromLatest() - { - // Act & Assert - // Verify that History and Latest are different version types - Assert.NotEqual(ResourceVersionType.Latest, ResourceVersionType.History); - } - - [Fact] - public void SearchOptions_WithCombinedVersionTypes_QualifiesForOptimization() - { - // Arrange - Combined Latest and History should qualify for optimization - var searchOptions = new SearchOptions - { - ResourceVersionTypes = ResourceVersionType.Latest | ResourceVersionType.History, - CountOnly = false, - }; - - // Act & Assert - Assert.False(searchOptions.CountOnly); - Assert.True(searchOptions.ResourceVersionTypes.HasFlag(ResourceVersionType.History)); - Assert.True(searchOptions.ResourceVersionTypes.HasFlag(ResourceVersionType.Latest)); - } - } -} diff --git a/src/Microsoft.Health.Fhir.SqlServer/Features/Search/SqlServerSearchService.cs b/src/Microsoft.Health.Fhir.SqlServer/Features/Search/SqlServerSearchService.cs index 9db134e76a..0a79cb5628 100644 --- a/src/Microsoft.Health.Fhir.SqlServer/Features/Search/SqlServerSearchService.cs +++ b/src/Microsoft.Health.Fhir.SqlServer/Features/Search/SqlServerSearchService.cs @@ -499,6 +499,13 @@ await _sqlRetryService.ExecuteSql( searchResult = simpleReadResult; return; } + else if (await TryHandleMultipleResourceReadsAsync(expression, clonedSearchOptions, connection, sqlCommand, cancellationToken) is SearchResult multipleReadResult) + { + // Multiple resource IDs - handled using GetResources stored procedure + _logger.LogInformation("Multiple resource read using GetResources stored procedure."); + searchResult = multipleReadResult; + return; + } else { var stringBuilder = new IndentedStringBuilder(new StringBuilder()); @@ -929,6 +936,201 @@ private static bool TryExtractSimpleStringValue(Expression expression, out strin return false; } + /// + /// Attempts to handle multiple resource read requests using the GetResources stored procedure. + /// This provides better performance than generated SQL for multiple resource lookups. + /// + /// The SQL root expression to analyze + /// Search options for the request + /// The SQL connection + /// The SQL command to populate and execute + /// Cancellation token + /// SearchResult if this was a multiple resource read, null otherwise + private async Task TryHandleMultipleResourceReadsAsync(SqlRootExpression expression, SqlSearchOptions searchOptions, SqlConnection connection, SqlCommand command, CancellationToken cancellationToken) + { + // Only optimize for non-count queries without sorting or includes operations + if (searchOptions.CountOnly || + searchOptions.IncludeCount != 0 || + searchOptions.Sort?.Count > 0 || + searchOptions.IsIncludesOperation) + { + return null; + } + + // Only optimize for Latest searches (no history) + if (searchOptions.ResourceVersionTypes != ResourceVersionType.Latest) + { + return null; + } + + // Ensure we have exactly two resource table expressions (one for resource type, one for resource ID) + if (expression.ResourceTableExpressions.Count != 2 || + expression.SearchParamTableExpressions.Count > 0) + { + return null; + } + + // Extract resource type and IDs from the expressions + if (!TryExtractResourceTypeAndIds(expression, out string resourceType, out List resourceIds)) + { + return null; + } + + // Must have multiple IDs for this optimization + if (resourceIds == null || resourceIds.Count == 0) + { + return null; + } + + // Get resource type ID + if (!_model.TryGetResourceTypeId(resourceType, out short resourceTypeId)) + { + return null; + } + + try + { + // Create a DataTable for the table-valued parameter + var resourceKeysTable = new DataTable(); + resourceKeysTable.Columns.Add("ResourceTypeId", typeof(short)); + resourceKeysTable.Columns.Add("ResourceId", typeof(string)); + resourceKeysTable.Columns.Add("Version", typeof(int)); + + // Populate the table with resource keys + foreach (var resourceId in resourceIds) + { + resourceKeysTable.Rows.Add(resourceTypeId, resourceId, DBNull.Value); + } + + // Populate command to use GetResources stored procedure + command.CommandType = CommandType.StoredProcedure; + command.CommandText = "dbo.GetResources"; + command.Connection = connection; + + var parameter = command.Parameters.AddWithValue("@ResourceKeys", resourceKeysTable); + parameter.SqlDbType = SqlDbType.Structured; + parameter.TypeName = "dbo.ResourceKeyList"; + + LogSqlCommand(command); + + // Execute and read results + var results = new List(); + + using (var reader = await command.ExecuteReaderAsync(CommandBehavior.SequentialAccess, cancellationToken)) + { + while (await reader.ReadAsync(cancellationToken)) + { + // GetResources returns: ResourceTypeId, ResourceId, ResourceSurrogateId, Version, IsDeleted, IsHistory, RawResource, IsRawResourceMetaSet, SearchParamHash + short resTypeId = reader.GetInt16(0); // ResourceTypeId + string resId = reader.GetString(1); // ResourceId + long resourceSurrogateId = reader.GetInt64(2); // ResourceSurrogateId + int version = reader.GetInt32(3); // Version + bool isDeleted = reader.GetBoolean(4); // IsDeleted + bool isHistory = reader.GetBoolean(5); // IsHistory + byte[] rawResourceBytes = reader.GetSqlBytes(6).Value; // RawResource + bool isRawResourceMetaSet = reader.GetBoolean(7); // IsRawResourceMetaSet + string searchParameterHash = await reader.IsDBNullAsync(8, cancellationToken) ? null : reader.GetString(8); // SearchParamHash + + // Skip deleted or history records + if (isDeleted || isHistory) + { + continue; + } + + Lazy rawResource = new Lazy(() => + { + using var rawResourceStream = new MemoryStream(rawResourceBytes); + var decompressedResource = _compressedRawResourceConverter.ReadCompressedRawResource(rawResourceStream); + + if (string.IsNullOrEmpty(decompressedResource)) + { + decompressedResource = MissingResourceFactory.CreateJson(resId, _model.GetResourceTypeName(resTypeId), "warning", "incomplete"); + _requestContextAccessor.SetMissingResourceCode(System.Net.HttpStatusCode.PartialContent); + } + + return decompressedResource; + }); + + var searchResultEntry = new SearchResultEntry( + new ResourceWrapper( + resId, + version.ToString(CultureInfo.InvariantCulture), + _model.GetResourceTypeName(resTypeId), + searchOptions.OnlyIds ? null : new RawResource(rawResource, FhirResourceFormat.Json, isMetaSet: isRawResourceMetaSet), + new ResourceRequest("GET"), + resourceSurrogateId.ToLastUpdated(), + isDeleted, + null, + null, + null, + searchParameterHash, + resourceSurrogateId), + SearchEntryMode.Match); + + results.Add(searchResultEntry); + } + } + + return new SearchResult(results, null, null, searchOptions.UnsupportedSearchParams); + } + catch + { + // If anything goes wrong, fall back to SQL generation + return null; + } + } + + /// + /// Attempts to extract resource type and multiple IDs from an expression with IN clause. + /// + /// The SQL root expression to analyze + /// The extracted resource type + /// The extracted list of resource IDs + /// True if extraction was successful, false otherwise + private static bool TryExtractResourceTypeAndIds(SqlRootExpression expression, out string resourceType, out List resourceIds) + { + resourceType = null; + resourceIds = null; + + // Find resource type and ID expressions in ResourceTableExpressions + SearchParameterExpression resourceTypeExpression = null; + SearchParameterExpression resourceIdExpression = null; + + foreach (var searchParamExpression in expression.ResourceTableExpressions.OfType()) + { + if (searchParamExpression.Parameter.Name == SearchParameterNames.ResourceType) + { + resourceTypeExpression = searchParamExpression; + } + else if (searchParamExpression.Parameter.Name == SearchParameterNames.Id) + { + resourceIdExpression = searchParamExpression; + } + } + + if (resourceTypeExpression == null || resourceIdExpression == null) + { + return false; + } + + // Extract resource type + if (!TryExtractSimpleStringValue(resourceTypeExpression.Expression, out resourceType)) + { + return false; + } + + // Check if the ID expression is an IN expression with multiple values + if (resourceIdExpression.Expression is InExpression inExpression) + { + // Extract all IDs from the IN expression + resourceIds = new List(inExpression.Values); + return resourceIds.Count > 0; + } + + // Not a multi-ID expression + return false; + } + /// /// Searches for resources by their type and surrogate id and optionally a searchParamHash and will return resources /// @@ -2108,7 +2310,7 @@ private void ProcessPredicateForStats( break; default: - // Non-search-parameter leaf (e.g. compartment) – ignore for stats + // Non-search-parameter leaf (e.g. compartment) � ignore for stats break; } } diff --git a/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Microsoft.Health.Fhir.Shared.Tests.Integration.projitems b/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Microsoft.Health.Fhir.Shared.Tests.Integration.projitems index 85b445bbf2..497901c15b 100644 --- a/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Microsoft.Health.Fhir.Shared.Tests.Integration.projitems +++ b/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Microsoft.Health.Fhir.Shared.Tests.Integration.projitems @@ -31,6 +31,7 @@ + diff --git a/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/SqlServerSearchServiceResourceReadOptimizationTests.cs b/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/SqlServerSearchServiceResourceReadOptimizationTests.cs new file mode 100644 index 0000000000..1ff0fe34df --- /dev/null +++ b/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/SqlServerSearchServiceResourceReadOptimizationTests.cs @@ -0,0 +1,361 @@ +// ------------------------------------------------------------------------------------------------- +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License (MIT). See LICENSE in the repo root for license information. +// ------------------------------------------------------------------------------------------------- + +using System; +using System.Collections.Generic; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using Hl7.Fhir.Model; +using Microsoft.Health.Fhir.Core.Extensions; +using Microsoft.Health.Fhir.Core.Features.Search; +using Microsoft.Health.Fhir.Core.Models; +using Microsoft.Health.Fhir.Tests.Common; +using Microsoft.Health.Fhir.Tests.Common.FixtureParameters; +using Microsoft.Health.Test.Utilities; +using Xunit; +using Xunit.Abstractions; +using Task = System.Threading.Tasks.Task; + +namespace Microsoft.Health.Fhir.Tests.Integration.Persistence +{ + /// + /// Integration tests for SqlServerSearchService optimization for single and multiple resource ID reads. + /// These tests verify that the optimized code paths correctly handle: + /// 1. Single resource reads by type and ID + /// 2. Multiple resource reads by type and multiple IDs + /// 3. Versioned resource reads (_history) + /// 4. Edge cases like deleted resources, missing resources, and mixed scenarios + /// + [FhirStorageTestsFixtureArgumentSets(DataStore.SqlServer)] + [Trait(Traits.OwningTeam, OwningTeam.Fhir)] + [Trait(Traits.Category, Categories.DataSourceValidation)] + public class SqlServerSearchServiceResourceReadOptimizationTests : IClassFixture + { + private readonly FhirStorageTestsFixture _fixture; + private readonly ITestOutputHelper _output; + + public SqlServerSearchServiceResourceReadOptimizationTests(FhirStorageTestsFixture fixture, ITestOutputHelper testOutputHelper) + { + _fixture = fixture; + _output = testOutputHelper; + } + + [Fact] + public async Task GivenASingleResourceId_WhenSearchedByTypeAndId_ThenOptimizedPathReturnsCorrectResource() + { + // Arrange - Create a test patient + var patient = Samples.GetJsonSample("Patient").ToPoco(); + patient.Id = Guid.NewGuid().ToString(); + patient.Name = new List + { + new HumanName { Family = "TestSingle", Given = new[] { "Single" } }, + }; + + var saveResult = await _fixture.Mediator.UpsertResourceAsync(patient.ToResourceElement()); + + // Act - Search by _type and _id + var query = new List> + { + new Tuple("_type", "Patient"), + new Tuple("_id", saveResult.RawResourceElement.Id), + }; + + var searchResult = await _fixture.SearchService.SearchAsync("Patient", query, CancellationToken.None); + + // Assert + Assert.NotNull(searchResult); + Assert.Single(searchResult.Results); + Assert.Equal(saveResult.RawResourceElement.Id, searchResult.Results.First().Resource.ResourceId); + Assert.Equal("Patient", searchResult.Results.First().Resource.ResourceTypeName); + _output.WriteLine($"Successfully retrieved single resource: Patient/{saveResult.RawResourceElement.Id}"); + } + + [Fact] + public async Task GivenMultipleResourceIds_WhenSearchedByTypeAndIds_ThenOptimizedPathReturnsAllResources() + { + // Arrange - Create multiple test patients + var patient1 = Samples.GetJsonSample("Patient").ToPoco(); + patient1.Id = Guid.NewGuid().ToString(); + patient1.Name = new List { new HumanName { Family = "TestMulti1" } }; + + var patient2 = Samples.GetJsonSample("Patient").ToPoco(); + patient2.Id = Guid.NewGuid().ToString(); + patient2.Name = new List { new HumanName { Family = "TestMulti2" } }; + + var patient3 = Samples.GetJsonSample("Patient").ToPoco(); + patient3.Id = Guid.NewGuid().ToString(); + patient3.Name = new List { new HumanName { Family = "TestMulti3" } }; + + var saveResult1 = await _fixture.Mediator.UpsertResourceAsync(patient1.ToResourceElement()); + var saveResult2 = await _fixture.Mediator.UpsertResourceAsync(patient2.ToResourceElement()); + var saveResult3 = await _fixture.Mediator.UpsertResourceAsync(patient3.ToResourceElement()); + + // Act - Search by _type and multiple _id values + var idList = $"{saveResult1.RawResourceElement.Id},{saveResult2.RawResourceElement.Id},{saveResult3.RawResourceElement.Id}"; + var query = new List> + { + new Tuple("_type", "Patient"), + new Tuple("_id", idList), + }; + + var searchResult = await _fixture.SearchService.SearchAsync("Patient", query, CancellationToken.None); + + // Assert + Assert.NotNull(searchResult); + Assert.Equal(3, searchResult.Results.Count()); + + var returnedIds = searchResult.Results.Select(r => r.Resource.ResourceId).ToHashSet(); + Assert.Contains(saveResult1.RawResourceElement.Id, returnedIds); + Assert.Contains(saveResult2.RawResourceElement.Id, returnedIds); + Assert.Contains(saveResult3.RawResourceElement.Id, returnedIds); + + _output.WriteLine($"Successfully retrieved {searchResult.Results.Count()} resources"); + } + + [Fact] + public async Task GivenMultipleResourceIdsWithSomeMissing_WhenSearched_ThenOnlyExistingResourcesReturned() + { + // Arrange - Create two patients and use one non-existent ID + var patient1 = Samples.GetJsonSample("Patient").ToPoco(); + patient1.Id = Guid.NewGuid().ToString(); + patient1.Name = new List { new HumanName { Family = "TestPartial1" } }; + + var patient2 = Samples.GetJsonSample("Patient").ToPoco(); + patient2.Id = Guid.NewGuid().ToString(); + patient2.Name = new List { new HumanName { Family = "TestPartial2" } }; + + var saveResult1 = await _fixture.Mediator.UpsertResourceAsync(patient1.ToResourceElement()); + var saveResult2 = await _fixture.Mediator.UpsertResourceAsync(patient2.ToResourceElement()); + var nonExistentId = Guid.NewGuid().ToString(); + + // Act - Search with mix of existing and non-existent IDs + var idList = $"{saveResult1.RawResourceElement.Id},{nonExistentId},{saveResult2.RawResourceElement.Id}"; + var query = new List> + { + new Tuple("_type", "Patient"), + new Tuple("_id", idList), + }; + + var searchResult = await _fixture.SearchService.SearchAsync("Patient", query, CancellationToken.None); + + // Assert - Only existing resources should be returned + Assert.NotNull(searchResult); + Assert.Equal(2, searchResult.Results.Count()); + + var returnedIds = searchResult.Results.Select(r => r.Resource.ResourceId).ToHashSet(); + Assert.Contains(saveResult1.RawResourceElement.Id, returnedIds); + Assert.Contains(saveResult2.RawResourceElement.Id, returnedIds); + Assert.DoesNotContain(nonExistentId, returnedIds); + + _output.WriteLine($"Successfully filtered out non-existent resource and returned {searchResult.Results.Count()} valid resources"); + } + + [Fact] + public async Task GivenASingleResourceIdWithVersionedRead_WhenSearched_ThenCorrectVersionReturned() + { + // Arrange - Create a patient and update it to create versions + var patient = Samples.GetJsonSample("Patient").ToPoco(); + patient.Id = Guid.NewGuid().ToString(); + patient.Name = new List { new HumanName { Family = "TestVersion", Given = new[] { "Version1" } } }; + + var saveResult = await _fixture.Mediator.UpsertResourceAsync(patient.ToResourceElement()); + + // Update to create version 2 + patient.Name[0].Given = new[] { "Version2" }; + var updateResult = await _fixture.Mediator.UpsertResourceAsync(patient.ToResourceElement(), Core.Features.Persistence.WeakETag.FromVersionId(saveResult.RawResourceElement.VersionId)); + + // Act - Search for specific version using history + var query = new List> + { + new Tuple("_type", "Patient"), + new Tuple("_id", saveResult.RawResourceElement.Id), + }; + + // Search with History version type to test versioned read optimization + var searchResult = await _fixture.SearchService.SearchAsync( + "Patient", + query, + CancellationToken.None, + resourceVersionTypes: ResourceVersionType.Latest | ResourceVersionType.History); + + // Assert + Assert.NotNull(searchResult); + Assert.NotEmpty(searchResult.Results); + Assert.All(searchResult.Results, r => Assert.Equal(saveResult.RawResourceElement.Id, r.Resource.ResourceId)); + + _output.WriteLine($"Successfully retrieved versioned resource: Patient/{saveResult.RawResourceElement.Id}"); + } + + [Fact] + public async Task GivenMultipleResourceIdsWithDeletedResource_WhenSearched_ThenOnlyActiveResourcesReturned() + { + // Arrange - Create patients and delete one + var patient1 = Samples.GetJsonSample("Patient").ToPoco(); + patient1.Id = Guid.NewGuid().ToString(); + patient1.Name = new List { new HumanName { Family = "TestDeleted1" } }; + + var patient2 = Samples.GetJsonSample("Patient").ToPoco(); + patient2.Id = Guid.NewGuid().ToString(); + patient2.Name = new List { new HumanName { Family = "TestDeleted2" } }; + + var patient3 = Samples.GetJsonSample("Patient").ToPoco(); + patient3.Id = Guid.NewGuid().ToString(); + patient3.Name = new List { new HumanName { Family = "TestDeleted3" } }; + + var saveResult1 = await _fixture.Mediator.UpsertResourceAsync(patient1.ToResourceElement()); + var saveResult2 = await _fixture.Mediator.UpsertResourceAsync(patient2.ToResourceElement()); + var saveResult3 = await _fixture.Mediator.UpsertResourceAsync(patient3.ToResourceElement()); + + // Delete patient2 + await _fixture.Mediator.DeleteResourceAsync(new Core.Features.Persistence.ResourceKey("Patient", saveResult2.RawResourceElement.Id), Core.Messages.Delete.DeleteOperation.SoftDelete); + + // Act - Search including the deleted resource ID + var idList = $"{saveResult1.RawResourceElement.Id},{saveResult2.RawResourceElement.Id},{saveResult3.RawResourceElement.Id}"; + var query = new List> + { + new Tuple("_type", "Patient"), + new Tuple("_id", idList), + }; + + var searchResult = await _fixture.SearchService.SearchAsync("Patient", query, CancellationToken.None); + + // Assert - Deleted resource should not be returned (default is Latest only) + Assert.NotNull(searchResult); + Assert.Equal(2, searchResult.Results.Count()); + + var returnedIds = searchResult.Results.Select(r => r.Resource.ResourceId).ToHashSet(); + Assert.Contains(saveResult1.RawResourceElement.Id, returnedIds); + Assert.DoesNotContain(saveResult2.RawResourceElement.Id, returnedIds); // Deleted + Assert.Contains(saveResult3.RawResourceElement.Id, returnedIds); + + _output.WriteLine($"Successfully excluded deleted resource, returned {searchResult.Results.Count()} active resources"); + } + + [Fact] + public async Task GivenSearchWithAdditionalParameters_WhenSearched_ThenOptimizationMayNotApply() + { + // Arrange + var patient = Samples.GetJsonSample("Patient").ToPoco(); + patient.Id = Guid.NewGuid().ToString(); + patient.Name = new List { new HumanName { Family = "TestAdditional" } }; + patient.Gender = AdministrativeGender.Female; + + var saveResult = await _fixture.Mediator.UpsertResourceAsync(patient.ToResourceElement()); + + // Act - Search with additional parameter (should not use optimization) + var query = new List> + { + new Tuple("_type", "Patient"), + new Tuple("_id", saveResult.RawResourceElement.Id), + new Tuple("gender", "female"), // Additional parameter + }; + + var searchResult = await _fixture.SearchService.SearchAsync("Patient", query, CancellationToken.None); + + // Assert + Assert.NotNull(searchResult); + Assert.Single(searchResult.Results); + Assert.Equal(saveResult.RawResourceElement.Id, searchResult.Results.First().Resource.ResourceId); + + _output.WriteLine("Search with additional parameters completed successfully (expected fallback to standard search)"); + } + + [Fact] + public async Task GivenLargeNumberOfResourceIds_WhenSearched_ThenOptimizationHandlesEfficiently() + { + // Arrange - Create 10 patients + var createdIds = new List(); + for (int i = 0; i < 10; i++) + { + var patient = Samples.GetJsonSample("Patient").ToPoco(); + patient.Id = Guid.NewGuid().ToString(); + patient.Name = new List { new HumanName { Family = $"TestLarge{i}" } }; + + var saveResult = await _fixture.Mediator.UpsertResourceAsync(patient.ToResourceElement()); + createdIds.Add(saveResult.RawResourceElement.Id); + } + + // Act - Search for all 10 resources + var idList = string.Join(",", createdIds); + var query = new List> + { + new Tuple("_type", "Patient"), + new Tuple("_id", idList), + }; + + var searchResult = await _fixture.SearchService.SearchAsync("Patient", query, CancellationToken.None); + + // Assert + Assert.NotNull(searchResult); + Assert.Equal(10, searchResult.Results.Count()); + + var returnedIds = searchResult.Results.Select(r => r.Resource.ResourceId).ToHashSet(); + foreach (var id in createdIds) + { + Assert.Contains(id, returnedIds); + } + + _output.WriteLine($"Successfully retrieved all {searchResult.Results.Count()} resources efficiently"); + } + + [Fact] + public async Task GivenSearchWithCountOnly_WhenSearched_ThenOptimizationDoesNotApply() + { + // Arrange + var patient = Samples.GetJsonSample("Patient").ToPoco(); + patient.Id = Guid.NewGuid().ToString(); + patient.Name = new List { new HumanName { Family = "TestCount" } }; + + var saveResult = await _fixture.Mediator.UpsertResourceAsync(patient.ToResourceElement()); + + // Act - Search with count only + var query = new List> + { + new Tuple("_type", "Patient"), + new Tuple("_id", saveResult.RawResourceElement.Id), + new Tuple("_summary", "count"), + }; + + var searchResult = await _fixture.SearchService.SearchAsync("Patient", query, CancellationToken.None); + + // Assert + Assert.NotNull(searchResult); + _output.WriteLine($"Count-only search completed successfully with total: {searchResult.TotalCount}"); + } + + [Fact] + public async Task GivenSearchWithSort_WhenSearched_ThenOptimizationDoesNotApply() + { + // Arrange + var patient1 = Samples.GetJsonSample("Patient").ToPoco(); + patient1.Id = Guid.NewGuid().ToString(); + patient1.Name = new List { new HumanName { Family = "Aardvark" } }; + + var patient2 = Samples.GetJsonSample("Patient").ToPoco(); + patient2.Id = Guid.NewGuid().ToString(); + patient2.Name = new List { new HumanName { Family = "Zebra" } }; + + var saveResult1 = await _fixture.Mediator.UpsertResourceAsync(patient1.ToResourceElement()); + var saveResult2 = await _fixture.Mediator.UpsertResourceAsync(patient2.ToResourceElement()); + + // Act - Search with sort parameter + var idList = $"{saveResult1.RawResourceElement.Id},{saveResult2.RawResourceElement.Id}"; + var query = new List> + { + new Tuple("_type", "Patient"), + new Tuple("_id", idList), + new Tuple("_sort", "family"), + }; + + var searchResult = await _fixture.SearchService.SearchAsync("Patient", query, CancellationToken.None); + + // Assert + Assert.NotNull(searchResult); + Assert.Equal(2, searchResult.Results.Count()); + _output.WriteLine("Search with sort parameter completed successfully (expected fallback to standard search)"); + } + } +} From 4f5a503545463b2269b9f163295fdba7e8f1f8b7 Mon Sep 17 00:00:00 2001 From: Paul Taladay Date: Sun, 14 Dec 2025 14:01:32 -0800 Subject: [PATCH 2/4] Updated tests nased on parsing of resource type. --- ...verSearchServiceResourceReadOptimizationTests.cs | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/SqlServerSearchServiceResourceReadOptimizationTests.cs b/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/SqlServerSearchServiceResourceReadOptimizationTests.cs index 1ff0fe34df..40127afc6f 100644 --- a/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/SqlServerSearchServiceResourceReadOptimizationTests.cs +++ b/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/SqlServerSearchServiceResourceReadOptimizationTests.cs @@ -56,10 +56,9 @@ public async Task GivenASingleResourceId_WhenSearchedByTypeAndId_ThenOptimizedPa var saveResult = await _fixture.Mediator.UpsertResourceAsync(patient.ToResourceElement()); - // Act - Search by _type and _id + // Act - Search by _id only (resource type is already specified in SearchAsync) var query = new List> { - new Tuple("_type", "Patient"), new Tuple("_id", saveResult.RawResourceElement.Id), }; @@ -93,11 +92,10 @@ public async Task GivenMultipleResourceIds_WhenSearchedByTypeAndIds_ThenOptimize var saveResult2 = await _fixture.Mediator.UpsertResourceAsync(patient2.ToResourceElement()); var saveResult3 = await _fixture.Mediator.UpsertResourceAsync(patient3.ToResourceElement()); - // Act - Search by _type and multiple _id values + // Act - Search by multiple _id values var idList = $"{saveResult1.RawResourceElement.Id},{saveResult2.RawResourceElement.Id},{saveResult3.RawResourceElement.Id}"; var query = new List> { - new Tuple("_type", "Patient"), new Tuple("_id", idList), }; @@ -135,7 +133,6 @@ public async Task GivenMultipleResourceIdsWithSomeMissing_WhenSearched_ThenOnlyE var idList = $"{saveResult1.RawResourceElement.Id},{nonExistentId},{saveResult2.RawResourceElement.Id}"; var query = new List> { - new Tuple("_type", "Patient"), new Tuple("_id", idList), }; @@ -170,7 +167,6 @@ public async Task GivenASingleResourceIdWithVersionedRead_WhenSearched_ThenCorre // Act - Search for specific version using history var query = new List> { - new Tuple("_type", "Patient"), new Tuple("_id", saveResult.RawResourceElement.Id), }; @@ -216,7 +212,6 @@ public async Task GivenMultipleResourceIdsWithDeletedResource_WhenSearched_ThenO var idList = $"{saveResult1.RawResourceElement.Id},{saveResult2.RawResourceElement.Id},{saveResult3.RawResourceElement.Id}"; var query = new List> { - new Tuple("_type", "Patient"), new Tuple("_id", idList), }; @@ -248,7 +243,6 @@ public async Task GivenSearchWithAdditionalParameters_WhenSearched_ThenOptimizat // Act - Search with additional parameter (should not use optimization) var query = new List> { - new Tuple("_type", "Patient"), new Tuple("_id", saveResult.RawResourceElement.Id), new Tuple("gender", "female"), // Additional parameter }; @@ -282,7 +276,6 @@ public async Task GivenLargeNumberOfResourceIds_WhenSearched_ThenOptimizationHan var idList = string.Join(",", createdIds); var query = new List> { - new Tuple("_type", "Patient"), new Tuple("_id", idList), }; @@ -314,7 +307,6 @@ public async Task GivenSearchWithCountOnly_WhenSearched_ThenOptimizationDoesNotA // Act - Search with count only var query = new List> { - new Tuple("_type", "Patient"), new Tuple("_id", saveResult.RawResourceElement.Id), new Tuple("_summary", "count"), }; @@ -345,7 +337,6 @@ public async Task GivenSearchWithSort_WhenSearched_ThenOptimizationDoesNotApply( var idList = $"{saveResult1.RawResourceElement.Id},{saveResult2.RawResourceElement.Id}"; var query = new List> { - new Tuple("_type", "Patient"), new Tuple("_id", idList), new Tuple("_sort", "family"), }; From 2e860021a90e28806f416d11aa5e142dd7dae261 Mon Sep 17 00:00:00 2001 From: Paul Taladay Date: Mon, 15 Dec 2025 09:25:08 -0800 Subject: [PATCH 3/4] Removed unneeded tests. --- ...rchServiceResourceReadOptimizationTests.cs | 59 ------------------- 1 file changed, 59 deletions(-) diff --git a/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/SqlServerSearchServiceResourceReadOptimizationTests.cs b/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/SqlServerSearchServiceResourceReadOptimizationTests.cs index 40127afc6f..4c7df0d6cf 100644 --- a/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/SqlServerSearchServiceResourceReadOptimizationTests.cs +++ b/test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/SqlServerSearchServiceResourceReadOptimizationTests.cs @@ -229,34 +229,6 @@ public async Task GivenMultipleResourceIdsWithDeletedResource_WhenSearched_ThenO _output.WriteLine($"Successfully excluded deleted resource, returned {searchResult.Results.Count()} active resources"); } - [Fact] - public async Task GivenSearchWithAdditionalParameters_WhenSearched_ThenOptimizationMayNotApply() - { - // Arrange - var patient = Samples.GetJsonSample("Patient").ToPoco(); - patient.Id = Guid.NewGuid().ToString(); - patient.Name = new List { new HumanName { Family = "TestAdditional" } }; - patient.Gender = AdministrativeGender.Female; - - var saveResult = await _fixture.Mediator.UpsertResourceAsync(patient.ToResourceElement()); - - // Act - Search with additional parameter (should not use optimization) - var query = new List> - { - new Tuple("_id", saveResult.RawResourceElement.Id), - new Tuple("gender", "female"), // Additional parameter - }; - - var searchResult = await _fixture.SearchService.SearchAsync("Patient", query, CancellationToken.None); - - // Assert - Assert.NotNull(searchResult); - Assert.Single(searchResult.Results); - Assert.Equal(saveResult.RawResourceElement.Id, searchResult.Results.First().Resource.ResourceId); - - _output.WriteLine("Search with additional parameters completed successfully (expected fallback to standard search)"); - } - [Fact] public async Task GivenLargeNumberOfResourceIds_WhenSearched_ThenOptimizationHandlesEfficiently() { @@ -317,36 +289,5 @@ public async Task GivenSearchWithCountOnly_WhenSearched_ThenOptimizationDoesNotA Assert.NotNull(searchResult); _output.WriteLine($"Count-only search completed successfully with total: {searchResult.TotalCount}"); } - - [Fact] - public async Task GivenSearchWithSort_WhenSearched_ThenOptimizationDoesNotApply() - { - // Arrange - var patient1 = Samples.GetJsonSample("Patient").ToPoco(); - patient1.Id = Guid.NewGuid().ToString(); - patient1.Name = new List { new HumanName { Family = "Aardvark" } }; - - var patient2 = Samples.GetJsonSample("Patient").ToPoco(); - patient2.Id = Guid.NewGuid().ToString(); - patient2.Name = new List { new HumanName { Family = "Zebra" } }; - - var saveResult1 = await _fixture.Mediator.UpsertResourceAsync(patient1.ToResourceElement()); - var saveResult2 = await _fixture.Mediator.UpsertResourceAsync(patient2.ToResourceElement()); - - // Act - Search with sort parameter - var idList = $"{saveResult1.RawResourceElement.Id},{saveResult2.RawResourceElement.Id}"; - var query = new List> - { - new Tuple("_id", idList), - new Tuple("_sort", "family"), - }; - - var searchResult = await _fixture.SearchService.SearchAsync("Patient", query, CancellationToken.None); - - // Assert - Assert.NotNull(searchResult); - Assert.Equal(2, searchResult.Results.Count()); - _output.WriteLine("Search with sort parameter completed successfully (expected fallback to standard search)"); - } } } From 0fe596dbffe9310189a58bbcb34d2c09a2338654 Mon Sep 17 00:00:00 2001 From: Paul Taladay Date: Mon, 5 Jan 2026 10:49:07 -0800 Subject: [PATCH 4/4] Updated logic for parsing expression and grabbing the id list when multiple ids are sent for a simple read operation. --- .../Features/Search/SqlServerSearchService.cs | 25 ++++++++++++++----- 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/src/Microsoft.Health.Fhir.SqlServer/Features/Search/SqlServerSearchService.cs b/src/Microsoft.Health.Fhir.SqlServer/Features/Search/SqlServerSearchService.cs index 0a79cb5628..189e98f5dc 100644 --- a/src/Microsoft.Health.Fhir.SqlServer/Features/Search/SqlServerSearchService.cs +++ b/src/Microsoft.Health.Fhir.SqlServer/Features/Search/SqlServerSearchService.cs @@ -950,8 +950,7 @@ private async Task TryHandleMultipleResourceReadsAsync(SqlRootExpr { // Only optimize for non-count queries without sorting or includes operations if (searchOptions.CountOnly || - searchOptions.IncludeCount != 0 || - searchOptions.Sort?.Count > 0 || + searchOptions.IncludeTotal != TotalType.None || searchOptions.IsIncludesOperation) { return null; @@ -1119,11 +1118,25 @@ private static bool TryExtractResourceTypeAndIds(SqlRootExpression expression, o return false; } - // Check if the ID expression is an IN expression with multiple values - if (resourceIdExpression.Expression is InExpression inExpression) + // Check if the ID expression is an OR expression with multiple values + if (resourceIdExpression.Expression is MultiaryExpression multiaryExpression) { - // Extract all IDs from the IN expression - resourceIds = new List(inExpression.Values); + // Extract all IDs from the OR expression + resourceIds = new List(); + foreach (var subExpression in multiaryExpression.Expressions) + { + if (TryExtractSimpleStringValue(subExpression, out string idValue)) + { + resourceIds.Add(idValue); + } + else + { + // If any sub-expression is not a simple string, fail the extraction + resourceIds.Clear(); + return false; + } + } + return resourceIds.Count > 0; }