Filter stat for ReferenceSearchParam table#5531
Open
apurvabhaleMS wants to merge 14 commits into
Open
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5531 +/- ##
=======================================
Coverage ? 77.69%
=======================================
Files ? 985
Lines ? 36192
Branches ? 5498
=======================================
Hits ? 28120
Misses ? 6717
Partials ? 1355 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds filtered-statistics support for additional SQL search parameter tables (ReferenceSearchParam and UriSearchParam) and verifies the behavior with new unit/integration tests.
Changes:
- Extend
GetKeyColumnsto includedbo.ReferenceSearchParamanddbo.UriSearchParamkey columns. - Add unit tests covering
GetKeyColumnsacross single/two-column tables and unknown/null tables. - Add integration tests asserting filtered stats are created (and skipped for
:missing/ NotExists expressions).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| test/Microsoft.Health.Fhir.Shared.Tests.Integration/Persistence/SqlServerCreateStatsTests.cs | Adds integration coverage for stats creation for Reference/Uri params and for NotExists behavior. |
| src/Microsoft.Health.Fhir.SqlServer/Features/Search/SqlServerSearchService.cs | Expands key-column mapping for filtered stats to Reference/Uri tables and shares it with the stats pipeline. |
| src/Microsoft.Health.Fhir.SqlServer.UnitTests/Features/Search/SqlServerSearchServiceTests.cs | Adds unit tests validating the expanded GetKeyColumns behavior. |
apurvabhaleMS
added
Area-SQL
Contributor
|
I think stats on ref search param should be ST_ReferenceResourceId_WHERE_ResourceTypeId_40_SearchParamId_1219_ReferenceResourceTypeId_XXX |
…i-table # Conflicts: # src/Microsoft.Health.Fhir.SqlServer.UnitTests/Features/Search/SqlServerSearchServiceTests.cs
…r-stat-for-reference-uri-table
feordin
previously approved these changes
Jun 26, 2026
feordin
changed the title
…table - bump stats sprocs to V115
Comment on lines
+2010
to
2013
| internal static ICollection<(string TableName, string ColumnName, short ResourceTypeId, short SearchParamId, short? ReferenceResourceTypeId)> GetStatsFromCache() | ||
| { | ||
| return _resourceSearchParamStats.GetStatsFromCache(); | ||
| } |
Comment on lines
2031
to
+2036
| var split = stats.Split("_"); | ||
| var column = split[1]; | ||
| var resorceTypeId = short.Parse(split[4]); | ||
| var searchParamId = short.Parse(split[6]); | ||
| return ("dbo." + table, column, resorceTypeId, searchParamId); | ||
| short? referenceResourceTypeId = split.Length > 8 ? short.Parse(split[8]) : null; | ||
| return ("dbo." + table, column, resorceTypeId, searchParamId, referenceResourceTypeId); |
| AND (T.name LIKE @Table OR @Table IS NULL) | ||
| AND (S.name LIKE '%ResourceTypeId[_]'+convert(varchar,@ResourceTypeId)+'[_]%' OR @ResourceTypeId IS NULL) | ||
| AND (S.name LIKE '%SearchParamId[_]'+convert(varchar,@SearchParamId) OR @SearchParamId IS NULL) | ||
| AND (S.name LIKE '%SearchParamId[_]'+convert(varchar,@SearchParamId)+'%' OR @SearchParamId IS NULL) |
| AND S.name LIKE 'ST[_]%' | ||
| AND (T.name LIKE @Table OR @Table IS NULL) | ||
| AND (S.name LIKE '%ResourceTypeId[_]'+convert(varchar,@ResourceTypeId)+'[_]%' OR @ResourceTypeId IS NULL) | ||
| AND (S.name LIKE '%SearchParamId[_]'+convert(varchar,@SearchParamId)+'%' OR @SearchParamId IS NULL) |
Comment on lines
+135
to
+137
| Assert.True( | ||
| cacheAfter.Count(MatchesStat) > cacheBefore.Count(MatchesStat), | ||
| "Expected the search to add a filtered statistics entry in the cache for ReferenceResourceId with ReferenceResourceTypeId filter."); |
Comment on lines
+140
to
+142
| Assert.True( | ||
| databaseAfter.Count(MatchesStat) > databaseBefore.Count(MatchesStat), | ||
| "Expected the search to add a filtered statistics entry in the database for ReferenceResourceId with ReferenceResourceTypeId filter."); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds 3-column filtered statistics on dbo.ReferenceSearchParam for the ReferenceResourceId column, filtered by (ResourceTypeId, SearchParamId, ReferenceResourceTypeId). This gives SQL Server per-target-type histograms, enabling better cardinality estimates for reference joins (e.g., DiagnosticReport?subject=Patient/123).
What changed
Schema (V115)
CreateResourceSearchParamStats: new optional @ReferenceResourceTypeId smallint = NULL parameter. When provided, creates a stat named ST_ReferenceResourceId_WHERE_ResourceTypeId_X_SearchParamId_Y_ReferenceResourceTypeId_Z with a 3-column WHERE filter.
GetResourceSearchParamStats: new optional @ReferenceResourceTypeId parameter for lookup. Uses LIKE with no trailing % on ReferenceResourceTypeId to prevent false positives (e.g., _2 matching _20).
Related issues
Addresses AB189018.
Testing
Describe how this change was tested.
FHIR Team Checklist
Semver Change (docs)
Patch|Skip|Feature|Breaking (reason)