ARROW-13989: [C++] Add support for month-day-nano interval to compute functions #11525
Conversation
|
|
| @@ -1302,7 +1304,8 @@ struct CopyFixedWidth<BooleanType> { | |||
| }; | |||
|
|
|||
| template <typename Type> | |||
| struct CopyFixedWidth<Type, enable_if_number<Type>> { | |||
| struct CopyFixedWidth< | |||
| Type, enable_if_t<is_number_type<Type>::value || is_interval_type<Type>::value>> { | |||
There was a problem hiding this comment.
Out of curiosity, why is_interval_type here while IfElseFunctor only allows month-day-nano intervals?
There was a problem hiding this comment.
This helper is used by coalesce/choose/case_when so this is part of adding support for all of those functions.
if_else should support all the interval types. It uses GenerateTypeAgnosticPrimitive, so DayTime and Month intervals are handled as integers.
| this->mask("[true, true]"), this->scalar("null"), | ||
| this->array("[null, null]")); | ||
| this->Assert(ReplaceWithMask, this->array("[[1, 2, 4], [3, 4, -2], [-5, 6, 7]]"), | ||
| this->mask("[true, false, null]"), this->scalar("[7, 0, 8]"), |
There was a problem hiding this comment.
Slightly unrelated to this PR, but it seems that amongst the ReplaceWithMask tests, the mask is always of the form (regex) true* (false|null)*, meaning that the replacement index is always trivially equal to the source index. Is that deliberate?
There was a problem hiding this comment.
It's not deliberate. I'll update the tests to avoid this.
|
Benchmark runs are scheduled for baseline = 140b0b2 and contender = f4dd806. f4dd806 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
No description provided.