[jira] [Comment Edited] (ARROW-14123) [C++] DayTimeIntervalType::DayMilliseconds comparison function seems incorrect

["Phillip Cloud (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/ARROW-14123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17470819#comment-17470819 ] 

Phillip Cloud edited comment on ARROW-14123 at 1/7/22, 6:49 PM:
----------------------------------------------------------------

Two problems inherent to this comparison:

1. How should these two intervals compare?

{code}
DayMilliseconds = { days = 1, milliseconds = -1 }
DayMilliseconds = { days = 0, milliseconds = 86399999 }
{code}

2. Are intervals correctly comparable without a time zone? The specific start and end of an interval determines its length. Without that information there isn't any correct way to compare two interval types. For example, day 2 - day 1 might be 23 or 25 hours if there's a timezone change between day 2 and day 1.


was (Author: cpcloud):
Two problems inherent to this comparison:

1. How should these two intervals compare?

{code}
DayMilliseconds = { days = 1, milliseconds = -1 }
DayMilliseconds = { days = 0, milliseconds = 86399999 }
{code}

2. Are intervals correctly comparable without a time zone? The specific start and end of an interval determines it's length. Without that information there isn't any correct way to compare two interval types.

> [C++] DayTimeIntervalType::DayMilliseconds comparison function seems incorrect
> ------------------------------------------------------------------------------
>
>                 Key: ARROW-14123
>                 URL: https://issues.apache.org/jira/browse/ARROW-14123
>             Project: Apache Arrow
>          Issue Type: Bug
>          Components: C++
>            Reporter: Phillip Cloud
>            Assignee: Alvin Chunga Mamani
>            Priority: Major
>              Labels: kernel
>             Fix For: 8.0.0
>
>
> The less than operator ({{operator<}}) implemented on {{DayTimeIntervalType::DayMilliseconds}} seems incorrect:
> {code:cpp}
>     bool operator<(DayMilliseconds other) const {
>       return this->days < other.days || this->milliseconds < other.milliseconds;
>     }
> {code}
> With this implementation, an example (in pseudocode) such as 
> {code:cpp}
> DayMilliseconds { days = 10, milliseconds = 999 } < DayMilliseconds { days = 9, milliseconds = 1000 }
> {code}
> would return {{true}} when it should return {{false}}.



--
This message was sent by Atlassian Jira
(v8.20.1#820001)