Re: Add missing function abs (interval)

On Sun, 26 Sept 2021 at 13:42, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> I wrote:
> > Isaac Morland <isaac(dot)morland(at)gmail(dot)com> writes:
> >> I've attached a patch for this. Turns out there was a comment in the
> source
> >> explaining that there is no interval_abs because it's not clear what to
> >> return; but I think it's clear that if i is an interval the larger of i
> and
> >> -i should be considered to be the absolute value, the same as would be
> done
> >> for any type; essentially, if the type is orderable and has a meaningful
> >> definition of unary minus, the definition of abs follows from those.
>
> > The problem with that blithe summary is the hidden assumption that
> > values that compare "equal" aren't interesting to distinguish.
>
> After thinking about this some more, it seems to me that it's a lot
> clearer that the definition of abs(interval) is forced by our comparison
> rules if you define it as
>
> CASE WHEN x < '0'::interval THEN -x ELSE x END
>
> In particular, this makes it clear what happens and why for values
> that compare equal to zero. The thing that is bothering me about
> the formulation GREATEST(x, -x) is exactly that whether you get x
> or -x in such a case depends on a probably-unspecified implementation
> detail inside GREATEST().
>

Thanks very much for continuing to think about this. It really reinforces
my impression that this community takes seriously input and suggestions,
even when it takes some thought to work out how (or whether) to proceed
with a proposed change.

So I think I will prepare a revised patch that uses this formulation; and
if I still have any suggestions that aren't directly related to adding
abs(interval) I will split them off into a separate discussion.

BTW, you could implement this by something along the lines of
> (cf generate_series_timestamp()):
>
> MemSet(&interval_zero, 0, sizeof(Interval));
> if (interval_cmp_internal(interval, &interval_zero) < 0)
> return interval_um(fcinfo);
> else
> PG_RETURN_INTERVAL_P(interval);
>
> which would avoid the need to refactor interval_um().
>
> regards, tom lane
>