Skip to content

fix(dataset): reject a zero add_columns batch size instead of panicking - #9233

Merged
wjones127 merged 1 commit into
lance-format:mainfrom
zhangyue19921010:add-columns-reject-zero-batch-size
Sep 17, 2026
Merged

wjones127 merged 1 commit into
lance-format:mainfrom
zhangyue19921010:add-columns-reject-zero-batch-size

Conversation

@zhangyue19921010

Copy link
Copy Markdown
Collaborator

A batch_size of Some(0) passed to Dataset::add_columns or FileFragment::add_columns reached FragmentReader::read_all(0), whose system-column read path calls step_by(0) and panics. Through the Java bindings that panic crosses an extern "system" entrypoint and aborts the JVM instead of raising an input error.

Reject it in Updater::try_new, which every add_columns transform and binding goes through, with an InvalidInput error.

A batch_size of Some(0) passed to Dataset::add_columns or
FileFragment::add_columns reached FragmentReader::read_all(0), whose
system-column read path calls step_by(0) and panics. Through the Java
bindings that panic crosses an extern "system" entrypoint and aborts the JVM
instead of raising an input error.

Reject it in Updater::try_new, which every add_columns transform and binding
goes through, with an InvalidInput error.

@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.

The zero batch size is now rejected as InvalidInput before fragment reads or writes, so Rust and binding callers cannot reach the step_by(0) panic. The shared updater guard covers the dataset and fragment data-rewriting paths, and the regression test exercises the public dataset path.

@lance-gatekeeper lance-gatekeeper Bot added the K-approved Latest Gatekeeper recommendation permits acceptance. label Sep 15, 2026
@wjones127
wjones127 merged commit 4e61b78 into lance-format:main Sep 17, 2026
57 of 62 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 K-approved Latest Gatekeeper recommendation permits acceptance.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants