refactor(drive): unify indexOnly terminal matching into the dpp index matcher - #4502
Conversation
… matcher 'Which index serves this query' now has one answer site: Index::matches is extracted into a shared component-list algorithm, and matches_including_terminal runs the SAME algorithm with the terminal (an indexOnly index's member-key property) as the deepest matchable component — coverage, the in-position rule and the order-by suffix all extend naturally, and an unused terminal never costs score, so results stay comparable with the generic matcher's. index_for_types_matching_including_terminal wraps it with the route precedence encoded in selection itself: a candidate covering the query WITHOUT its terminal always beats every terminal-using candidate, and within each class the difference scoring (and index-map-order tie-break) is exactly the generic matcher's — best-match replaces the drive-side first-satisfying-index-in-declaration-order loop, which is deleted. Drive's terminal selection now only validates clause SHAPE on the matcher's winner (prefix equalities, non-equality placement, range-needs-orderBy). Shares the index_for_types version gate: terminal participation can only change outcomes on indexOnly types, which cannot exist below PV14. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
ℹ️ Review skipped (commit a0d1b23) |
f251693
into
feat/index-only-read-completion
2 participants
Issue being fixed or feature implemented
First of three stacked architecture PRs on #4499 (see its review discussion): the terminal-clause route shipped with a second, hand-rolled index matcher living in rs-drive beside dpp's
index_for_types_matching— "which index serves this query" was answered in two places, and the terminal matcher took the first satisfying index in declaration order instead of scoring candidates.What was done?
One matching algorithm.
Index::matchesis extracted into a shared component-list core, and the newIndex::matches_including_terminalruns the SAME core with the terminal (an indexOnly index's member-key property) as the index's deepest matchable component — field coverage, thein-position rule and the order-by contiguous-suffix rule all extend naturally. An unused terminal never costs score (it is the entry level itself, always reachable), so scores stay comparable with the generic matcher's and an index is never penalized for merely having a terminal.Precedence encoded in selection, not in routing.
index_for_types_matching_including_terminalreturns(index, difference, terminal_used)with the route precedence inside the matcher itself: a candidate covering the query WITHOUT its terminal always beats every terminal-using candidate (the terminal route is a stand-in for "no ordinary index serves this", never a competitor to one that does), and within each class the difference scoring and index-map-order tie-break are exactly the generic matcher's. On any document type without terminals this is exactlyindex_for_types_matching. Shares theindex_for_typesversion gate — terminal participation can only change outcomes on indexOnly document types, which cannot exist below PV14.Drive's selection shrinks to shape validation. The candidate loop in
index_only_terminal_clause_selectionis deleted; it now asks the dpp matcher for the winner and validates clause SHAPE on it — every prefix property equality-bound, non-equality clauses only on the terminal, a terminal range/inrequires ordering by it. First-match-in-declaration-order becomes difference-scored best-match (byLiker's exact terminal cover now beatsbyHashtagPost's costlier one for{$ownerId, postId}, deterministically).How Has This Been Tested?
New dpp test
terminal_aware_matching_prefers_generic_and_scores_candidatespins the three semantics: generic precedence (byPostbeatsbyLiker's terminal cover of the same field), best-match scoring between terminal candidates, and zero-cost unused terminals. All 15 drive indexOnly e2e tests (including every terminal-route and refusal case from #4499), the 699-test drive query suite, 264 dpp index tests and the drive-abci indexOnly pipeline are green; workspace check and clippy-D warningsclean.Breaking Changes
None on any released surface. Within the unreleased #4499 feature, multi-candidate terminal selection can pick a different (better-scoring) index than declaration order did; selection remains fully deterministic.
Checklist:
Stack: #4499 → this → clause classification (
InternalClausesterminals) → mixed prefix-range + terminal shapes.🤖 Generated with Claude Code