Initial new parser implementation - #5751
Open
Robert Johnson (LTA-Thinking) wants to merge 100 commits into
Open
Initial new parser implementation #5751Robert Johnson (LTA-Thinking) wants to merge 100 commits into
Robert Johnson (LTA-Thinking) wants to merge 100 commits into
Conversation
…terate - BaseSqlParser: Fix column selection in chain mode to select target resource IDs (RefResourceTypeId/RefResourceSurrogateId) instead of source IDs when ChainLevel > 0 - ChainedSqlParser: Fix source join for multi-level chains, rewrite final CTE assembly to properly join back through ref CTE - ReversedChainSqlParser: Complete rewrite using builder pattern, fix column naming (flip Ref columns for reverse direction), remove IsHistory/IsDeleted check on ReferenceSearchParam table - IncludeSqlParser: Fix JoinMultiLine -> InnerJoin, support iterate includes without Row column dependency - RevIncludeSqlParser: Simplify to match IncludeSqlParser structure, fix IsHistory/IsDeleted check on correct table alias - ParserUtil: Add Row=0 to UNION ALL includes, add includeRow param for iterate dependency unions - ParserOptions: Add IsIterateInclude flag - SqlServerSearchService: Fix async method without await warning Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
- Fix SortSqlParser.CreateSortCte: remove references to IsMatch/IsPartial/Row columns that don't exist in the source CTE at that point - Fix ORDER BY ORDER BY duplication: CreateOrderByClause no longer includes the 'ORDER BY' prefix since .OrderBy() adds it - Fix Count CTE ordering: use SortValue for ROW_NUMBER() and ORDER BY when a sort CTE exists, instead of always using ResourceTypeId/ResourceSurrogateId - Fix sort pagination: preserve ResourceSurrogateId from continuation token as tiebreaker in sort CTE to prevent duplicate results across pages - Fix special sort (_lastUpdated) pagination: don't clear ContinuationToken when sort is a special parameter (it's needed by base CTE for filtering) - Fix _include/_revinclude without other search params: generate base system CTE when only include parameters are present (prevents null return) - Use hasSortCte flag for reliable SortValue column detection Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
- Fix ascending sort pagination: set SortQuerySecondPhase=true when continuation token has a SortValue for ascending non-_lastUpdated sorts, preventing phase 1 (missing=true) from re-running on subsequent pages. - Skip two-phase sort logic when a continuation token is already present (not the first page). - Fix UNION ALL column order mismatch when sort + include are combined: use explicit column list to ensure count CTE and include CTE columns align positionally. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Implement NotReferencedSqlParser that generates NOT EXISTS subqueries against the ReferenceSearchParam table to find resources that are not referenced by other resources. Supports: - *:* (wildcard all - no references from any resource/param) - <ResourceType>:* (wildcard param - no refs from specific type) - <ResourceType>:<param> (specific resource type and search param) Invalid formats (no colon separator) are gracefully skipped, letting the SearchOptionsFactory handle the warning. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Add ResultCteName property to ParserOptions so chain/reverse-chain
parsers communicate the actual CTE name they produce. Previously,
outer parsers assumed a fixed CTE naming pattern (cte{N}chain{level})
that broke when the inner parser was itself a chain or reverse chain,
which produces cte{N}chain{parent}_search instead.
Updated ChainedSqlParser, ReversedChainSqlParser, and BaseSqlParser
to set/read ResultCteName, fixing:
- Nested reverse chain (_has:...:_has:...)
- Reverse chain with chained search param (_has:...:device.manufacturer)
- Forward chain into reverse chain (link:Patient._has:...)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
Copilot started reviewing on behalf of
Robert Johnson (LTA-Thinking)
September 11, 2026 17:02
View session
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Direct SQL parsing and reindex execution contain multiple correctness, compatibility, and completion-accounting regressions.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Replaces the SQL expression-tree search pipeline with direct query-parameter-to-SQL parsing, alongside substantial reindexing, logging, tooling, dependency, and test changes.
Changes:
- Adds direct SQL parsers and removes legacy expression visitors/generators.
- Reworks reindex job execution, persistence, and reporting.
- Adds inbound request logging and updates build/test infrastructure.
File summaries
| File | Description |
|---|---|
tools/SqlSearchDebugger/SqlSearchDebugger.csproj |
Adds debugger project. |
tools/SqlSearchDebugger/Mocks/FakeServiceProviders.cs |
Adds debugger mocks. |
tools/SqlSearchDebugger/Mocks/FakeMediator.cs |
Adds mediator mock. |
tools/Microsoft.Health.Fhir.R4.ResourceParser/Code/MinimalSearchParameterDefinitionManager.cs |
Implements updated interface. |
test/.../Persistence/SqlServerWatchdogTests.cs |
Removes temporary stats checks. |
test/.../Persistence/SqlServerSearchParameterStatusDataStoreTests.cs |
Updates reindex construction. |
test/.../Persistence/SqlServerCreateStatsTests.cs |
Isolates newly cached stats. |
test/.../Persistence/QueryPlanReuseCheckerTests.cs |
Updates collection type. |
test/.../Persistence/FhirStorageTestsFixture.cs |
Reuses context accessor. |
test/.../Persistence/CosmosDbFhirStorageTestsFixture.cs |
Supplies model provider. |
test/.../Features/Smart/SmartSearchSharedFixture.cs |
Simplifies wrapper hash setup. |
test/.../Rest/Search/FailingSearchParameterStatusManager.cs |
Updates status-manager contract. |
test/.../Rest/InProcTestFhirServer.cs |
Changes test configuration and limits. |
test/.../Rest/CompartmentTests.cs |
Advances pagination guard. |
test/.../Rest/BundleEdgeCaseTests.cs |
Tests resource-less entries. |
src/Microsoft.Health.TaskManagement/JobExecutionSoftFailureException.cs |
Allows null inner exception. |
src/Microsoft.Health.Fhir.Tests.Common/TestFiles/Normative/Bundle-TransactionWithResourcelessEntry.json |
Adds validation fixture. |
src/Microsoft.Health.Fhir.Tests.Common/Microsoft.Health.Fhir.Tests.Common.csproj |
Embeds validation fixture. |
src/Microsoft.Health.Fhir.SqlServer/Registration/FhirServerBuilderSqlServerRegistrationExtensions.cs |
Registers direct SQL parser. |
src/Microsoft.Health.Fhir.SqlServer/Features/Storage/SqlServerFhirModel.cs |
Handles abstract resource types. |
src/Microsoft.Health.Fhir.SqlServer/Features/Storage/SqlServerFhirDataStore.cs |
Changes reindex conflict handling. |
src/Microsoft.Health.Fhir.SqlServer/Features/Search/SqlServerSearchParameterValidator.cs |
Replaces SQL validation logic. |
src/Microsoft.Health.Fhir.SqlServer/Features/Search/SqlSearchParser/SqlSearchParameterHashAppender.cs |
Adds parameter hash comments. |
src/Microsoft.Health.Fhir.SqlServer/Features/Search/SqlSearchParser/SpecialParsers/SystemSqlParser.cs |
Adds system search SQL. |
src/Microsoft.Health.Fhir.SqlServer/Features/Search/SqlSearchParser/SearchParameterIdWrapper.cs |
Adds parameter-ID model. |
src/Microsoft.Health.Fhir.SqlServer/Features/Search/SqlSearchParser/QueryStringParser.cs |
Adds query-string parsing. |
src/Microsoft.Health.Fhir.SqlServer/Features/Search/SqlSearchParser/ISqlParser.cs |
Defines parser contract. |
src/Microsoft.Health.Fhir.SqlServer/Features/Search/SqlSearchParser/ISearchParameterSqlParser.cs |
Defines search parser API. |
src/Microsoft.Health.Fhir.SqlServer/Features/Search/SqlSearchParser/CompositeParsers/TokenTokenCompositeSqlParser.cs |
Adds token-token parser. |
src/Microsoft.Health.Fhir.SqlServer/Features/Search/SqlSearchParser/CompositeParsers/TokenStringCompositeSqlParser.cs |
Adds token-string parser. |
src/Microsoft.Health.Fhir.SqlServer/Features/Search/SqlSearchParser/CompositeParsers/TokenQuantityCompositeSqlParser.cs |
Adds token-quantity parser. |
src/Microsoft.Health.Fhir.SqlServer/Features/Search/SqlSearchParser/CompositeParsers/TokenNumberNumberCompositeSqlParser.cs |
Adds range composite parser. |
src/Microsoft.Health.Fhir.SqlServer/Features/Search/SqlSearchParser/CompositeParsers/TokenDateTimeCompositeSqlParser.cs |
Adds token-date parser. |
src/Microsoft.Health.Fhir.SqlServer/Features/Search/SqlSearchParser/CompositeParsers/ReferenceTokenCompositeSqlParser.cs |
Adds reference-token parser. |
src/Microsoft.Health.Fhir.SqlServer/Features/Search/SqlSearchParser/CompositeParsers/CompositeType.cs |
Defines composite kinds. |
src/Microsoft.Health.Fhir.SqlServer/Features/Search/SqlSearchParser/ChainSearchEntry.cs |
Models chained searches. |
src/Microsoft.Health.Fhir.SqlServer/Features/Search/SqlSearchParser/BaseParsers/StringSqlParser.cs |
Adds string SQL parsing. |
src/Microsoft.Health.Fhir.SqlServer/Features/Search/SqlSearchOptions.cs |
Derives sort state from parameters. |
src/Microsoft.Health.Fhir.SqlServer/Features/Search/SqlSearchConstants.cs |
Adds hash markers. |
src/Microsoft.Health.Fhir.SqlServer/Features/Search/SqlQueryHashCalculator.cs |
Removes new hash comments. |
src/Microsoft.Health.Fhir.SqlServer/Features/Search/QueryPlanReuseChecker.cs |
Removes legacy dependency. |
src/Microsoft.Health.Fhir.SqlServer/Features/Search/Expressions/Visitors/TopRewriter.cs |
Removes legacy rewriter. |
src/Microsoft.Health.Fhir.SqlServer/Features/Search/Expressions/Visitors/SqlExpressionRewriterWithInitialContext.cs |
Removes legacy base class. |
src/Microsoft.Health.Fhir.SqlServer/Features/Search/Expressions/Visitors/SqlExpressionRewriter.cs |
Removes SQL visitor base. |
src/Microsoft.Health.Fhir.SqlServer/Features/Search/Expressions/Visitors/SearchParamTableExpressionReorderer.cs |
Removes expression reordering. |
src/Microsoft.Health.Fhir.SqlServer/Features/Search/Expressions/Visitors/RemoveIncludesRewriter.cs |
Removes include rewriting. |
src/Microsoft.Health.Fhir.SqlServer/Features/Search/Expressions/Visitors/QueryGenerators/UriQueryGenerator.cs |
Removes URI generator. |
src/.../QueryGenerators/TokenTokenCompositeQueryGenerator.cs |
Removes composite generator. |
src/.../QueryGenerators/TokenTextQueryGenerator.cs |
Removes token-text generator. |
src/.../QueryGenerators/TokenStringCompositeQueryGenerator.cs |
Removes composite generator. |
src/.../QueryGenerators/TokenQuantityCompositeQueryGenerator.cs |
Removes composite generator. |
src/.../QueryGenerators/TokenNumberNumberQueryGenerator.cs |
Removes range generator. |
src/.../QueryGenerators/TokenDateTimeCompositeQueryGenerator.cs |
Removes composite generator. |
src/.../QueryGenerators/StringQueryGenerator.cs |
Removes string generator. |
src/.../QueryGenerators/SearchParamTableExpressionQueryGenerator.cs |
Removes generator base. |
src/.../QueryGenerators/SearchParameterQueryGeneratorContext.cs |
Removes generator context. |
src/.../QueryGenerators/ResourceTypeIdParameterQueryGenerator.cs |
Removes type generator. |
src/.../QueryGenerators/ResourceTableSearchParameterQueryGenerator.cs |
Removes resource generator. |
src/.../QueryGenerators/ResourceSurrogateIdParameterQueryGenerator.cs |
Removes surrogate generator. |
src/.../QueryGenerators/ResourceIdParameterQueryGenerator.cs |
Removes ID generator. |
src/.../QueryGenerators/ReferenceTokenCompositeQueryGenerator.cs |
Removes composite generator. |
src/.../QueryGenerators/ReferenceQueryGenerator.cs |
Removes reference generator. |
src/.../QueryGenerators/PrimaryKeyRangeParameterQueryGenerator.cs |
Removes range generator. |
src/.../QueryGenerators/NumberQueryGenerator.cs |
Removes number generator. |
src/.../QueryGenerators/NotReferencedQueryGenerator.cs |
Removes anti-reference generator. |
src/.../QueryGenerators/InQueryGenerator.cs |
Removes IN generator. |
src/.../QueryGenerators/IncludeQueryGenerator.cs |
Removes include generator. |
src/.../QueryGenerators/DateTimeQueryGenerator.cs |
Removes date generator. |
src/.../QueryGenerators/CompositeQueryGenerator.cs |
Removes composite base. |
src/.../QueryGenerators/CompartmentQueryGenerator.cs |
Removes compartment generator. |
src/.../QueryGenerators/ChainLinkQueryGenerator.cs |
Removes chain generator. |
src/.../Visitors/NumericRangeRewriter.cs |
Removes numeric rewriting. |
src/.../Visitors/ISqlExpressionVisitor.cs |
Removes visitor contract. |
src/.../Visitors/IncludesOperationRewriter.cs |
Removes include operation rewriting. |
src/.../Visitors/IncludeMatchSeedRewriter.cs |
Removes include seed rewriting. |
src/.../Visitors/FlatteningRewriter.cs |
Removes expression flattening. |
src/.../Visitors/DefaultSqlExpressionVisitor.cs |
Removes default visitor. |
src/.../Expressions/SqlFieldName.cs |
Removes SQL field constants. |
src/.../Expressions/SearchParamTableExpressionKind.cs |
Removes expression kinds. |
src/Microsoft.Health.Fhir.SqlServer/Features/Operations/Export/SqlExportOrchestratorJob.cs |
Validates export types. |
src/Microsoft.Health.Fhir.SqlServer/AssemblyInfo.cs |
Exposes internals to debugger. |
src/Microsoft.Health.Fhir.SqlServer.UnitTests/Microsoft.Health.Fhir.SqlServer.UnitTests.csproj |
Updates test dependencies. |
src/.../SqlSearchParser/ParserOptionsTests.cs |
Tests parameter binding. |
src/.../Search/SqlQueryHashCalculatorTests.cs |
Tests hash removal. |
src/.../Search/QueryPlanReuseCheckerTests.cs |
Updates collection type. |
src/.../SearchParamTableExpressionQueryGeneratorFactoryTests.cs |
Removes legacy tests. |
src/.../QueryGenerators/ModelInfoProviderFixture.cs |
Removes legacy fixture. |
src/.../Expressions/RemoveIncludesRewriterTests.cs |
Removes rewriter tests. |
src/.../Expressions/NotReferencingSqlGenerationTests.cs |
Removes generator tests. |
src/.../Export/SqlExportOrchestratorJobTests.cs |
Tests export validation. |
src/Microsoft.Health.Fhir.Shared.Core/Features/Validation/ServerProvideProfileValidation.cs |
Adds resource-type query. |
src/.../Patch/ConditionalPatchResourceHandler.cs |
Propagates weak ETag. |
src/.../SearchParameterState/SearchParameterStateUpdateHandler.cs |
Updates manager API. |
src/.../SearchParameterState/SearchParameterStateHandler.cs |
Updates manager API. |
src/.../Extensions/ReindexMediatorExtensions.cs |
Changes reindex numeric types. |
src/.../Extensions/ReindexJobRecordExtensions.cs |
Revises progress reporting. |
src/Microsoft.Health.Fhir.Shared.Core.UnitTests/...projitems |
Includes export tests. |
src/.../Features/Search/SearchServiceTests.cs |
Updates method visibility. |
src/.../SearchParameterDefinitionManagerTests.cs |
Updates status API. |
src/.../SearchParameterBehaviorTests.cs |
Updates status API. |
src/.../ConditionalPatchResourceHandlerTests.cs |
Tests weak ETags. |
src/.../DeletionServiceTests.cs |
Updates deletion API. |
src/.../ResourceWrapperFactoryTests.cs |
Tests hash refresh. |
src/.../SearchParameterStateUpdateHandlerTests.cs |
Updates operation assertions. |
src/.../MemberMatchServiceTests.cs |
Checks direct query parameters. |
src/Microsoft.Health.Fhir.Shared.Api/Registration/FhirServerServiceCollectionExtensions.cs |
Registers request logger. |
src/.../Filters/OperationOutcomeExceptionFilterAttribute.cs |
Supplies logger to result. |
src/.../Filters/FhirRequestContextRouteDataPopulatingFilterAttribute.cs |
Adds result logger. |
src/.../Exceptions/BaseExceptionMiddleware.cs |
Adds result logger. |
src/.../ActionResults/OperationVersionsResult.cs |
Requires logger. |
src/.../ActionResults/MemberMatchResult.cs |
Requires logger. |
src/.../Controllers/ValidateController.cs |
Adds response logger. |
src/.../Controllers/TerminologyController.cs |
Adds response logger. |
src/.../Controllers/OperationDefinitionController.cs |
Adds response logger. |
src/.../Controllers/MemberMatchController.cs |
Supplies result logger. |
src/.../Controllers/IncludesController.cs |
Adds response logger. |
src/.../Controllers/ImportController.cs |
Supplies import logger. |
src/.../Controllers/EverythingController.cs |
Adds response logger. |
src/.../Controllers/DocRefController.cs |
Adds response logger. |
src/Microsoft.Health.Fhir.Shared.Api.UnitTests/...projitems |
Includes logger tests. |
src/.../ThrottlingMiddlewareTests.cs |
Supplies inbound logger. |
src/.../RuntimeStateMiddlewareTests.cs |
Supplies inbound logger. |
src/.../ImportResultExtensionsTests.cs |
Supplies null logger. |
src/.../ExportResultExtensionsTests.cs |
Supplies null logger. |
src/.../FhirRequestContextRouteDataPopulatingFilterAttributeTests.cs |
Updates filter setup. |
src/.../AuditLoggingFilterAttributeTests.cs |
Updates result setup. |
src/.../OperationVersionsResultTests.cs |
Updates result setup. |
src/.../ValidateControllerTests.cs |
Updates controller setup. |
src/.../TerminologyControllerTests.cs |
Updates controller setup. |
src/.../OperationDefinitionControllerTests.cs |
Updates controller setup. |
src/.../IncludesControllerTests.cs |
Updates controller setup. |
src/.../FhirControllerTests.cs |
Updates controller setup. |
src/.../ExportControllerTests.cs |
Updates controller setup. |
src/.../EverythingControllerTests.cs |
Updates controller setup. |
src/.../DocRefControllerTests.cs |
Updates controller setup. |
src/.../BulkUpdateControllerTests.cs |
Updates controller setup. |
src/.../BulkDeleteControllerTests.cs |
Updates controller setup. |
src/Microsoft.Health.Fhir.R4.Core.UnitTests/.../MemberMatchServiceTests.cs |
Checks direct query parameters. |
src/Microsoft.Health.Fhir.CosmosDb/Features/Storage/Queues/CosmosQueueClient.cs |
Initializes running jobs. |
src/.../Operations/CosmosFhirOperationDataStore.cs |
Removes legacy reindex lookup. |
src/.../Storage/CosmosFhirDataStore.cs |
Changes conflict handling. |
src/.../Export/CosmosExportOrchestratorJob.cs |
Validates export types. |
src/Microsoft.Health.Fhir.CosmosDb.UnitTests/.../CosmosExportOrchestratorJobTests.cs |
Tests export validation. |
src/Microsoft.Health.Fhir.Core/Properties/AssemblyInfo.cs |
Exposes internals to debugger. |
src/.../Messages/Reindex/CreateReindexRequest.cs |
Changes batch-size types. |
src/.../Logging/LongRunningOperationStatistics.cs |
Removes unused import. |
src/.../Features/Search/SearchService.cs |
Extends search operations. |
src/.../Features/Search/SearchResultReindex.cs |
Removes continuation metadata. |
src/.../Features/Search/SearchOptions.cs |
Adds direct query parameters. |
src/.../Registry/ISearchParameterStatusManager.cs |
Revises status contract. |
src/.../Parameters/ISearchParameterOperations.cs |
Revises operation contract. |
src/.../Search/ISearchService.cs |
Adds range/type operations. |
src/.../Routing/IUrlResolver.cs |
Adds resolution availability. |
src/.../Persistence/ResourceWrapperFactory.cs |
Refreshes index hash. |
src/.../Persistence/ResourceWrapper.cs |
Encapsulates hash mutation. |
src/.../Reindex/ReindexProcessingJobResult.cs |
Simplifies processing result. |
src/.../Reindex/ReindexProcessingJobDefinition.cs |
Changes batch-size types. |
src/.../Reindex/ReindexOrchestratorJobResult.cs |
Revises job result. |
src/.../Reindex/ReindexOrchestratorJobDefinition.cs |
Changes batch-size types. |
src/.../Models/ReindexJobQueryStatusConverter.cs |
Removes legacy converter. |
src/.../Models/ReindexJobQueryStatus.cs |
Removes legacy query status. |
src/.../Models/ReindexJobQueryResourceCountsConverter.cs |
Removes legacy converter. |
src/.../Reindex/ILegacyReindexOperationDataStore.cs |
Removes legacy contract. |
src/.../Reindex/CreateReindexRequestHandler.cs |
Creates new queue jobs. |
src/.../Reindex/CancelReindexRequestHandler.cs |
Uses queue cancellation. |
src/.../Operations/IFhirOperationDataStore.cs |
Revises reindex persistence API. |
src/.../Export/ExportOrchestratorJob.cs |
Adds type validation helper. |
src/.../Features/KnownQueryParameterNames.cs |
Adds reverse-chain name. |
src/.../SupportedSearchParameterDefinitionManager.cs |
Exposes derived types. |
src/.../SearchParameterDefinitionManager.cs |
Adds direct query parameters. |
src/.../SearchableSearchParameterDefinitionManager.cs |
Exposes derived types. |
src/.../ISearchParameterDefinitionManager.cs |
Extends definition contract. |
src/.../Conformance/SystemConformanceProvider.cs |
Adds background context. |
src/.../RuntimeStateConfigurationExtensions.cs |
Validates configuration input. |
src/.../Configs/ReindexJobConfiguration.cs |
Revises reindex settings. |
src/Microsoft.Health.Fhir.Api/Features/Throttling/ThrottlingMiddleware.cs |
Logs terminal throttling. |
src/.../Routing/UrlResolver.cs |
Implements availability check. |
src/.../Logging/IHttpInboundRequestLogger.cs |
Defines request logger. |
src/.../ActionResults/TooManyRequestsActionResult.cs |
Requires logger. |
src/.../ActionResults/ResourceActionResult.cs |
Adds logging dependency. |
src/.../ActionResults/OperationOutcomeResult.cs |
Requires logger. |
src/.../ActionResults/JobResult.cs |
Requires logger. |
src/.../ActionResults/ExportResult.cs |
Requires logger. |
nuget.config |
Changes public package source. |
global.json |
Updates .NET SDK. |
Directory.Packages.props |
Updates package versions. |
build/pr-variables.yml |
Configures reindex polling. |
build/jobs/scripts/Provision-AcaDeploy.ps1 |
Deploys polling setting. |
build/jobs/provision-deploy.yml |
Passes polling setting. |
build/jobs/docker-build-push.yml |
Configures Docker storage. |
build/jobs/docker-build-all.yml |
Normalizes final line. |
build/dotnet9-compat/global.json |
Updates compatibility SDK. |
build/docker/Dockerfile |
Updates SDK/runtime images. |
build/ci-variables.yml |
Configures reindex polling. |
.github/workflows/validate-prs.yml |
Updates workflow actions. |
.github/workflows/codeql.yml |
Updates workflow actions. |
.github/agents/fast-coding-agent.agent.md |
Updates agent model. |
.github/agents/complex-coding-agent.agent.md |
Updates agent model. |
.github/agents/coding-agent.agent.md |
Updates agent model. |
Review details
- Files reviewed: 180/390 changed files
- Comments generated: 12
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
57
to
+59
| var jobRecord = new ReindexJobRecord( | ||
| request.TargetResourceTypes, | ||
| request.MaximumResourcesPerQuery ?? _reindexJobConfiguration.MaximumNumberOfResourcesPerQuery, | ||
| request.MaximumResourcesPerWrite ?? _reindexJobConfiguration.MaximumNumberOfResourcesPerWrite, | ||
| request.QueryDelayIntervalInMilliseconds ?? _reindexJobConfiguration.QueryDelayIntervalInMilliseconds); | ||
| request.MaximumResourcesPerQuery ?? _reindexJobConfiguration.MaximumNumberOfResourcesPerQuery, | ||
| request.MaximumResourcesPerWrite ?? _reindexJobConfiguration.MaximumNumberOfResourcesPerWrite); |
Comment on lines
665
to
+667
| case HttpStatusCode.PreconditionFailed: | ||
| _logger.LogError(string.Format(Microsoft.Health.Fhir.Core.Resources.ResourceVersionConflict, WeakETag.FromVersionId(resourceWrapper.Version))); | ||
| throw new PreconditionFailedException(string.Format(Microsoft.Health.Fhir.Core.Resources.ResourceVersionConflict, WeakETag.FromVersionId(resourceWrapper.Version))); | ||
| _logger.LogWarning(string.Format(Microsoft.Health.Fhir.Core.Resources.ResourceVersionConflict, WeakETag.FromVersionId(resourceWrapper.Version))); | ||
| return resourceWrapper; |
| return false; | ||
| } | ||
|
|
||
| return QueryParams.ContainsKey(Sort[0].searchParameterInfo.Code); |
Comment on lines
+27
to
+37
| return valueModifier switch | ||
| { | ||
| "gt" => $"{endDateTimeReference} > {options.AddParameter(VLatest.DateTimeSearchParam.EndDateTime, parsedValue.End, includeInHash: true)}", | ||
| "ge" => $"{endDateTimeReference} >= {options.AddParameter(VLatest.DateTimeSearchParam.EndDateTime, parsedValue.Start, includeInHash: true)}", | ||
| "lt" => $"{startDateTimeReference} < {options.AddParameter(VLatest.DateTimeSearchParam.StartDateTime, parsedValue.Start, includeInHash: true)}", | ||
| "le" => $"{startDateTimeReference} <= {options.AddParameter(VLatest.DateTimeSearchParam.StartDateTime, parsedValue.End, includeInHash: true)}", | ||
| "sa" => $"{startDateTimeReference} > {options.AddParameter(VLatest.DateTimeSearchParam.StartDateTime, parsedValue.End, includeInHash: true)}", | ||
| "eb" => $"{endDateTimeReference} < {options.AddParameter(VLatest.DateTimeSearchParam.EndDateTime, parsedValue.Start, includeInHash: true)}", | ||
| "ne" => $"({endDateTimeReference} > {options.AddParameter(VLatest.DateTimeSearchParam.EndDateTime, parsedValue.End, includeInHash: true)} OR {startDateTimeReference} < {options.AddParameter(VLatest.DateTimeSearchParam.StartDateTime, parsedValue.Start, includeInHash: true)})", | ||
| "eq" => $"{endDateTimeReference} >= {options.AddParameter(VLatest.DateTimeSearchParam.EndDateTime, parsedValue.Start, includeInHash: true)} AND {startDateTimeReference} <= {options.AddParameter(VLatest.DateTimeSearchParam.StartDateTime, parsedValue.End, includeInHash: true)}", | ||
| _ => throw new InvalidOperationException($"Unsupported modifier: {valueModifier}"), |
Comment on lines
+115
to
+117
| "ne" => $"({highValueReference} > {options.AddParameter(highValueColumn, parsedValue, includeInHash: true)} OR {lowValueReference} < {options.AddParameter(lowValueColumn, parsedValue, includeInHash: true)})", | ||
| "eq" => $"{highValueReference} >= {options.AddParameter(highValueColumn, parsedValue, includeInHash: true)} AND {lowValueReference} <= {options.AddParameter(lowValueColumn, parsedValue, includeInHash: true)}", | ||
| "ap" => BuildApproximateCondition(parsedValue, options, suffix, tableName, highValueColumn, highValueColumnName, lowValueColumn, lowValueColumnName), |
Comment on lines
984
to
+986
| if (failedResourceCount != 0) | ||
| { | ||
| string message = string.Format(Core.Resources.ReindexingResourceVersionConflictWithCount, failedResourceCount); | ||
| string userAction = Core.Resources.ReindexingUserAction; | ||
| _logger.LogError("{Error}", message); | ||
| throw new PreconditionFailedException(message + " " + userAction); | ||
| _logger.LogWarning(string.Format(Core.Resources.ReindexingResourceVersionConflictWithCount, failedResourceCount) + " " + Core.Resources.ReindexingUserAction); |
Comment on lines
126
to
128
| public override async Task<ReindexJobWrapper> GetReindexJobByIdAsync(string jobId, CancellationToken cancellationToken) | ||
| { | ||
| if (IsLegacyJob(jobId)) | ||
| { | ||
| // try old job records | ||
| var oldJobs = (ILegacyReindexOperationDataStore)this; | ||
| return await oldJobs.GetLegacyReindexJobByIdAsync(jobId, cancellationToken); | ||
| } | ||
|
|
||
| return await base.GetReindexJobByIdAsync(jobId, cancellationToken); |
Comment on lines
+134
to
+137
| var lowerBoundParameter = options.AddParameter(highValueColumn, parsedValue, includeInHash: true); | ||
| var upperBoundParameter = options.AddParameter(lowValueColumn, parsedValue, includeInHash: true); | ||
|
|
||
| return $"({highValueReference} >= {lowerBoundParameter} * 0.9 AND {lowValueReference} <= {upperBoundParameter} * 1.1)"; |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
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
Removes the Expression based SQL parser and replaces it with one that goes directly from query parameters to SQL queries.
Related issues
Addresses User Story 199220
Testing
Existing E2E tests are utilized heavily to ensure feature parity.
FHIR Team Checklist
Semver Change (docs)
Patch|Skip|Feature|Breaking (reason)