Re: Date-Time dangling unit fix

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Joseph Koshakow <koshy44(at)gmail(dot)com>
Cc: Alexander Lakhin <exclusion(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Date-Time dangling unit fix
Date: 2023-03-10 00:26:31
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Joseph Koshakow <koshy44(at)gmail(dot)com> writes:
> Also I removed some dead code from the previous patch.

This needs a rebase over bcc704b52, so I did that and made a
couple of other adjustments.

I'm inclined to think that you removed too much from DecodeTimeOnly.
That does accept date specs (at least for timetz), and I see no very
good reason for it not to accept a Julian date spec. I also wonder
why you removed the UNITS: case there. Seems like we want these
functions to accept the same syntax as much as possible.

I think the code is still a bit schizophrenic about placement of
ptype specs. In the UNITS: case, we don't insist that a unit
apply to exactly the very next field; instead it applies to the next
one where it disambiguates. So for instead this is accepted:

regression=# select 'J PM 1234567 1:23'::timestamp;
1333-01-11 13:23:00 BC

That's a little weird, or maybe even a lot weird, but it's not
inherently nonsensical so I'm hesitant to stop accepting it.
However, if UNITS acts that way, then why is ISOTIME different?
So I'm inclined to remove ISOTIME's lookahead check

if (i >= nf - 1 ||
(ftype[i + 1] != DTK_NUMBER &&
ftype[i + 1] != DTK_TIME &&
ftype[i + 1] != DTK_DATE))

and rely on the ptype-still-set error at the bottom of the loop
to complain about nonsensical cases.

Also, if we do keep the lookahead checks, the one in DecodeTimeOnly
could be simplified --- it's accepting some cases that actually
aren't supported there.

regards, tom lane

Attachment Content-Type Size
v4-0001-Remove-unknown-ISO-format-handle-dandling-units.patch text/x-diff 11.1 KB

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2023-03-10 00:46:39 Re: Sub-millisecond [autovacuum_]vacuum_cost_delay broken
Previous Message Thomas Munro 2023-03-10 00:25:52 Re: Sub-millisecond [autovacuum_]vacuum_cost_delay broken