Skip to content

fix: choose cheapest blanks when restoring deleted rows - #8946

Open
chenyu-x wants to merge 1 commit into
lance-format:mainfrom
chenyu-x:fix-add-blank
Open

fix: choose cheapest blanks when restoring deleted rows#8946
chenyu-x wants to merge 1 commit into
lance-format:mainfrom
chenyu-x:fix-add-blank

Conversation

@chenyu-x

@chenyu-x chenyu-x commented Sep 2, 2026

Copy link
Copy Markdown

What

Adding or updating columns can fail with an Arrow offset overflow when a fragment contains many deleted rows and a variable-width column has a large value in its first live row.

The updater restores deleted physical rows with placeholder values. Previously every placeholder copied row zero, so large string, binary, or nested values were repeated for every deleted row.

Root cause

Every data file in a fragment must have the same physical row count. Before writing a new column, DeletionRestorer therefore inserts blank rows at deleted positions.

add_blanks implemented every blank with take(0). For variable-width columns this copied the payload of the first live row once per deleted row. A sufficiently large deletion run could grow the values buffer past the Arrow offset limit even though those placeholder values are never visible to readers.

V2 files also appended an unbounded trailing deletion run to the current output batch, allowing one update batch to become much larger than the configured batch size.

Fix

Choose the cheapest valid blank independently for each column:

  • use null when both the dataset write schema and Arrow field are nullable and the file version supports nulls;
  • use empty values for strings and binary arrays;
  • rebuild List, LargeList, and Map offsets without copying child values;
  • recursively optimize Struct and FixedSizeList children;
  • keep using take for fixed-width and dictionary arrays, where copying a value does not increase payload size.

Bound restored and deferred blank batches by the updater batch size. Retain a compact one-row source for layouts that still require take, so trailing deletion runs can be written in bounded chunks without keeping the original batch payload alive.

The write-schema nullability and exact storage-version policies are preserved. Zero batch sizes and unfinished deletion restoration now return descriptive errors.

Tests

Add coverage for:

  • heavy deletion runs with large binary payloads;
  • nullable and non-nullable variable-width columns;
  • file-version-specific null support;
  • dictionary, List, Map, Struct, and FixedSizeList columns;
  • bounded leading and trailing deletion runs;
  • dataset write-schema nullability;
  • add_columns and update_columns integration paths;
  • zero batch sizes and unaccounted deletions at finish.

Fixes #8890

## What

Adding or updating columns can fail with an Arrow offset overflow when
a fragment contains many deleted rows and a variable-width column has a
large value in its first live row.

The updater restores deleted physical rows with placeholder values.
Previously every placeholder copied row zero, so large string, binary,
or nested values were repeated for every deleted row.

## Root cause

Every data file in a fragment must have the same physical row count.
Before writing a new column, `DeletionRestorer` therefore inserts blank
rows at deleted positions.

`add_blanks` implemented every blank with `take(0)`. For variable-width
columns this copied the payload of the first live row once per deleted
row. A sufficiently large deletion run could grow the values buffer
past the Arrow offset limit even though those placeholder values are
never visible to readers.

V2 files also appended an unbounded trailing deletion run to the current
output batch, allowing one update batch to become much larger than the
configured batch size.

## Fix

Choose the cheapest valid blank independently for each column:

- use null when both the dataset write schema and Arrow field are
  nullable and the file version supports nulls;
- use empty values for strings and binary arrays;
- rebuild List, LargeList, and Map offsets without copying child values;
- recursively optimize Struct and FixedSizeList children;
- keep using `take` for fixed-width and dictionary arrays, where copying
  a value does not increase payload size.

Bound restored and deferred blank batches by the updater batch size.
Retain a compact one-row source for layouts that still require `take`,
so trailing deletion runs can be written in bounded chunks without
keeping the original batch payload alive.

The write-schema nullability and exact storage-version policies are
preserved. Zero batch sizes and unfinished deletion restoration now
return descriptive errors.

## Tests

Add coverage for:

- heavy deletion runs with large binary payloads;
- nullable and non-nullable variable-width columns;
- file-version-specific null support;
- dictionary, List, Map, Struct, and FixedSizeList columns;
- bounded leading and trailing deletion runs;
- dataset write-schema nullability;
- `add_columns` and `update_columns` integration paths;
- zero batch sizes and unaccounted deletions at finish.

Fixes lance-format#8890
@github-actions github-actions Bot added the bug Something isn't working label Sep 2, 2026
lance-gatekeeper[bot]

This comment was marked as outdated.

@lance-gatekeeper lance-gatekeeper Bot 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.

Gate recommendation: approve.

This removes deleted-row payload amplification with valid per-column blanks while preserving file-version and schema-nullability constraints. Restored rows are bounded by the configured batch size, with focused coverage across nested, dictionary, JSON/blob, legacy, add-column, and update-column paths.

@lance-gatekeeper lance-gatekeeper Bot added the K-approved Latest Gatekeeper recommendation permits acceptance. label Sep 2, 2026
@LuciferYang

Copy link
Copy Markdown
Contributor

cc @Xuanwo @wjones127 FYI

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working K-approved Latest Gatekeeper recommendation permits acceptance.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

update_columns on a heavily deleted fragment fails with "Failed to add blanks: Offset overflow error"

2 participants