alamb commented on code in PR #8125:
URL: https://github.com/apache/arrow-rs/pull/8125#discussion_r2274013728
##########
parquet-variant-compute/src/cast_to_variant.rs:
##########
@@ -482,6 +539,111 @@ mod tests {
)
}
+ #[test]
+ fn test_cast_to_variant_interval_year_month() {
+ run_test(
+ Arc::new(IntervalYearMonthArray::from(vec![
+ Some(0),
+ None,
+ Some(12), // 1 year
+ Some(-6), // -6 months
+ Some(i32::MAX),
+ Some(i32::MIN),
+ ])),
+ vec![
+ Some(Variant::Int32(0)),
Review Comment:
I double checked that there doesn't seem to be a interval / duration type in
Variant:
https://github.com/apache/parquet-format/blob/master/VariantEncoding.md#encoding-types
Converting to Int32 / VariantBinary is about the best we can do for now.
However, it might be misleading
Another behavior might be to throw a "InvalidArgument" Error and make it
clear that casting from Intervals to Variant is not well defined / supported.
What do you think?
##########
parquet-variant-compute/src/cast_to_variant.rs:
##########
@@ -482,6 +539,111 @@ mod tests {
)
}
+ #[test]
+ fn test_cast_to_variant_interval_year_month() {
+ run_test(
+ Arc::new(IntervalYearMonthArray::from(vec![
+ Some(0),
+ None,
+ Some(12), // 1 year
+ Some(-6), // -6 months
+ Some(i32::MAX),
+ Some(i32::MIN),
+ ])),
+ vec![
+ Some(Variant::Int32(0)),
+ None,
+ Some(Variant::Int32(12)),
+ Some(Variant::Int32(-6)),
+ Some(Variant::Int32(i32::MAX)),
+ Some(Variant::Int32(i32::MIN)),
+ ],
+ );
+ }
+
+ #[test]
+ fn test_cast_to_variant_interval_day_time() {
+ let intervals = vec![
+ Some(IntervalDayTime::new(0, 0)),
+ None,
+ Some(IntervalDayTime::new(1, 1000)), // 1 day, 1 second
+ Some(IntervalDayTime::new(-5, -500)), // -5 days, -500 ms
+ Some(IntervalDayTime::new(i32::MAX, i32::MAX)),
+ Some(IntervalDayTime::new(i32::MIN, i32::MIN)),
+ ];
+
+ let expected = intervals
+ .iter()
+ .map(|opt_interval| {
+ opt_interval.map(|interval| {
+ let days_bytes = interval.days.to_le_bytes();
+ let millis_bytes = interval.milliseconds.to_le_bytes();
+ let mut bytes = Vec::with_capacity(8);
+ bytes.extend_from_slice(&days_bytes);
+ bytes.extend_from_slice(&millis_bytes);
+ bytes
Review Comment:
I think if you made this `Variant::Binary(bytes)` you could then use the
existing `run_test` function rather than adding `run_binary_interval_test`
--
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]
