Skip to content

fix(fts): preserve corpus statistics for restricted matches - #9243

Open
ruslan-shaydullin wants to merge 7 commits into
lance-format:mainfrom
ruslan-shaydullin:codex/lance-9058-restricted-match-corpus
Open

ruslan-shaydullin wants to merge 7 commits into
lance-format:mainfrom
ruslan-shaydullin:codex/lance-9058-restricted-match-corpus

Conversation

@ruslan-shaydullin

@ruslan-shaydullin ruslan-shaydullin commented Sep 15, 2026 •

Copy link
Copy Markdown

Related to #9058.

For the two reported single-column Match cases, index optimization changes the winner from alpha to beta after incorporating already appended rows, although the searchable data and query are unchanged.

This partial fix scores eligible restricted matches against committed posting statistics plus the complete unindexed corpus. Flat candidate filtering runs after corpus collection; indexed masks still constrain indexed top-k. A private owner gives each execution a fresh shared scorer. The restricted path handles an effective top-k limit of zero without constructing a zero-sized sort.

The correction covers exact, row-level scalar-string Match queries with a prefilter or explicit fragment selection on the supported append-only corpus. Indexed scoring uses V2/V3 postings. Deletion/stale-text-overlay scoring, legacy postings, fuzzy/phrase/Boolean queries, BM25F/combined_fields, and the unfiltered Hybrid approximation remain outside this correction. #9058 remains open for the broader scoring work.

Validation

Synchronized with upstream main at 978362bcd on October 7, after the combined_fields flat scoring change (#9444) and the DataFusion 55 cleanup (#9640). Only the four import lists conflicted; the four-file PR patch is otherwise the same as the September 28 version. The earlier integration preserved upstream BM25F and fragment-scoped scalar prefilters, adapted the added candidate-filter loader to the current constructor, and checked the exact residual fragment IDs in the scalar-candidate regression. The original single-column scope and full-corpus score/result assertions remain unchanged.

Fresh local validation on synchronized head cc5e99657 with Rust 1.98.1: all 4 plan variants, 65 issue regressions, 35 FTS executor tests, 39 BM25F dataset tests, and 10 upstream scalar-pruning tests pass (the groups overlap; the executor and BM25F groups grew with upstream's own tests). Formatting and workspace clippy with warnings denied pass. Upstream code workflows for this new head have not yet run; the previous head's results below are historical.

Previous head 79ba32875 passed all 39 upstream code checks on September 28, including Rust, Python, Java JNI, license-header, and typo workflows; the four cancelled entries were the PR title, label and format-vote jobs. Those results predate this synchronization. The prior follow-up aligned four Legacy/Stable plan expectations with MatchCorpus without changing runtime code.

Original implementation validation (before this synchronization):

  • Reproduced eight ranking failures on unchanged base a43e0500; checked scores against an independent raw-text BM25 reference.
  • 65 targeted cases passed, including default query construction and offset, stable residual-candidate IDs, real BTree allow/block masks, corpus/candidate boundaries, maintenance, empty/null/deletion controls, failure/cancellation, metrics, and repeated execution.
  • Compatibility suites passed: 31 FTS executor tests, 156 dataset/index tests, 46 overlay tests (one existing benchmark ignored), and 782 inverted-index tests. These groups overlap; all added cases are included in the final targeted run.
  • Final workspace check (tests and benches), clippy with warnings denied, formatting, and applicable commit hooks passed. Validation used Rust 1.97 on macOS arm64 with the locked ci profile.

The global corpus requires reading/tokenizing all unindexed text, including rows outside the candidate set. Corpus construction reuses committed posting statistics without retokenizing indexed text. A scalar prefilter with an overlay-stale index can still contain a consumable OneShotExec; fresh scorer state does not make that existing input replayable. Sequential/concurrent reuse is verified on the append-only fixtures. No performance claim is made.

Thanks to @sbrunk and lance-gatekeeper[bot] for the report and reproductions during #7905; its emission-filter/shared-scorer work informed this follow-up. Developed with AI assistance and checked against the independent reference and native tests.

@github-actions github-actions Bot added the bug Something isn't working label Sep 15, 2026
lance-gatekeeper[bot]

This comment was marked as outdated.

@lance-gatekeeper lance-gatekeeper Bot added K-approved Latest Gatekeeper recommendation permits acceptance. K-risk Latest Gatekeeper recommendation includes a non-blocking risk. labels Sep 15, 2026
Signed-off-by: Ruslan Shaydullin <shaydullin.r.d@outlook.com>
@ruslan-shaydullin

Copy link
Copy Markdown
Author

Thanks @wjones127 for enabling the previous CI run. It exposed outdated expected plans in test_plans; 5328449 updates both storage-format expectations and the comment without changing production code. All four plan variants and 65 FTS regressions now pass locally, as do formatting and workspace clippy with warnings denied.

Could you approve the five code workflows on this updated head? They currently report action_required, including the Rust run. The PR description now reflects these results. Thanks!

@lance-gatekeeper lance-gatekeeper Bot removed K-approved Latest Gatekeeper recommendation permits acceptance. K-risk Latest Gatekeeper recommendation includes a non-blocking risk. labels Sep 21, 2026
lance-gatekeeper[bot]

This comment was marked as outdated.

@lance-gatekeeper lance-gatekeeper Bot added K-approved Latest Gatekeeper recommendation permits acceptance. K-risk Latest Gatekeeper recommendation includes a non-blocking risk. and removed K-approved Latest Gatekeeper recommendation permits acceptance. K-risk Latest Gatekeeper recommendation includes a non-blocking risk. labels Sep 21, 2026
@ruslan-shaydullin

Copy link
Copy Markdown
Author

Hi @sbrunk, would you be able to check the two single-column Match reproductions from #9058 against this PR (5328449)? The covered cases are exact, row-level scalar-string matches on an append-only corpus with either prefilter(true) / id < 2 or with_fragments([0]), using V2/V3 postings. With the same data and query, the selected row and BM25 score should remain stable before and after optimize_indices().

The updated head has passed Rust, Python and Java CI. This remains a partial fix: combined_fields/BM25F, deletion or stale-text overlays, legacy postings, fuzzy/phrase/Boolean queries, and unfiltered Hybrid scoring are outside its scope. A result for either original single-column reproduction would be useful; #9058 stays open for the broader work. Thanks!

@lance-gatekeeper lance-gatekeeper Bot added K-approved Latest Gatekeeper recommendation permits acceptance. K-risk Latest Gatekeeper recommendation includes a non-blocking risk. and removed K-approved Latest Gatekeeper recommendation permits acceptance. K-risk Latest Gatekeeper recommendation includes a non-blocking risk. labels Sep 22, 2026
@sbrunk

sbrunk commented Sep 26, 2026

Copy link
Copy Markdown
Contributor

@ruslan-shaydullin I couldn't find the orignal repro code anymore but based on the repro example in the ticket I checked againt and it looks good!

@lance-gatekeeper lance-gatekeeper Bot added K-approved Latest Gatekeeper recommendation permits acceptance. K-risk Latest Gatekeeper recommendation includes a non-blocking risk. and removed K-approved Latest Gatekeeper recommendation permits acceptance. K-risk Latest Gatekeeper recommendation includes a non-blocking risk. labels Sep 26, 2026
Merge current main while preserving restricted Match corpus scoring.
Adapt the candidate-prefilter constructor and assert exact residual
fragment coverage introduced by upstream scalar-index pruning.

Signed-off-by: Ruslan Shaydullin <shaydullin.r.d@outlook.com>
@lance-gatekeeper lance-gatekeeper Bot removed K-approved Latest Gatekeeper recommendation permits acceptance. K-risk Latest Gatekeeper recommendation includes a non-blocking risk. labels Sep 27, 2026
lance-gatekeeper[bot]

This comment was marked as outdated.

@lance-gatekeeper lance-gatekeeper Bot added K-approved Latest Gatekeeper recommendation permits acceptance. K-risk Latest Gatekeeper recommendation includes a non-blocking risk. labels Sep 27, 2026
Merge upstream main after the Rust 1.98.1 toolchain update.
Preserve the existing four-file restricted Match patch unchanged.

Signed-off-by: Ruslan Shaydullin <shaydullin.r.d@outlook.com>
@lance-gatekeeper lance-gatekeeper Bot removed K-approved Latest Gatekeeper recommendation permits acceptance. K-risk Latest Gatekeeper recommendation includes a non-blocking risk. labels Sep 28, 2026
lance-gatekeeper[bot]

This comment was marked as outdated.

@lance-gatekeeper lance-gatekeeper Bot added K-approved Latest Gatekeeper recommendation permits acceptance. K-risk Latest Gatekeeper recommendation includes a non-blocking risk. labels Sep 28, 2026
Merge upstream main after the combined_fields flat scoring change
(lance-format#9444) and the DataFusion 55 cleanup (lance-format#9640). Only the four import
lists conflicted; they now carry both sides, and the restricted Match
patch itself is unchanged.

Signed-off-by: Ruslan Shaydullin <shaydullin.r.d@outlook.com>
@lance-gatekeeper lance-gatekeeper Bot removed K-approved Latest Gatekeeper recommendation permits acceptance. K-risk Latest Gatekeeper recommendation includes a non-blocking risk. labels Oct 6, 2026

@lance-gatekeeper lance-gatekeeper Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Gate recommendation: approve with a non-blocking risk.

The merge with main preserves the separation between candidate filtering and corpus statistics for exact, row-level single-column Match. The new code-analyzer prefilter fixture also keeps its scores and top result stable across index optimization here. This remains a partial fix; #9058 still tracks broader scoring work.

Selective prefilters and fragment selections still read and tokenize the full unindexed text tail, so latency and peak memory scale with that tail. Keeping the FTS index current mitigates this cost.

@lance-gatekeeper lance-gatekeeper Bot added K-approved Latest Gatekeeper recommendation permits acceptance. K-risk Latest Gatekeeper recommendation includes a non-blocking risk. labels Oct 6, 2026
@ruslan-shaydullin

Copy link
Copy Markdown
Author

Synced with main after #9444 and #9640; only the import lists conflicted, the patch itself is unchanged. The plan, #9058, FTS executor, BM25F and scalar-pruning test groups, fmt and clippy all pass locally on the new head. @wjones127 could you approve the workflow runs again when you get a chance?

@lance-gatekeeper lance-gatekeeper Bot added K-approved Latest Gatekeeper recommendation permits acceptance. K-risk Latest Gatekeeper recommendation includes a non-blocking risk. and removed K-risk Latest Gatekeeper recommendation includes a non-blocking risk. K-approved Latest Gatekeeper recommendation permits acceptance. labels Oct 7, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working K-approved Latest Gatekeeper recommendation permits acceptance. K-risk Latest Gatekeeper recommendation includes a non-blocking risk.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants