Skip to content

fix: don't generate nulls for Decimal128 and Decimal256 when field is non-nullable and have non-zero null_density#9046

Merged
alamb merged 2 commits intoapache:mainfrom
rluvaton:fix-nullability
Dec 30, 2025
Merged

fix: don't generate nulls for Decimal128 and Decimal256 when field is non-nullable and have non-zero null_density#9046
alamb merged 2 commits intoapache:mainfrom
rluvaton:fix-nullability

Conversation

@rluvaton
Copy link
Member

Which issue does this PR close?

N/A

Rationale for this change

if decimal field is non nullable and have null density we should not have nulls in the genrated array

What changes are included in this PR?

Override null_density for non nested and non dictionary to avoid future problems like this
added assertion and test

Are these changes tested?

yes

Are there any user-facing changes?

working generate for Decimal128 and Decimal256

… non-nullable and have non-zero null_density
@github-actions github-actions bot added the arrow Changes to the arrow crate label Dec 25, 2025
// For dictionary it handle the nullability internally
if !field.data_type().is_nested() && !matches!(field.data_type(), Dictionary(_, _)) {
// Override null density with 0.0 if the array is non-nullable
null_density = match field.is_nullable() {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

overriding this instead of making the decimal use primitive_null_density to avoid future problems like this

Copy link
Contributor

@Jefffrey Jefffrey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I notice we're missing the arms for Decimal32 and Decimal64 here too

@alamb
Copy link
Contributor

alamb commented Dec 30, 2025

I notice we're missing the arms for Decimal32 and Decimal64 here too

Maybe we can fix this as a follow on PR

@alamb
Copy link
Contributor

alamb commented Dec 30, 2025

Thank you @Jefffrey and @rluvaton

@alamb alamb merged commit 9b16fb3 into apache:main Dec 30, 2025
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

arrow Changes to the arrow crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants