Skip to content

Conversation

@yanghua
Copy link
Collaborator

@yanghua yanghua commented Jan 21, 2026

No description provided.

@github-actions github-actions bot added the enhancement New feature or request label Jan 21, 2026
@yanghua yanghua marked this pull request as draft January 21, 2026 11:18
@github-actions
Copy link
Contributor

PR Review: feat: introduce RowIdSet and RowIdMask

Summary

This PR adds RowIdSet and RowIdMask types that mirror the existing RowAddrTreeMap and RowAddrMask but operate on stable 64-bit row IDs using RoaringTreemap directly. The implementation is straightforward and follows established patterns in the codebase.

P0/P1 Issues

P1: Missing tests - The new RowIdSet and RowIdMask types have no test coverage. The existing file has comprehensive tests for RowAddrMask and RowAddrTreeMap (lines 927-1123), but no corresponding tests for the new types. Given the project requirement that "We do not merge code without tests", please add tests covering:

  • RowIdSet: new(), iter(), union(), difference(), and RowSetOps trait methods
  • RowIdMask: all_rows(), allow_nothing(), from_allowed(), from_block(), selected(), selected_indices(), also_block(), also_allow(), max_len(), iter_ids()
  • Edge cases for from_sorted_iter() with non-sorted input

P1: Missing public exports - The new types are not re-exported from the module's public API. Users will need to import them, but they're not currently accessible outside the module. Check if they need to be added to the module's pub use statements.

Notes

  • The code correctly follows the existing pattern established by RowAddrMask/RowAddrTreeMap
  • Good use of RoaringTreemap for 64-bit row IDs (vs RoaringBitmap for 32-bit)
  • The #[track_caller] annotation on from_sorted_iter is appropriate for debugging

@codecov
Copy link

codecov bot commented Jan 21, 2026

Codecov Report

❌ Patch coverage is 0% with 106 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
rust/lance-core/src/utils/mask.rs 0.00% 106 Missing ⚠️

📢 Thoughts on this report? Let us know!

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

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant