Skip to content

ci: generate multiple fragments in forward compatibility tests and doc RowIdTreeMap.serialize…#5105

Merged
wjones127 merged 9 commits intolance-format:mainfrom
ddupg:feat/forward-compat-multi-frags
Nov 5, 2025
Merged

ci: generate multiple fragments in forward compatibility tests and doc RowIdTreeMap.serialize…#5105
wjones127 merged 9 commits intolance-format:mainfrom
ddupg:feat/forward-compat-multi-frags

Conversation

@ddupg
Copy link
Copy Markdown
Contributor

@ddupg ddupg commented Oct 30, 2025

…_into stable

Closes: #5093

This PR made the following changes:

  1. doc RowIdTreeMap.serialize_info as stable, which was used in bitmap index
  2. make sure that the forward compatibility tests datasets have multiple fragments to guarantee older versions Lance can correctly load bitmap index.
  3. make sure backward compatibility test test_old_btree_bitmap_indices use bitmap index
  4. fix fails introduced by fix: forward compatibility of FTS index #4906

@github-actions github-actions Bot added enhancement New feature or request python labels Oct 30, 2025
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Oct 30, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.82%. Comparing base (e2a9079) to head (1eb812d).
⚠️ Report is 8 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5105      +/-   ##
==========================================
+ Coverage   81.77%   81.82%   +0.04%     
==========================================
  Files         341      341              
  Lines      140933   140540     -393     
  Branches   140933   140540     -393     
==========================================
- Hits       115255   114997     -258     
+ Misses      21862    21735     -127     
+ Partials     3816     3808       -8     
Flag Coverage Δ
unittests 81.82% <ø> (+0.04%) ⬆️

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.

@ddupg ddupg force-pushed the feat/forward-compat-multi-frags branch 4 times, most recently from 346ccc3 to 871eb82 Compare October 30, 2025 11:50
@ddupg ddupg changed the title feat: add backward compatibility tests and doc RowIdTreeMap.serialize… ci: add backward compatibility tests and doc RowIdTreeMap.serialize… Oct 30, 2025
@github-actions github-actions Bot added the ci Github Action or Test issues label Oct 30, 2025
@ddupg ddupg force-pushed the feat/forward-compat-multi-frags branch 7 times, most recently from 5b6e676 to b9eeb33 Compare October 31, 2025 08:30
@ddupg ddupg marked this pull request as ready for review October 31, 2025 09:04
@ddupg ddupg changed the title ci: add backward compatibility tests and doc RowIdTreeMap.serialize… ci: generate multiple fragments in forward compatibility tests and doc RowIdTreeMap.serialize… Nov 3, 2025
ddupg added 3 commits November 3, 2025 17:19
…nto stable

Change-Id: Id60677e4214171588d7a50d0cb7fe10d1a7483f2
Change-Id: I8e098ce629f29b9e1afdd1cdab9b35c700c5d063
Change-Id: I882ee76b94fac0e30647204c41dd25cac664c090
@ddupg ddupg force-pushed the feat/forward-compat-multi-frags branch from a3db46a to a69bce9 Compare November 3, 2025 09:21
Change-Id: If7e0bd74e7d4b2c9cca6c9b90dae3d76c5a8cd88
@ddupg ddupg force-pushed the feat/forward-compat-multi-frags branch from 23bf222 to d67bfe0 Compare November 3, 2025 09:38
@wjones127 wjones127 self-assigned this Nov 3, 2025
@ddupg ddupg force-pushed the feat/forward-compat-multi-frags branch 2 times, most recently from ae52700 to 4838c10 Compare November 4, 2025 02:11
Change-Id: I27ce298f8c7ce280c75309ff390e86e2a5112269
@ddupg ddupg force-pushed the feat/forward-compat-multi-frags branch from 4838c10 to 00ff7da Compare November 4, 2025 03:23
@ddupg
Copy link
Copy Markdown
Contributor Author

ddupg commented Nov 4, 2025

Forward compatibility tests test_write_scalar_index and test_write_fts introduced by #5116 failed after generated dataset contained multiple fragments. When ds.optimize.compact_files(), trigger indexes remap, different indexes fail for different reasons:

OSError: Invalid user input: Cannot write batch.  The batch was missing the column `row_ids`, /Users/runner/work/lance/lance/rust/lance-file/src/v2/writer.rs:360:35
  • zonemap and bloomfilter: was introduced in 0.36.0, older versions cannot parse it.

I've also attempted to fix above problems here. Please @jackye1995 help review this.

@ddupg
Copy link
Copy Markdown
Contributor Author

ddupg commented Nov 4, 2025

The failed tests index::vector::ivf::v2::tests::* appears unrelated to this PR.

@ddupg
Copy link
Copy Markdown
Contributor Author

ddupg commented Nov 5, 2025

@wjones127 @westonpace could please help review this?

Copy link
Copy Markdown
Contributor

@wjones127 wjones127 left a comment

Choose a reason for hiding this comment

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

Looks good. I have some minor comments I can change before I merge.

Comment thread python/python/tests/forward_compat/test_compat.py Outdated
Comment thread rust/lance-core/src/utils/mask.rs Outdated
Comment on lines +232 to +235
# Dataset::load_manifest does not do retain_supported_indices
# so this can only work with no cache
session = lance.Session(index_cache_size_bytes=0, metadata_cache_size_bytes=0)
ds = lance.dataset(tmp_path, session=session)
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.

🤦 We should really fix this bug.

@wjones127 wjones127 merged commit 82b4671 into lance-format:main Nov 5, 2025
27 checks passed
jackye1995 pushed a commit to jackye1995/lance that referenced this pull request Jan 21, 2026
…c RowIdTreeMap.serialize… (lance-format#5105)

…_into stable

Closes: lance-format#5093

This PR made the following changes:
1. doc RowIdTreeMap.serialize_info as stable, which was used in bitmap
index
2. make sure that the forward compatibility tests datasets have multiple
fragments to guarantee older versions Lance can correctly load bitmap
index.
3. make sure backward compatibility test `test_old_btree_bitmap_indices`
use bitmap index
4. fix fails introduced by lance-format#4906

---------

Co-authored-by: Will Jones <willjones127@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci Github Action or Test issues enhancement New feature or request python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The serialization method for BitmapIndex is not stable

3 participants