Conversation
a1aebb0 to
a6f385c
Compare
f14f1c0 to
8606a22
Compare
|
Important Format specification voteThis PR modifies the Lance format specification, so it requires 3 binding +1 votes from PMC members (excluding the proposer), at least one of them on the latest commit, and a minimum 72-hour voting period, weekends excluded, before it can merge. Vote by approving this PR (+1) or requesting changes (−1, a veto). See the voting process. Approvals carry over across pushes, so a rebase or a typo fix does not send everyone back to re-vote. Whoever approves the latest commit is vouching that nothing substantive has changed since the earlier approvals; if something has, ask for fresh votes. Status: ❌ Blocked — 0 of 3 required approvals
Updated automatically by the format-spec vote gate, which re-checks every 15 minutes — just voted? Re-check now (press Run workflow; leave the input blank to re-check every open format PR). A PMC member may apply the |
8606a22 to
38f8674
Compare
|
this one has no format change, wrong label |
Four multivector floor tests shared a body and three asserted the same outcome, and two partition-band tests differed only in a row count.
There was a problem hiding this comment.
The new commit consolidates repeated quantizer-floor and partition-cap tests without dropping their case coverage or changing production behavior. The author-accepted risk remains: a deprecated explicit num_partitions can be lost across deferred creation, so IVF8 may later train as IVF1 and change index shape and performance. The deferral warning and persisted target_partition_size are the mitigation.
Append is the mode scheduled maintenance uses, and the only definition test in that mode covered the case where the table still cannot train.
There was a problem hiding this comment.
The new test covers the missing scheduled-maintenance path: append-mode optimization supersedes an empty definition and trains the available data once the vector floor is met. Production behavior is unchanged. The author-accepted risk remains: a deprecated explicit num_partitions can be lost across deferred creation, so IVF8 may later train as IVF1 and change index shape and performance. The deferral warning and persisted target_partition_size are the mitigation.
Optimize either appends to a trained index or trains one for the first time, and the two share only an entry point. Naming the predicate and lifting the training arm out leaves one early return where there were two.
There was a problem hiding this comment.
The extraction preserves the existing branch semantics: definition-only segments still train the whole column when unindexed data arrives and remain a no-op otherwise, while dormant and trained segments retain their prior rebuild and append paths. The author-accepted risk remains: a deprecated explicit num_partitions can be lost across deferred creation, so IVF8 may later train as IVF1 and change index shape and performance. The deferral warning and persisted target_partition_size are the mitigation.
Fourteen assertions spelled out the same bitmap check inline.
There was a problem hiding this comment.
The new test-only helper consolidates repeated empty-coverage assertions without changing their predicate, negation, or case coverage; production behavior is unchanged. The author-accepted risk remains: a deprecated explicit num_partitions can be lost across deferred creation, so IVF8 may later train as IVF1 and change index shape and performance. The deferral warning and persisted target_partition_size are the mitigation.
The problem
Creating a vector index on a table with too few rows fails, and so does
maintaining one on a table that later shrinks.
create_index(train=False)hits atodo!()for vector indices, so deferred training never worked for them eventhough it works for scalar and FTS.
The shrink half is worse: an IVF-PQ index whose table falls below the PQ minimum
makes
optimize_indices()fail outright, so maintenance stays stuck untilsomeone drops and recreates the index.
What this does
Implements #4034 for vector indices, keeping the empty-index contract #3940 set
for scalar and FTS.
data file, covering no rows. The next
optimize_indices()with enough datatrains it for real — in append mode as well as retrain, since append is what
scheduled maintenance runs. This applies on creation and when a table
shrinks, so maintenance is never blocked.
2^num_bits, so 256 at 8 bits). RQ and flatstorage build normally on a small table.
multivector row offers its whole list — 100 rows of 10 vectors give 1,000.
min(requested, vectors / sample_rate), and log it. A centroid built from ahandful of vectors prunes nothing and still costs compression accuracy.
A query skips an index that covers nothing and reads the table;
fast_searchreturns nothing; statistics report zero rows indexed.
Main pieces
should_train_index(create.rs) — train now, or record the settings andtrain later. A build given explicit fragments is exempt: the pieces of one
index must agree on their partition count.
count_trainable_vectors(vector.rs) — one definition of how many vectorsthere are, used by that decision and by the partition cap.
train_from_definition(append.rs) — trains an index that is still onlysettings, read from its metadata. It decides that before opening the index
file, because there is no file yet.
pq_quantizer_minimum_rowsandhas_vectors_to_trainare public, so a calleroutside the crate can ask these questions instead of restating the rules.
Tests
Creation, thresholds, partition sizing, queries and statistics against an index
that covers nothing, and the lifecycle through delete-all, update-all,
compaction, repeated append-mode optimize, and regrowth. Multivector and
null-vector columns are covered because they are what make rows the wrong thing
to count.
Each new test was checked by breaking the code under it and confirming it fails.
cargo nextest run -p lance --lib— 3871 pass, 3 skipped.cargo fmt --all --check,cargo clippy -p lance --lib --tests -D warningsandci/check_proto_comments.pyare clean.For review
num_partitionsbecomes a maximum, not an exact request, oncreate_indexas well asoptimize_indices(). Top-level indices part 2: empty vector indices #4034 asks for it; worthconfirming you want it on creation too. Narrowing it to "only reduce when the
build would otherwise fail" is one match arm in
supported_num_partitions.Fifteen existing tests asserted the count they requested on fixtures too small
to support it. Each test about index metadata was given the vectors its request
needs rather than having its assertion lowered; tests measuring search quality
kept their data, since changing data under a recall threshold is how those go
flaky.
The two 256s are separate. The training floor is the codebook size
(
2^num_bits). The per-partition number for the cap is the IVF sample rate,which is configurable and independent of the quantizer — a build declaring 64
samples per centroid supports four partitions on 300 vectors where the default
supports one. The cap applies only where a codebook makes thin partitions a bad
trade.
A requested
num_partitionsdoes not survive the wait.VectorIndexDetailscarries metric, quantizer and
target_partition_size, but not a partitioncount, so an index that trains later is sized from the data it then holds: a
table created as IVF-8 below the floor comes back at what the data supports,
not at 8. The same applies to a table that shrinks and regrows.
target_partition_sizeis the way to state a shape that outlives a deferral.Creating a definition with a count now logs a warning naming the count that
will not be kept, so the loss isn't silent.
Finishes the direction of #7211 (@burlacio), whose creation, statistics and
detection changes this supersedes; happy to rebase onto it instead if preferred.
Fixes: #4034