On Mon, Sep 18, 2023 at 5:09 PM Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
>
> On Wed, 13 Sept 2023 at 11:13, Ashutosh Bapat
> <ashutosh.bapat.oss@gmail.com> wrote:
> >
> > On Tue, Sep 12, 2023 at 2:39 PM Dean Rasheed <dean.a.rasheed@gmail.com> 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).
The since the code was there earlier, I missed that part. Sorry.
>
> I'd also suggest a comment to indicate why itm_in isn't updated in
> this case (see similar case in DecodeDateTime(), for example).
>
Added but in the function prologue since it's part of the API.
>
> 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.
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