refactor: rename RowIdSelection to RowAddrSelection

[yanghua]

As a step to distinguish the row ID and the row address, we should rename RowIdSelection to RowAddrSelection. To show its effect more clearly.

[jackye1995]

Try reopen PR to fix CI

[wjones127]

Why is this appropriate? The docs above say that it can be row ids. Do stable row ids not use this structure at all?

lance/rust/lance-core/src/utils/mask.rs

Lines 365 to 368 in cb9e613

	/// These row ids may either be stable-style (where they can be an incrementing
	/// u64 sequence) or address style, where they are a fragment id and a row offset.
	/// When address style, this supports setting entire fragments as selected,
	/// without needing to enumerate all the ids in the fragment.

[yanghua]

Will change related comments when doing RowIdTreeMap -> RowAddrMap in #5266 .

[jackye1995]

I think for this one we should use the term row ID? Because this is more used to accommodate stable row ID, and for row address technically directly use RoaringTreemap?

[yanghua]

It seems no matter RowIdSelection nor RowAddrSelection can cover the two semantics of row ID and row address. There is a full plan in my mind(sorry, did not write it down before):

It would be better to split the responsibility of the struct for distinguishing row ID(stable) and row address.

• for row address:

  • RowIdTreeMap -> RowAddrMap（BTreeMap<u32, RowAddrSelection>）
  • RowIdSelection → RowAddrSelection（Full | Partial(RoaringBitmap)）
  • SearchResult(scalar.rs) → RowAddrSearchResult (Exact(RowAddrMap) | AtMost(RowAddrMap))
  • RowIdMask → RowAddrMask

• for (stable) row id:

  • add rowid::RowIdSet (RoaringTreemap，64-bit set)
  • add rowid::RowIdMask
  • add RowIdResolver(row_id -> row_addr)

• for both:

  • add RowSetOps for RowIdSet and RowAddrMap

WDYT? cc @wjones127

[wjones127]

That's okay. Could you put this design in an issue?

Remember that you're collaborating with other developers and they need to be able to follow what you are doing.

[yanghua]

Thanks for the suggestion. Have filed a ticket: #5326

[codecov]

Codecov Report

❌ Patch coverage is 63.15789% with 21 lines in your changes missing coverage. Please review.

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

📢 Thoughts on this report? Let us know!

[jackye1995]

overall looks good to me, 2 minor comments