Skip to content

fix(merge_insert): keep nested-field index correct on stable row id update#7410

Merged
jackye1995 merged 2 commits into
lance-format:mainfrom
jackye1995:jack/nested-index-update-corruption
Jun 23, 2026
Merged

fix(merge_insert): keep nested-field index correct on stable row id update#7410
jackye1995 merged 2 commits into
lance-format:mainfrom
jackye1995:jack/nested-index-update-corruption

Conversation

@jackye1995

Copy link
Copy Markdown
Contributor

With stable row ids, a full-row merge_insert moves updated rows to a new fragment while preserving their row ids. The RewriteRows index maintenance (register_pure_rewrite_rows_update_frags_in_indices) then extends a scalar index's fragment bitmap to the new fragment unless the index covers a modified field.

merge_insert built that modified-field set from the top-level schema().fields, so a nested-field index's leaf id was absent. The index was therefore wrongly extended over the rewritten fragment, which was then treated as indexed and never re-scanned — so updated rows were silently dropped from queries on that index (false negatives).

The fix builds the set with fields_pre_order() so nested leaf field ids are included. Flat schemas are unaffected (fields_pre_order() equals the top-level list when there are no children).

Adds two regression tests in dataset_merge_update.rs: a flat-index control and a nested-field guard that asserts the rewritten fragment is still data-scanned (via the fragments_scanned metric) so the updated value is found.

@codecov

codecov Bot commented Jun 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

jackye1995 added a commit that referenced this pull request Jun 23, 2026
…olumn (#7412)

The update API rejects nested column references (`set` errors on a `.`
in the column name), so a nested field can only be changed by setting
its whole **top-level struct column** (e.g. `SET s = named_struct('x',
…)`).

`commit_impl` built `fields_for_preserving_frag_bitmap` from the struct
column's own (parent) field id via `field_id(column_name)`, so an index
on a nested **child** field (a different leaf id) was absent from the
set. `register_pure_rewrite_rows_update_frags_in_indices` then wrongly
extended that child-field index over the rewritten fragment, which was
therefore treated as indexed and never re-scanned — so the updated rows
were silently dropped from queries on the index (false negatives).

The fix collects the full field subtree of each updated column, so a
struct-column update marks all descendant field ids as modified. Flat
columns are unaffected (no children).

This is the SQL `UPDATE` counterpart of the merge_insert fix in
#7410. Adds a regression test
(`test_update_struct_column_keeps_nested_index`): a BTree index on
`s.x`, an update of the struct column `s`, and an assertion that the
index's effective fragment bitmap is not extended over the rewritten
fragment so the updated value is still found.
…pdate

With stable row ids, a full-row merge_insert moves updated rows to a new
fragment while preserving their row ids. The RewriteRows index maintenance
then extends a scalar index's fragment bitmap to the new fragment unless the
index covers a modified field. merge_insert built that modified-field set from
the top-level `schema().fields`, so a nested-field index's leaf id was absent
and the index was wrongly extended over the rewritten fragment. The fragment
was then treated as indexed and never re-scanned, so updated rows were
silently dropped from queries on that index.

Build the set with `fields_pre_order()` so nested leaf field ids are included.
…pdate

A merge_insert full-row update treats the whole schema as modified, so every
column index drops the rewritten fragment from coverage -- including indexes on
columns that were not changed. Add a multi-index test documenting this, with a
TODO for a potential stable-row-id optimization that would preserve coverage for
indexes on unchanged columns.
@jackye1995
jackye1995 force-pushed the jack/nested-index-update-corruption branch from 839d945 to 4876cf5 Compare June 23, 2026 17:03
@jackye1995
jackye1995 merged commit aa56f8c into lance-format:main Jun 23, 2026
31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants