Skip to content

test(frag-reuse): cover the compact open path - #9295

Draft
amunra wants to merge 1 commit into
lance-format:mainfrom
rerun-io:upstream/frag-reuse-open-path-tests
Draft

amunra wants to merge 1 commit into
lance-format:mainfrom
rerun-io:upstream/frag-reuse-open-path-tests

Conversation

@amunra

@amunra amunra commented Sep 16, 2026

Copy link
Copy Markdown

Background

We had independently implemented the equivalent of #8887 on our own branch: opening the fragment-reuse index into a compact per-fragment representation instead of expanding every reuse version into a per-row map. Same reason, the expansion dominates memory on a long reuse chain, and much the same shape, positional mapping of rewritten rows onto new fragments with rank lookups over roaring bitmaps.

By the time we came to propose it, #8887 was already merged, so none of that implementation is needed here. What our branch also carried was the tests written while getting the mapping right, and those cover behaviours the current suite does not. This PR is only those tests, adapted to the code on main.

What it adds

The existing tests in rust/lance-table/src/system_index/frag_reuse.rs cover a one-to-many split, a two-link chain, corrupt bytes and three layout validations. These fill in the gaps:

  • Rank across container types. A 200k-row fragment with holes at 7, 65535, 65536 and 131072, so the rewritten offsets land in run and bitmap containers rather than the small array containers every existing test produces. This exercises all three RankedOffsets variants at the sizes real payloads reach, where a rank error returns a wrong row address rather than failing.
  • Agreement with the direct transposition. transpose_row_ids_from_digest is still the compaction-side path under IndexRemapMode::Direct. One test in the lance crate checks the compact index against it on every address of an ascending-fragment payload, the ordering compaction actually produces. Nothing in the tree asserted that the two agree.
  • Row-count validation, both directions. Too few and too many new rows; the only coverage of that check.
  • Moved, deleted and untouched rows in one group, including that an output address is not remapped again and that an offset past a fragment's recorded row count passes through unchanged.
  • Positional pairing in the order the old fragments are listed rather than by address, since the builder assigns positions by walking that list rather than sorting it.
  • A task whose output rolls into five new fragments, so the destination search lands strictly inside the list.
  • An emptied fragment mid-group not shifting later positions; a 32-link chain; a deletion mid-chain being terminal; groups within one version keeping independent positions; empty payloads.
  • A truncated changed-row-addrs body, added as a second case to the existing corrupt-bytes test.

Notes

Test-only; no production code changes. The tests build FragReuseIndexDetails directly with the module's existing helpers plus group and details constructors, and run synchronously.


Tracking: Ported in part from rerun-io#59.

@github-actions github-actions Bot added the chore label Sep 16, 2026
`CompactFragReuseIndex::try_new` builds the index every dataset open loads,
and its lookups decide whether an index row is found, moved or gone after a
compaction. The existing tests cover a one-to-many split, a two-link chain,
corrupt bytes and three layout validations. Add unit tests for the behaviours
between those: moved, deleted and untouched rows in one group, including that
an output address is not remapped again and an offset past a fragment's
recorded row count passes through; positional pairing in the order the old
fragments are listed rather than by address; offsets in run and
bitmap containers on a 200k-row fragment; a task rolling into five new
fragments; an emptied fragment mid-group not shifting later positions; a
32-link chain; a deletion mid-chain being terminal; independent positions for
groups sharing a version; empty payloads; and both directions of the
rewritten-rows versus new-rows count check. The corrupt-bytes test gains a
truncated-body case.

In the `lance` crate, check the compact index against
`transpose_row_ids_from_digest`, the per-row transposition compaction uses
under `IndexRemapMode::Direct`, on every address of an ascending-fragment
payload: that is the case compaction produces, and the two must agree on it.
@amunra
amunra force-pushed the upstream/frag-reuse-open-path-tests branch from 48f4a8b to 0d97be1 Compare September 17, 2026 08:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant