Skip to content

Experiment: atomic target-first chained search with sort support - #5726

Merged
Mikael Weaver (mikaelweave) merged 5 commits into
fix/selective-first-chained-searchfrom
experiment/atomic-target-first-sort
Aug 16, 2026
Merged

Experiment: atomic target-first chained search with sort support#5726
Mikael Weaver (mikaelweave) merged 5 commits into
fix/selective-first-chained-searchfrom
experiment/atomic-target-first-sort

Conversation

@mikaelweave

@mikaelweave Mikael Weaver (mikaelweave) commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

Comparison goal

This draft compares a cleaner atomic target-first chained-search rewrite against PR #5725's narrowly gated implementation.

Changes

  • recognizes and emits the complete target-first CTE group at the root query-generator level
  • owns selective target reference, source backlink, source date predicate, and an immediately following sort projection as one atomic rewrite
  • preserves ascending/descending plain Sort and SortWithFilter behavior, Min/Max selection for multi-valued rows, deduplication, and continuation-token tie breaks
  • preserves missing-value two-phase sort selection, Top, count-only, include/revinclude, and trailing-filter propagation
  • uses an explicit match result with consumed-expression count and returned CTE IDs instead of out parameters, magic return counts, and counter-derived IDs
  • extracts repeated status-filter test setup

Intentional limitation

A sort separated from the target-first predicates by another source filter is deliberately not consumed atomically. The optimized three-CTE filter feeds the existing generic filter/sort pipeline so the intervening predicate is applied before sort projection; this avoids partial semantic changes while retaining the target-first optimization.

Validation

  • baseline SQL Server unit test project build passed before changes
  • updated SQL Server unit test project build passed with 0 warnings and 0 errors
  • focused SqlQueryGeneratorTests passed: 35/35
  • isolated CompartmentQueryGeneratorTests passed: 26/26 after an unrelated transient full-suite failure
  • full Microsoft.Health.Fhir.SqlServer.UnitTests suite passed: 1,072/1,072
  • rubber-duck review found no correctness or simplicity issues

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@codecov-commenter

Codecov Comments Bot (codecov-commenter) commented Aug 16, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.38%. Comparing base (b3f01fa) to head (76dbbd4).

Additional details and impacted files

Impacted file tree graph

@@                          Coverage Diff                           @@
##           fix/selective-first-chained-search    #5726      +/-   ##
======================================================================
+ Coverage                               77.25%   77.38%   +0.13%     
======================================================================
  Files                                    1012     1012              
  Lines                                   37393    37415      +22     
  Branches                                 5690     5689       -1     
======================================================================
+ Hits                                    28887    28953      +66     
+ Misses                                   7131     7098      -33     
+ Partials                                 1375     1364      -11     

see 15 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Preserve later sorts in the generic pipeline so intervening predicates are applied before sort projection, while making atomic match consumption and CTE identities explicit.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Verify the target-first matcher rejects the atomic rewrite and preserve the generic numeric SortValue path for both sort directions.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@mikaelweave
Mikael Weaver (mikaelweave) marked this pull request as ready for review August 16, 2026 06:02
@mikaelweave
Mikael Weaver (mikaelweave) requested a review from a team as a code owner August 16, 2026 06:02
@mikaelweave
Mikael Weaver (mikaelweave) merged commit 56667ea into fix/selective-first-chained-search Aug 16, 2026
47 checks passed
@mikaelweave
Mikael Weaver (mikaelweave) deleted the experiment/atomic-target-first-sort branch August 16, 2026 06:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants