fix: choose cheapest blanks when restoring deleted rows - #8946
Open
chenyu-x wants to merge 1 commit into
Open
Conversation
## 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
Contributor
There was a problem hiding this comment.
✅ 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.
Contributor
|
cc @Xuanwo @wjones127 FYI |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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,
DeletionRestorertherefore inserts blank rows at deleted positions.add_blanksimplemented every blank withtake(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:
takefor 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:
add_columnsandupdate_columnsintegration paths;Fixes #8890