jorgecarleitao commented on a change in pull request #779:
URL: https://github.com/apache/arrow-rs/pull/779#discussion_r763640120
##########
File path: arrow/src/compute/kernels/sort.rs
##########
@@ -243,6 +243,11 @@ pub fn sort_to_indices(
DataType::Interval(IntervalUnit::DayTime) => {
sort_primitive::<IntervalDayTimeType, _>(values, v, n, cmp,
&options, limit)
}
+ DataType::Interval(IntervalUnit::MonthDayNano) => {
+ sort_primitive::<IntervalMonthDayNanoType, _>(
Review comment:
The i128 order relationship does not hold for `months,days,nanos`. AFAIK
`month,days,nanos` do not have a partial order relationship.
##########
File path: arrow/src/datatypes/numeric.rs
##########
@@ -348,6 +348,7 @@ make_numeric_type!(Time64MicrosecondType, i64, i64x8,
m64x8);
make_numeric_type!(Time64NanosecondType, i64, i64x8, m64x8);
make_numeric_type!(IntervalYearMonthType, i32, i32x16, m32x16);
make_numeric_type!(IntervalDayTimeType, i64, i64x8, m64x8);
+make_numeric_type!(IntervalMonthDayNanoType, i128, i128x4, m128x4);
Review comment:
Semantically, the numerics of an `i128` are not the same as the numerics
of `(months,days,nanos)` since `i128 + i128 != (months, days,nanos) + (months,
days,nanos)`.
The consequence of defining this type numerically here is that arithmetic
kernels will accept this type, but they will yield a semantically incorrect
result (e.g. `i128 + i128` to sum two intervals of 1 month each).
--
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
