Subject: Re: Infinite Interval

From: Dean Rasheed <[email-masked]>
Date: Mon, 18 Sep 2023 12:39:35 +0100

On Wed, 13 Sept 2023 at 11:13, Ashutosh Bapat
<[email-masked]> wrote:
>
> On Tue, Sep 12, 2023 at 2:39 PM Dean Rasheed <[email-masked]> wrote:
> >
> > and it looks like the infinite interval
> > input code is broken.
>
> The code required to handle 'infinity' as an input value was removed by
> d6d1430f404386162831bc32906ad174b2007776. I have added a separate
> commit which reverts that commit as 0004, which should be merged into
> 0003.
>

I think that simply reverting d6d1430f404386162831bc32906ad174b2007776
is not sufficient. This does not make it clear what the point is of
the code in the "case RESERV" block. That code really should check the
value returned by DecodeSpecial(), otherwise invalid inputs are not
caught until later, and the error reported is not ideal. For example:

select interval 'now';
ERROR:  unexpected dtype 12 while parsing interval "now"

So DecodeInterval() should return DTERR_BAD_FORMAT in such cases (see
similar code in DecodeTimeOnly(), for example).

I'd also suggest a comment to indicate why itm_in isn't updated in
this case (see similar case in DecodeDateTime(), for example).

Another point to consider is what should happen if "ago" is specified
with infinite inputs. As it stands, it is accepted, but does nothing:

select interval 'infinity ago';
 interval
----------
 infinity
(1 row)

select interval '-infinity ago';
 interval
-----------
 -infinity
(1 row)

This could be made to invert the sign, as it does for finite inputs,
but I think perhaps it would be better to simply reject such inputs.

Regards,
Dean

---

From: Ashutosh Bapat <[email-masked]>
Date: Wed, 20 Sep 2023 18:59:52 +0530

> similar code in DecodeTimeOnly(), for example).

The since the code was there earlier, I missed that part. Sorry.

> this case (see similar case in DecodeDateTime(), for example).

Added but in the function prologue since it's part of the API.

> select interval 'infinity ago';

Fixed this. After studying what DecodeInterval is doing, I think it's
better to treat all infinity specifications similar to "ago". They
need to be the last part of the input string. Rest of the code makes
sure that nothing preceds infinity specification as other case blocks
do not handle RESERVE or DTK_LATE and DTK_EARLY. This means that
"+infinity", "-infinity" or "infinity" can be the only allowed word as
a valid interval input when either of them is specified.

I will post these changes in another email along with other patches.

--
Best Wishes,
Ashutosh Bapat