Repository navigation
Support fill_null and zip for more dtypes - #11415
Draft
robert3005 wants to merge 1 commit into
Draft
robert3005 wants to merge 1 commit into
robert3005 wants to merge 1 commit into
Conversation
fill_null failed at execution for all canonical arrays except Bool, Primitive, Decimal and VarBinView when the input had some nulls. zip had no native kernel for Decimal, Extension, FixedSizeList, Map and Null, and it panicked for Union and Variant. fill_null now fills Extension through its storage, Struct through its fields, and List, FixedSizeList and Map through a zip with a constant fill. zip has kernels for Decimal (shared with Primitive through the fixed-width code), Extension, FixedSizeList, Map and Null. Union and Variant return an error instead of a panic. Signed-off-by: Robert Kruszewski <github@robertk.io>
12 of 49 tasks
This branch has not been deployed
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.
Summary
fill_nullandzipdid not have uniform support across dtypes.fill_nullaccepted all dtypes at bind time, but failed at execution withNo FillNullKernel for canonical arrayfor all canonical arrays except Bool, Primitive, Decimal and VarBinView when the input had valid rows and null rows.ziphad no native kernel for Decimal, Extension, FixedSizeList, Map and Null. These used the builder fallback, which does one slice and one append for each mask run.zippanicked for Union and Variant, because these dtypes have no builder.Changes
fill_null:fill_nullwork for temporal columns.zipof the input with a constant fill over the input validity.FillNull::supports_dtypenow accepts all dtypes except Null and dtypes that contain a Union or a Variant, so theCASE WHENrewrite does not make afill_nullthat fails later.zip:zip_fixed_widthinarrays/fixed_width, shared with Primitive. The blend code moved out of the Primitive kernel without a change in behavior. Decimals with different storage widths use the builder.Not in this PR:
fill_nullkernels for Struct and Extension are called only from the canonical dispatch. They are not registered as execute-parent kernels.Tests: new tests next to each kernel, and the
fill_nulltest that recorded the old failure is now anrstestover all supported dtypes with a nullable and a non-nullable fill value.Checks run:
cargo clippy -p vortex-array --all-targets --all-features -- -D warningscargo +nightly-2026-09-10 fmt -p vortex-array --checkcargo nextest run -p vortex-array --all-features(5920 passed, 1 skipped)Checks not run: doctests, benchmarks, and the checks of other crates.
🤖 Generated with Claude Code