superserious-dev commented on code in PR #8081:
URL: https://github.com/apache/arrow-rs/pull/8081#discussion_r2260880087
##########
parquet-variant-compute/src/cast_to_variant.rs:
##########
@@ -110,14 +113,42 @@ pub fn cast_to_variant(input: &dyn Array) ->
Result<VariantArray, ArrowError> {
primitive_conversion!(UInt64Type, input, builder);
}
DataType::Float16 => {
- cast_conversion!(Float16Type, |v: f16| -> f32 { v.into() }, input,
builder);
+ cast_conversion!(
+ Float16Type,
+ as_primitive,
+ |v: f16| -> Result<f32, ArrowError> { Ok(v.into()) },
+ input,
+ builder
+ );
}
DataType::Float32 => {
primitive_conversion!(Float32Type, input, builder);
}
DataType::Float64 => {
primitive_conversion!(Float64Type, input, builder);
}
+ DataType::Date32 => {
+ cast_conversion!(
+ Date32Type,
+ as_primitive,
+ |v: i32| -> Result<NaiveDate, ArrowError> {
Ok(Date32Type::to_naive_date(v)) },
+ input,
+ builder
+ );
+ }
+ DataType::Date64 => {
+ cast_conversion!(
+ Date64Type,
+ as_primitive,
+ |v: i64| {
+
Date64Type::to_naive_date_opt(v).ok_or(ArrowError::CastError(format!(
Review Comment:
If we don't want to tweak the macro, we could call `expect` or `unwrap`
here.
##########
parquet-variant-compute/src/cast_to_variant.rs:
##########
@@ -40,17 +41,19 @@ macro_rules! primitive_conversion {
}};
}
-/// Convert the input array to a `VariantArray` row by row,
-/// transforming each element with `cast_fn`
+/// Convert the input array to a `VariantArray` row by row, using `method`
+/// to downcast the generic array to a specific array type and `cast_fn`
+/// to transform each element to a type compatible with Variant
+/// Note that `cast_fn` returns a result because it could fail.
macro_rules! cast_conversion {
- ($t:ty, $cast_fn:expr, $input:expr, $builder:expr) => {{
- let array = $input.as_primitive::<$t>();
+ ($t:ty, $method:ident, $cast_fn:expr, $input:expr, $builder:expr) => {{
Review Comment:
Carried over from Binary conversion PR
##########
parquet-variant-compute/src/cast_to_variant.rs:
##########
@@ -40,17 +41,19 @@ macro_rules! primitive_conversion {
}};
}
-/// Convert the input array to a `VariantArray` row by row,
-/// transforming each element with `cast_fn`
+/// Convert the input array to a `VariantArray` row by row, using `method`
+/// to downcast the generic array to a specific array type and `cast_fn`
+/// to transform each element to a type compatible with Variant
+/// Note that `cast_fn` returns a result because it could fail.
macro_rules! cast_conversion {
- ($t:ty, $cast_fn:expr, $input:expr, $builder:expr) => {{
- let array = $input.as_primitive::<$t>();
+ ($t:ty, $method:ident, $cast_fn:expr, $input:expr, $builder:expr) => {{
+ let array = $input.$method::<$t>();
for i in 0..array.len() {
if array.is_null(i) {
$builder.append_null();
continue;
}
- let cast_value = $cast_fn(array.value(i));
+ let cast_value = $cast_fn(array.value(i))?;
Review Comment:
New tweak to the macro here. Made `cast_fn` fallible so that an
ArrowError::CastError can be returned if something goes wring.
##########
parquet-variant-compute/src/cast_to_variant.rs:
##########
@@ -370,6 +402,45 @@ mod tests {
)
}
+ #[test]
+ fn test_cast_to_variant_date() {
+ // Date32Array
+ run_test(
+ Arc::new(Date32Array::from(vec![
+ Some(Date32Type::from_naive_date(NaiveDate::MIN)),
+ None,
+ Some(Date32Type::from_naive_date(
+ NaiveDate::from_ymd_opt(2025, 8, 1).unwrap(),
+ )),
+ Some(Date32Type::from_naive_date(NaiveDate::MAX)),
+ ])),
+ vec![
+ Some(Variant::Date(NaiveDate::MIN)),
+ None,
+ Some(Variant::Date(NaiveDate::from_ymd_opt(2025, 8,
1).unwrap())),
+ Some(Variant::Date(NaiveDate::MAX)),
+ ],
+ );
+
+ // Date64Array
+ run_test(
+ Arc::new(Date64Array::from(vec![
+ Some(Date64Type::from_naive_date(NaiveDate::MIN)),
+ None,
+ Some(Date64Type::from_naive_date(
+ NaiveDate::from_ymd_opt(2025, 8, 1).unwrap(),
+ )),
+ Some(Date64Type::from_naive_date(NaiveDate::MAX)),
+ ])),
+ vec![
+ Some(Variant::Date(NaiveDate::MIN)),
+ None,
+ Some(Variant::Date(NaiveDate::from_ymd_opt(2025, 8,
1).unwrap())),
+ Some(Variant::Date(NaiveDate::MAX)),
+ ],
+ );
Review Comment:
Could potentially add a negative test here, although it's difficult to
construct an invalid Date64Type.
--
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-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
