scovich commented on code in PR #8516:
URL: https://github.com/apache/arrow-rs/pull/8516#discussion_r2411678858
##########
parquet-variant-compute/src/type_conversion.rs:
##########
@@ -60,6 +89,35 @@ impl_primitive_from_variant!(datatypes::UInt64Type, as_u64);
impl_primitive_from_variant!(datatypes::Float16Type, as_f16);
impl_primitive_from_variant!(datatypes::Float32Type, as_f32);
impl_primitive_from_variant!(datatypes::Float64Type, as_f64);
+impl_primitive_from_variant!(
+ datatypes::Date32Type,
+ as_naive_date,
+ Date32Type::from_naive_date
+);
+impl_timestamp_from_variant!(
+ datatypes::TimestampMicrosecondType,
+ as_timestamp_ntz_micros,
+ ntz = true,
+ Self::make_value,
+);
+impl_timestamp_from_variant!(
+ datatypes::TimestampMicrosecondType,
+ as_timestamp_micros,
+ ntz = false,
+ Self::make_value_tz
Review Comment:
nit: Since we anyway need a second trait and macro for timestamp types, it's
no longer so important to have the MakeValueTz extension trait. Originally, the
hope was that we could have one uniform `Self::from_variant` method (perhaps
provided by several different traits) that would let builders invoke it in a
uniform way. But that's not the case and the trait definition is bigger than
the logic it abstracts away:
```suggestion
|timestamp| Self::make_value(timestamp.naive_utc())
```
(again below)
(your call tho -- the trait doesn't hurt anything)
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
