You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "liamzwbao (via GitHub)" <gi...@apache.org> on 2025/09/27 18:11:26 UTC
[PR] [Variant] Support strict casting for Decimals [arrow-rs]
liamzwbao opened a new pull request, #8483:
URL: https://github.com/apache/arrow-rs/pull/8483
# Which issue does this PR close?
- Closes #8303.
# Rationale for this change
# What changes are included in this PR?
Support strict casting for Decimal types, also fix the non-strict cast for temporal types where the results should be `Variant:Null` (Overflow) instead `None` (missing value)
# Are these changes tested?
Yes
# Are there any user-facing changes?
Yes, decimal casting now supports strict and non-strict modes
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Variant] Support strict casting for Decimals [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #8483:
URL: https://github.com/apache/arrow-rs/pull/8483
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Variant] Support strict casting for Decimals [arrow-rs]
Posted by "liamzwbao (via GitHub)" <gi...@apache.org>.
liamzwbao commented on code in PR #8483:
URL: https://github.com/apache/arrow-rs/pull/8483#discussion_r2384267804
##########
parquet-variant-compute/src/cast_to_variant.rs:
##########
@@ -2108,64 +2260,24 @@ mod tests {
}
fn run_test(values: ArrayRef, expected: Vec<Option<Variant>>) {
- run_test_with_options(values, expected, true);
- }
-
- fn run_test_non_strict(values: ArrayRef, expected: Vec<Option<Variant>>) {
- run_test_with_options(values, expected, false);
- }
-
- #[test]
Review Comment:
Moved above to group related tests together
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Variant] Support strict casting for Decimals [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #8483:
URL: https://github.com/apache/arrow-rs/pull/8483#issuecomment-3348730639
@scovich since you filed https://github.com/apache/arrow-rs/issues/8303 can you please help review this PR?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Variant] Support strict casting for Decimals [arrow-rs]
Posted by "scovich (via GitHub)" <gi...@apache.org>.
scovich commented on code in PR #8483:
URL: https://github.com/apache/arrow-rs/pull/8483#discussion_r2389340652
##########
parquet-variant-compute/src/arrow_to_variant.rs:
##########
@@ -400,39 +402,51 @@ define_row_builder!(
define_row_builder!(
struct Decimal32ArrowToVariantBuilder<'a> {
+ options: &'a CastOptions,
scale: i8,
},
|array| -> arrow::array::Decimal32Array { array.as_primitive() },
- |value| decimal_to_variant_decimal!(value, scale, i32, VariantDecimal4)
+ |value| -> Option<_> {
+ decimal_to_variant_decimal!(value, scale, i32, VariantDecimal4).map(Variant::from)
Review Comment:
I don't think we need these `Variant::from` calls?
The builder `append_value` should take `impl Into<Variant>`
and we already define the needed `impl From<DecimalXX> for Variant`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Variant] Support strict casting for Decimals [arrow-rs]
Posted by "liamzwbao (via GitHub)" <gi...@apache.org>.
liamzwbao commented on code in PR #8483:
URL: https://github.com/apache/arrow-rs/pull/8483#discussion_r2389408830
##########
parquet-variant-compute/src/arrow_to_variant.rs:
##########
@@ -400,39 +402,51 @@ define_row_builder!(
define_row_builder!(
struct Decimal32ArrowToVariantBuilder<'a> {
+ options: &'a CastOptions,
scale: i8,
},
|array| -> arrow::array::Decimal32Array { array.as_primitive() },
- |value| decimal_to_variant_decimal!(value, scale, i32, VariantDecimal4)
+ |value| -> Option<_> {
+ decimal_to_variant_decimal!(value, scale, i32, VariantDecimal4).map(Variant::from)
Review Comment:
Thanks for pointing it out
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org