alamb commented on code in PR #8401:
URL: https://github.com/apache/arrow-rs/pull/8401#discussion_r2373253135
##########
parquet-variant-compute/src/variant_array.rs:
##########
@@ -864,6 +866,51 @@ fn typed_value_to_variant(typed_value: &ArrayRef, index:
usize) -> Variant<'_, '
DataType::Float64 => {
primitive_conversion_single_value!(Float64Type, typed_value, index)
}
+ DataType::Timestamp(timeunit, tz) => {
+ match (timeunit, tz) {
+ (TimeUnit::Microsecond, Some(_)) => {
+ generic_conversion_single_value!(
+ TimestampMicrosecondType,
+ as_primitive,
+ |v| DateTime::from_timestamp_micros(v).unwrap(),
+ typed_value,
+ index
+ )
+ }
+ (TimeUnit::Microsecond, None) => {
+ generic_conversion_single_value!(
+ TimestampMicrosecondType,
+ as_primitive,
+ |v|
DateTime::from_timestamp_micros(v).unwrap().naive_utc(),
+ typed_value,
+ index
+ )
+ }
+ (TimeUnit::Nanosecond, Some(_)) => {
+ generic_conversion_single_value!(
+ TimestampNanosecondType,
+ as_primitive,
+ DateTime::from_timestamp_nanos,
+ typed_value,
+ index
+ )
+ }
+ (TimeUnit::Nanosecond, None) => {
+ generic_conversion_single_value!(
+ TimestampNanosecondType,
+ as_primitive,
+ |v| DateTime::from_timestamp_nanos(v).naive_utc(),
+ typed_value,
+ index
+ )
+ }
+ // Variant timestamp only support time unit with microsecond
or nanosecond precision
+ _ => panic!(
Review Comment:
yes, let's not panic here
ANother potential approach is to return a not yet implemented error -- maybe
by matching the supported types explicitly, so like
```rust
DataType::Timestamp(TimeUnit::Microsecond, Some(_)) => {
generic_conversion_single_value!(
TimestampMicrosecondType,
as_primitive,
|v| DateTime::from_timestamp_micros(v).unwrap(),
typed_value,
index
)
}
```
instead of
```rust
DataType::Timestamp(timeunit, tz) => {
match (timeunit, tz) {
(TimeUnit::Microsecond, Some(_)) => {
generic_conversion_single_value!(
TimestampMicrosecondType,
as_primitive,
|v| DateTime::from_timestamp_micros(v).unwrap(),
typed_value,
index
)
}
```
##########
parquet/tests/variant_integration.rs:
##########
@@ -119,23 +106,10 @@ variant_test_case!(30);
variant_test_case!(31);
// https://github.com/apache/arrow-rs/issues/8334
variant_test_case!(32, "Unsupported typed_value type: Time64(Microsecond)");
-// https://github.com/apache/arrow-rs/issues/8331
Review Comment:
🎉
--
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]
