Skip to content

fix: partition not used yet panic - #3769

Merged
BubbleCal merged 3 commits into
lance-format:mainfrom
BubbleCal:fix-part-not-used-yet-panic
May 2, 2025
Merged

fix: partition not used yet panic#3769
BubbleCal merged 3 commits into
lance-format:mainfrom
BubbleCal:fix-part-not-used-yet-panic

Conversation

@BubbleCal

@BubbleCal BubbleCal commented May 2, 2025

Copy link
Copy Markdown
Contributor

the bug is that we may call execute multiple times, and it causes a panic in datafusion RepartitionExec, the conditions to repro:

  1. the dataset has vector index and the vector index contains at least 2 deltas
  2. the dataset has a scalar index, there are any unindexed rows for the scalar index
  3. do vector search with prefilter

This also improves the performance of vector search with prefilter when there are more than 1 deltas, because before it performed scalar index query & scanning for each delta, but now it does only once

Signed-off-by: BubbleCal <bubble-cal@outlook.com>
@github-actions github-actions Bot added bug Something isn't working A-python Python bindings labels May 2, 2025
BubbleCal added 2 commits May 2, 2025 16:45
Signed-off-by: BubbleCal <bubble-cal@outlook.com>
Signed-off-by: BubbleCal <bubble-cal@outlook.com>
@BubbleCal
BubbleCal marked this pull request as ready for review May 2, 2025 10:00
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 85.71429% with 2 lines in your changes missing coverage. Please review.

Project coverage is 78.58%. Comparing base (670a816) to head (cb9c492).

Files with missing lines Patch % Lines
rust/lance/src/io/exec/knn.rs 85.71% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3769      +/-   ##
==========================================
- Coverage   78.59%   78.58%   -0.01%     
==========================================
  Files         272      272              
  Lines      102094   102085       -9     
  Branches   102094   102085       -9     
==========================================
- Hits        80238    80222      -16     
- Misses      18683    18694      +11     
+ Partials     3173     3169       -4     
Flag Coverage Δ
unittests 78.58% <85.71%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@westonpace westonpace left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Very good find!

@BubbleCal
BubbleCal merged commit a4253b7 into lance-format:main May 2, 2025
BubbleCal added a commit that referenced this pull request Aug 14, 2025
We've seen some bugs that could be reproduced with some operations in
some order, so adding this for finding out such bugs:
- #4108
- #4156
- #3769

These weekly tests would enumerate all permutations of write operations,
and run all of them, to make sure we can cover all cases. And we would
run these tests on latest 2 stable version and main, to find out any
regression and compatibility issues

This would take a long time, so we run these weekly, rather than on each
commit.

and we've seen some bugs which happened on only large dataset, we can
run some tests over a large dataset weekly, but not adding these for now

---------

Signed-off-by: BubbleCal <bubble-cal@outlook.com>
BubbleCal added a commit that referenced this pull request Aug 12, 2026
## What changed?

Extend the cross-version index maintenance-sequence search with a
bounded `IVF_PQ` + `BTREE` prefilter scenario.

- cover every single maintenance operation and ordered operation pair
across valid writer/reader splits
- add five curated deeper lifecycles, including the two-vector-delta
plus scalar-unindexed-row state from #3769
- compare scalar-index results with a full scan
- compare indexed ANN prefilter results with exact index-free KNN and
require recall@10 >= 0.5
- cap vector search at four shards and avoid exponential sequence growth
- include `IVF_PQ` in manual compat-pair `all`

## Why is this needed?

The existing sequence search covers scalar and FTS indexes but not
vector/scalar prefilter interactions. That is the main remaining unique
signal in the legacy recurring test. This adds deterministic, isolated
cross-version coverage with correctness oracles before that recurring
matrix is removed.

## Validation

- deterministic bounded-generator test
- current runtime: all 103 max-length-5 cases passed across four shards
in 10.51s
- Pylance 9.0.1 writer to 10.0.0 reader:
  - all 103 cases passed in one shard
  - the default four-shard run passed in 9.07s after environment setup
- existing `BTREE` sequence smoke passed
- `uv run make lint`
- workflow YAML parse
- `git diff --check`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-python Python bindings bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants