Skip to content

Update our ResourceSearchParamStats to handle SQL Not Exists case #5545

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
feordin wants to merge 2 commits into
base: main
Choose a base branch
from
+229 −3
Open

Update our ResourceSearchParamStats to handle SQL Not Exists case #5545

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ec6793f
create stats for non-exists or missing queries
feordin Apr 27, 2026
5f3ff73
Merge remote-tracking branch 'origin/main' into users/jaerwin/missing…
feordin May 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 103 additions & 0 deletions src/Microsoft.Health.Fhir.SqlServer.UnitTests/Features/Search/SqlServerSearchServiceTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,5 +299,108 @@ public void ReuseQueryPlansParameterId_HasExpectedValue()
// Assert
Assert.Equal("Search.ReuseQueryPlans.IsEnabled", SqlServerSearchService.ReuseQueryPlansParameterId);
}

[Fact]
public void CollectNotExistsLeaves_WithResourceSurrogateId_DetectsSurrogateIdAndMissingParam()
{
// Arrange: NotExists predicate with MissingSearchParameterExpression + ResourceSurrogateId constraint
var genderParam = new SearchParameterInfo("gender", "gender");
var missingExpr = Expression.MissingSearchParameter(genderParam, false);
var surrogateIdExpr = Expression.SearchParameter(
SqlSearchParameters.ResourceSurrogateIdParameter,
Expression.GreaterThanOrEqual(SqlFieldName.ResourceSurrogateId, null, 100L));

var predicate = Expression.And(missingExpr, surrogateIdExpr);

var missingParams = new List<MissingSearchParameterExpression>();
var resourceTypeIds = new HashSet<short>();
bool foundSurrogateId = false;

// Act
SqlServerSearchService.ResourceSearchParamStats.CollectNotExistsLeaves(
predicate, missingParams, resourceTypeIds, null, ref foundSurrogateId);

// Assert
Assert.Single(missingParams);
Assert.Equal("gender", missingParams[0].Parameter.Name);
Assert.True(foundSurrogateId, "Should detect ResourceSurrogateId constraint");
}

[Fact]
public void CollectNotExistsLeaves_WithoutResourceSurrogateId_DoesNotSetSurrogateIdFlag()
{
// Arrange: NotExists predicate with only MissingSearchParameterExpression
var genderParam = new SearchParameterInfo("gender", "gender");
var missingExpr = Expression.MissingSearchParameter(genderParam, false);

var missingParams = new List<MissingSearchParameterExpression>();
var resourceTypeIds = new HashSet<short>();
bool foundSurrogateId = false;

// Act
SqlServerSearchService.ResourceSearchParamStats.CollectNotExistsLeaves(
missingExpr, missingParams, resourceTypeIds, null, ref foundSurrogateId);

// Assert
Assert.Single(missingParams);
Assert.Equal("gender", missingParams[0].Parameter.Name);
Assert.False(foundSurrogateId, "Should not detect ResourceSurrogateId when absent");
}

[Fact]
public void CollectNotExistsLeaves_AmbiguousSameTablePredicates_CollectsMultipleMissingParams()
{
// Arrange: Two MissingSearchParameterExpression nodes in the same predicate (ambiguous)
var genderParam = new SearchParameterInfo("gender", "gender");
var codeParam = new SearchParameterInfo("code", "code");
var missing1 = Expression.MissingSearchParameter(genderParam, false);
var missing2 = Expression.MissingSearchParameter(codeParam, false);

var predicate = Expression.And(missing1, missing2);

var missingParams = new List<MissingSearchParameterExpression>();
var resourceTypeIds = new HashSet<short>();
bool foundSurrogateId = false;

// Act
SqlServerSearchService.ResourceSearchParamStats.CollectNotExistsLeaves(
predicate, missingParams, resourceTypeIds, null, ref foundSurrogateId);

// Assert: Two missing params collected — ProcessNotExistsForStats would skip this (ambiguous)
Assert.Equal(2, missingParams.Count);
Assert.False(foundSurrogateId);
}

[Fact]
public void CollectNotExistsLeaves_MixedPredicates_CollectsAllLeafTypes()
{
// Arrange: MissingSearchParameterExpression + unrelated param + ResourceSurrogateId
var genderParam = new SearchParameterInfo("gender", "gender");
var missingExpr = Expression.MissingSearchParameter(genderParam, false);

var otherParam = new SearchParameterInfo("active", "active");
var otherExpr = Expression.SearchParameter(
otherParam,
Expression.StringEquals(FieldName.TokenCode, null, "true", false));

var surrogateIdExpr = Expression.SearchParameter(
SqlSearchParameters.ResourceSurrogateIdParameter,
Expression.GreaterThanOrEqual(SqlFieldName.ResourceSurrogateId, null, 100L));

var predicate = Expression.And(missingExpr, otherExpr, surrogateIdExpr);

var missingParams = new List<MissingSearchParameterExpression>();
var resourceTypeIds = new HashSet<short>();
bool foundSurrogateId = false;

// Act
SqlServerSearchService.ResourceSearchParamStats.CollectNotExistsLeaves(
predicate, missingParams, resourceTypeIds, null, ref foundSurrogateId);

// Assert
Assert.Single(missingParams);
Assert.Equal("gender", missingParams[0].Parameter.Name);
Assert.True(foundSurrogateId);
}
}
}
129 changes: 126 additions & 3 deletions src/Microsoft.Health.Fhir.SqlServer/Features/Search/SqlServerSearchService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2295,7 +2295,7 @@ private static void PopulateGetResourceSurrogateIdRangesCommand(SqlCommand cmd,
cmd.Parameters.AddWithValue("@ActiveOnly", activeOnly);
}

private class ResourceSearchParamStats
internal class ResourceSearchParamStats
{
private readonly ConcurrentDictionary<(string TableName, string ColumnName, short ResourceTypeId, short SearchParamId), bool> _stats;
private readonly SearchParamTableExpressionQueryGeneratorFactory _queryGeneratorFactory;
Expand Down Expand Up @@ -2328,16 +2328,20 @@ public async Task Create(
{
var tableExpression = expression.SearchParamTableExpressions[tableIndex];

// We support Normal and Union. Skip include/sort/etc.
// We support Normal, Union, and NotExists. Skip include/sort/etc.
if (tableExpression.Kind != SearchParamTableExpressionKind.Normal &&
tableExpression.Kind != SearchParamTableExpressionKind.Union)
tableExpression.Kind != SearchParamTableExpressionKind.Union &&
tableExpression.Kind != SearchParamTableExpressionKind.NotExists)
{
continue;
}

// Collected raw triples (table, resourceTypeId, searchParamId)
var collected = new List<(string Table, short ResourceTypeId, short SearchParamId)>();

// Track whether we also need a ResourceSurrogateId filtered stat
bool hasResourceSurrogateId = false;

if (tableExpression.Kind == SearchParamTableExpressionKind.Normal)
{
ProcessPredicateForStats(tableExpression.Predicate, tableExpression.QueryGenerator, model, tableExpression.ChainLevel, expression, tableIndex, collected, logger, parentMultiaryContext: null, isUnionBranch: false);
Expand All @@ -2351,6 +2355,10 @@ public async Task Create(
ProcessUnionBranch(branch, tableExpression.QueryGenerator, model, tableExpression.ChainLevel, expression, tableIndex, collected, logger);
}
}
else if (tableExpression.Kind == SearchParamTableExpressionKind.NotExists)
{
ProcessNotExistsForStats(tableExpression.Predicate, tableExpression.QueryGenerator, model, collected, out hasResourceSurrogateId, logger);
}

// Emit stats rows
foreach (var (table, resourceTypeId, searchParamId) in collected)
Expand All @@ -2365,6 +2373,12 @@ public async Task Create(
{
await Create(table, column, resourceTypeId, searchParamId, sqlRetryService, logger, cancel);
}

// For NotExists with ResourceSurrogateId range constraint, create an additional filtered stat
if (hasResourceSurrogateId)
{
await Create(table, "ResourceSurrogateId", resourceTypeId, searchParamId, sqlRetryService, logger, cancel);
}
}
}
}
Expand Down Expand Up @@ -2394,6 +2408,115 @@ private void ProcessUnionBranch(
}
}

/// <summary>
/// Processes a NotExists predicate (produced by MissingSearchParamVisitor for :missing=true queries)
/// to extract the owning search parameter and optionally detect ResourceSurrogateId range constraints.
/// </summary>
private void ProcessNotExistsForStats(
Expression predicate,
SearchParamTableExpressionQueryGenerator defaultGenerator,
SqlServerFhirModel model,
List<(string Table, short ResourceTypeId, short SearchParamId)> collected,
out bool hasResourceSurrogateId,
ILogger logger)
{
hasResourceSurrogateId = false;

var missingParams = new List<MissingSearchParameterExpression>();
var resourceTypeIds = new HashSet<short>();
bool foundSurrogateId = false;

CollectNotExistsLeaves(predicate, missingParams, resourceTypeIds, model, ref foundSurrogateId);

// Conservative: skip if predicate resolves to anything other than exactly one owning search parameter
if (missingParams.Count != 1)
{
return;
}

var missingParam = missingParams[0];

// Skip synthetic parameters
if (missingParam.Parameter.Name == SqlSearchParameters.PrimaryKeyParameterName ||
missingParam.Parameter.Name == SqlSearchParameters.ResourceSurrogateIdParameterName)
{
return;
}

// Resolve the table for this parameter
var specificGenerator = missingParam.AcceptVisitor(_queryGeneratorFactory, _queryGeneratorFactory.InitialContext) ?? defaultGenerator;
var tableName = specificGenerator.Table.TableName;

// Resolve search param ID
if (!model.TryGetSearchParamId(missingParam.Parameter.Url, out var searchParamId) || searchParamId == 0)
{
return;
}

// Fall back to base resource types if none found in predicate
if (resourceTypeIds.Count == 0 && missingParam.Parameter.BaseResourceTypes?.Count > 0)
{
foreach (var baseType in missingParam.Parameter.BaseResourceTypes)
{
if (model.TryGetResourceTypeId(baseType, out var rtId))
{
resourceTypeIds.Add(rtId);
}
}
}

if (resourceTypeIds.Count == 0)
{
return;
}

hasResourceSurrogateId = foundSurrogateId;

foreach (var rtId in resourceTypeIds)
{
collected.Add((tableName, rtId, searchParamId));
}
}

/// <summary>
/// Recursively collects MissingSearchParameterExpression leaves, resource type constraints,
/// and detects ResourceSurrogateId range constraints from a NotExists predicate.
/// </summary>
internal static void CollectNotExistsLeaves(
Expression expression,
List<MissingSearchParameterExpression> missingParams,
HashSet<short> resourceTypeIds,
SqlServerFhirModel model,
ref bool foundSurrogateId)
{
switch (expression)
{
case MissingSearchParameterExpression msp:
missingParams.Add(msp);
break;

case SearchParameterExpression spe:
if (spe.Parameter.Name == SearchParameterNames.ResourceType)
{
CollectResourceTypesFromExpression(spe.Expression, model, resourceTypeIds);
}
else if (spe.Parameter.Name == SqlSearchParameters.ResourceSurrogateIdParameterName)
{
foundSurrogateId = true;
}

break;

case MultiaryExpression multi:
foreach (var inner in multi.Expressions)
{
CollectNotExistsLeaves(inner, missingParams, resourceTypeIds, model, ref foundSurrogateId);
}

break;
}
}

private void ProcessPredicateForStats(
Expression predicate,
SearchParamTableExpressionQueryGenerator defaultGenerator,
Expand Down
Loading