Skip to content

refactor: move mask code into new crate - #6879

Merged
westonpace merged 2 commits into
lance-format:mainfrom
westonpace:refactor-index-expr-result-2mask
May 21, 2026
Merged

refactor: move mask code into new crate#6879
westonpace merged 2 commits into
lance-format:mainfrom
westonpace:refactor-index-expr-result-2mask

Conversation

@westonpace

Copy link
Copy Markdown
Member

Some of the mask code was in lance-core and some was in lance-index. It was difficult to benchmark the stuff in lance-index since it's a bit dependency heavy. This moves all the mask code into a lance-select crate. It also adds a micro benchmark for some of the lance ops (I plan on optimizing these in a follow-on PR)

This PR was developed with assistance from Claude Code.

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@westonpace

Copy link
Copy Markdown
Member Author

Baseline benchmark numbers:


Baseline numbers (criterion --measurement-time 1, single-threaded,
median of 10 samples):

  Op   Variant pair              N=10K   N=100K  N=1M    N=10M
  not  Exact / AtMost / AtLeast  ~13 ns  ~12 ns  ~10 ns  ~10 ns
  and  Exact_Exact               251 ns  319 ns  1.23 µs 8.28 µs
  and  AtMost_AtMost             257 ns  305 ns  1.11 µs 8.39 µs
  and  AtLeast_AtLeast           234 ns  322 ns  1.11 µs 8.24 µs
  and  Exact_AtMost              245 ns  320 ns  1.06 µs 8.44 µs
  and  Exact_AtLeast (degenerate) 182 ns 239 ns  510 ns  2.17 µs
  and  AtMost_AtLeast (degen.)   178 ns  245 ns  528 ns  2.23 µs
  or   (mirror of `and`)         ~210 ns ~285 ns ~880 ns ~7.0 µs

@westonpace
westonpace force-pushed the refactor-index-expr-result-2mask branch from 9caf29a to 9ee946a Compare May 20, 2026 22:46
@codecov

codecov Bot commented May 20, 2026

Copy link
Copy Markdown

@wjones127 wjones127 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.

I think there's one important change missing. Also should make a note that the first time we publish there might be extra steps we need to take. Can handle that next week with 7.0.0 release.

Comment thread Cargo.toml Outdated
lance-namespace-impls = { version = "=7.0.0-beta.17", path = "./rust/lance-namespace-impls" }
lance-namespace-datafusion = { version = "=7.0.0-beta.9", path = "./rust/lance-namespace-datafusion" }
lance-namespace-reqwest-client = "0.7.5"
lance-select = { version = "=7.0.0-beta.17", path = "./rust/lance-select" }

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.

issue(blocking): we also need to update the bumpfile, right?

https://github.com/lance-format/lance/blob/main/.bumpversion.toml

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Updated.

@westonpace
westonpace force-pushed the refactor-index-expr-result-2mask branch from 9ee946a to efce4e5 Compare May 21, 2026 19:40
@westonpace
westonpace requested a review from wjones127 May 21, 2026 19:41
westonpace and others added 2 commits May 21, 2026 20:19
…ce-select

Moves the row-address mask types (`RowAddrMask`, `NullableRowAddrMask`,
`RowAddrTreeMap`, `RowIdMask`, `RowIdSet`, `NullableRowAddrSet`, the
`RowSetOps` trait, and the `bitmap_to_ranges` / `ranges_to_bitmap`
helpers) plus the certainty-tagged result types (`IndexExprResult` and
`NullableIndexExprResult`) — including their `Not` / `BitAnd` / `BitOr`
algebra and the `drop_nulls` collapser — into a new `lance-select`
crate. This sits below `lance-index` in the dependency graph (deps:
arrow-array, arrow-buffer, byteorder, deepsize, itertools, roaring,
lance-core) and gives consumers — benchmarks, prefilter / read-planner
code, and future per-range filter-pushdown work — a path to depend on
the mask substrate without pulling in `lance-index`.

Two new things, beyond the move:

* `impl Not for NullableIndexExprResult` — extracted from the inline
  match in `ScalarIndexExpr::evaluate_impl`. The body is identical
  (`Exact(!m)`, `AtMost↔AtLeast(!m)`); the evaluator now just calls
  `!result`.

* `index_expr_result_from_parts` / `serialize_index_expr_result` free
  functions on `lance-index::scalar::expression`, replacing the old
  inherent methods `IndexExprResult::{from_parts, serialize_to_arrow}`.
  The arrow wire format and its `INDEX_EXPR_RESULT_SCHEMA` constant
  stay in `lance-index` so `lance-select` doesn't have to take an
  arrow-schema dependency.

`From<SearchResult> for NullableIndexExprResult` likewise stays in
`lance-index` (where `SearchResult` is defined).

All 39 import sites for `lance_core::utils::mask::*` were updated to
`lance_select::*`. The `lance` / `lance-index` / `lance-table` crates
gained a `lance-select.workspace = true` dependency.

Verified:

  * lance-select unit tests: 97 passed (the mask + nullable suite)
  * lance-index --lib: 302 passed
  * lance-table --lib: 104 passed
  * lance --lib: 1672 passed
  * `cargo clippy --workspace --tests -- -D warnings`: clean

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Captures a baseline of the current 3-variant Exact/AtMost/AtLeast
algebra ahead of converting `IndexExprResult` /
`NullableIndexExprResult` to a 2-mask `{lower, upper}` representation.

Each variant is built from a `NullableRowAddrMask::AllowList` covering
a single contiguous run in one fragment (the shape produced by
zone-map / bloom-filter IsNull searches and by `mask_to_offset_ranges`
on a contiguous segment). Sweep N over 10K..10M to expose mask-size
scaling.

Baseline numbers (criterion --measurement-time 1, single-threaded,
median of 10 samples):

  Op   Variant pair              N=10K   N=100K  N=1M    N=10M
  not  Exact / AtMost / AtLeast  ~13 ns  ~12 ns  ~10 ns  ~10 ns
  and  Exact_Exact               251 ns  319 ns  1.23 µs 8.28 µs
  and  AtMost_AtMost             257 ns  305 ns  1.11 µs 8.39 µs
  and  AtLeast_AtLeast           234 ns  322 ns  1.11 µs 8.24 µs
  and  Exact_AtMost              245 ns  320 ns  1.06 µs 8.44 µs
  and  Exact_AtLeast (degenerate) 182 ns 239 ns  510 ns  2.17 µs
  and  AtMost_AtLeast (degen.)   178 ns  245 ns  528 ns  2.23 µs
  or   (mirror of `and`)         ~210 ns ~285 ns ~880 ns ~7.0 µs

Reading the numbers:

* NOT is constant-time across sizes (~10 ns) because
  `NullableRowAddrMask::Not` flips Allow↔Block without touching the
  underlying sets.

* Same-variant AND/OR do exactly one mask op (`lhs & rhs` or
  `lhs | rhs`) and scale with mask size as expected.

* Cross-variant AND/OR involving `AtLeast` and either `Exact` or
  `AtMost` are ~3-4x faster at 10M because the current algebra drops
  one side without doing a mask op: e.g. `AtMost(m) & AtLeast(_) →
  AtMost(m)`. This is the precision we're about to recover when the
  2-mask form lifts both endpoints elementwise — same-variant cost
  ~2x (two mask ops instead of one), cross-variant cost ~same as
  same-variant (no degeneracy).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@westonpace
westonpace force-pushed the refactor-index-expr-result-2mask branch from efce4e5 to 0f36f79 Compare May 21, 2026 20:19
@github-actions github-actions Bot added A-python Python bindings A-java Java bindings + JNI labels May 21, 2026
@westonpace
westonpace merged commit 52c6ac3 into lance-format:main May 21, 2026
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-java Java bindings + JNI A-python Python bindings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants