Bug in tm2timestamp

Magnus Hagander <magnus@hagander.net> writes:

AFAICT, there's a bug in tm2timestamp(). You can't do this:
postgres=# select '1999-12-31T24:00:00'::timestamptz;
ERROR: timestamp out of range: "1999-12-31T24:00:00"

But that's a perfectly legal date. It works fine for any other year -
and AFAICT this is because of the POSTGRES_EPOCH_JDATE being
2000-01-01.

The check in 1693 and forward comes with *result=0 and date=-1 in this
case, which AFAICT is fine.

Meh. Looks like I fixed this back in 2003, but didn't fix it right.
Will try again.

I'm not entirely sure what that check is guarding against (which may
be because I just came off a flight from canada and don't really have
the brain in full gear ATM). Perhaps it just needs an extra exclusion
for this special case?

I think we should just tweak the tests so that if "date" is 0 or -1,
we don't consider it an overflow even if the time adjustment pushes
the result to the other sign.

BTW, it strikes me that it's a bit silly to go to all this effort
here, and then ignore the possibility of overflow in the dt2local
adjustment just below. But we'd have to change the API of that
function, which I don't especially feel like doing right now.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Tom Lane wrote:

BTW, it strikes me that it's a bit silly to go to all this effort
here, and then ignore the possibility of overflow in the dt2local
adjustment just below. But we'd have to change the API of that
function, which I don't especially feel like doing right now.

Another point worth considering is that most of this is duplicated by
ecpg's libpgtypes. Do we want to fix that one too, or do we just let it
continue to be broken? I note that other bugs are already unfixed in
ecpg's copy. One other idea to consider is moving these things to
src/common, so we would have a single implementation. I already have a
patch that implements most of that, but it's only 90% there because it's
missing support for some things that the current code manipulates as
global variables (via GUC), and I didn't want to waste more time fixing
that. AFAICS it's just a SMOP, though, but I had postponed that whole
effort to the 9.4 cycle to avoid stalling 9.3 even longer.

But in light of this bug and other already fixed date/time bugs, perhaps
it's warranted? Opinions please.

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Alvaro Herrera wrote:

Tom Lane wrote:

BTW, it strikes me that it's a bit silly to go to all this effort
here, and then ignore the possibility of overflow in the dt2local
adjustment just below. But we'd have to change the API of that
function, which I don't especially feel like doing right now.

Another point worth considering is that most of this is duplicated by
ecpg's libpgtypes.

Bah, ignore this.

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Alvaro Herrera <alvherre@2ndquadrant.com> writes:

Alvaro Herrera wrote:

Another point worth considering is that most of this is duplicated by
ecpg's libpgtypes.

Bah, ignore this.

Huh? I think you're quite right that it'd be a good idea to get rid of
the duplicated code, if we could. It's always been the backend's free
reliance on palloc and elog/ereport that's been stopping that. I think
we might now have a realistic solution for palloc, but what about elog?

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Mon, Mar 4, 2013 at 8:54 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Magnus Hagander <magnus@hagander.net> writes:

AFAICT, there's a bug in tm2timestamp(). You can't do this:
postgres=# select '1999-12-31T24:00:00'::timestamptz;
ERROR: timestamp out of range: "1999-12-31T24:00:00"

But that's a perfectly legal date. It works fine for any other year -
and AFAICT this is because of the POSTGRES_EPOCH_JDATE being
2000-01-01.

The check in 1693 and forward comes with *result=0 and date=-1 in this
case, which AFAICT is fine.

Meh. Looks like I fixed this back in 2003, but didn't fix it right.
Will try again.

I'm not entirely sure what that check is guarding against (which may
be because I just came off a flight from canada and don't really have
the brain in full gear ATM). Perhaps it just needs an extra exclusion
for this special case?

I think we should just tweak the tests so that if "date" is 0 or -1,
we don't consider it an overflow even if the time adjustment pushes
the result to the other sign.

That's pretty much what I thought - thanks for confirming, and for
putting the fix in!

BTW, it strikes me that it's a bit silly to go to all this effort
here, and then ignore the possibility of overflow in the dt2local
adjustment just below. But we'd have to change the API of that
function, which I don't especially feel like doing right now.

Yeah, and it wouldn't be a good backpatchable thing anyway...

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Tom Lane wrote:

Alvaro Herrera <alvherre@2ndquadrant.com> writes:

Alvaro Herrera wrote:

Another point worth considering is that most of this is duplicated by
ecpg's libpgtypes.

Bah, ignore this.

Huh? I think you're quite right that it'd be a good idea to get rid of
the duplicated code, if we could. It's always been the backend's free
reliance on palloc and elog/ereport that's been stopping that. I think
we might now have a realistic solution for palloc, but what about elog?

Well, for instance ecpg's EncodeSpecialTimestamp uses a true/false
return value instead of raising an error (though its only caller does
not check it). There were few uses of elog/ereport that were really
problematic, though I admit that even a single one could mean going
through a lot of hoops.

Simply crafting an implementation of elog/ereport for frontend and ecpg
is probably not going to work, though, because ecpg normally returns
error codes for the caller to figure out. Maybe we could create a layer
on top of ereport, that gets both the error message, sqlstate etc, and
also a return code for ECPG. Or a replacement, so instead of ereport()
we have, say, cmn_ereport() that expands to errstart/errfinish on
backend and something else on ecpg.

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Mon, Mar 04, 2013 at 05:08:26PM -0300, Alvaro Herrera wrote:

Another point worth considering is that most of this is duplicated by
ecpg's libpgtypes. Do we want to fix that one too, or do we just let it
continue to be broken? I note that other bugs are already unfixed in
ecpg's copy. One other idea to consider is moving these things to

Meaning that a fix wasn't put there, too?

But in light of this bug and other already fixed date/time bugs, perhaps
it's warranted? Opinions please.

I'd love to go to a single source. Most of libpgtypes was taken from the
backend back when it was developed.

Michael
--
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org
Jabber: michael.meskes at gmail dot com
VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Mon, Mar 04, 2013 at 05:55:26PM -0300, Alvaro Herrera wrote:

error codes for the caller to figure out. Maybe we could create a layer
on top of ereport, that gets both the error message, sqlstate etc, and
...

Couldn't we just create ecpg's own version of ereport, that does "the right
thing" for ecpg?

Michael
--
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org
Jabber: michael.meskes at gmail dot com
VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Michael Meskes wrote:

On Mon, Mar 04, 2013 at 05:08:26PM -0300, Alvaro Herrera wrote:

Another point worth considering is that most of this is duplicated by
ecpg's libpgtypes. Do we want to fix that one too, or do we just let it
continue to be broken? I note that other bugs are already unfixed in
ecpg's copy. One other idea to consider is moving these things to

Meaning that a fix wasn't put there, too?

Yes, a fix was put there by Tom (which is why I retracted my comment
initially). I did note that the ecpg code has diverged from the backend
code; it's not unlikely that other bug fixes have not gone to the ecpg
copy. But I didn't investigate each difference in detail.

But in light of this bug and other already fixed date/time bugs, perhaps
it's warranted? Opinions please.

I'd love to go to a single source. Most of libpgtypes was taken from the
backend back when it was developed.

I will keep that in mind, if I get back to moving the timestamp/datetime
code to src/common. It's not a high priority item right now.

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers