to_char(OF) is broken

Started by David Fetterabout 10 years ago4 messagesbugs
Jump to latest
#1David Fetter
david@fetter.org

Folks,

In to_char(), the new-in-9.4 'OF' feature is straight-up broken, to
wit:

set timezone to 'America/Caracas'; select to_char(now(), 'OF');
SET
to_char
─────────
-04:-30
^OK ^NOT OK

Interestingly, in git master as of yesterday, it's broken in an
entirely different way:

set timezone to 'America/Caracas'; select to_char(now(), 'OF');
SET
to_char
─────────
-004:30
^ What's this extra zero doing here?
(1 row)

Fractional offset time zones should probably be in our regression test
suite for this feature.

Cheers,
David.
--
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

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

#2Thomas Munro
thomas.munro@gmail.com
In reply to: David Fetter (#1)
Re: to_char(OF) is broken

On Thu, Mar 17, 2016 at 3:02 AM, David Fetter <david@fetter.org> wrote:

Folks,

In to_char(), the new-in-9.4 'OF' feature is straight-up broken, to
wit:

set timezone to 'America/Caracas'; select to_char(now(), 'OF');
SET
to_char
─────────
-04:-30
^OK ^NOT OK

Interestingly, in git master as of yesterday, it's broken in an
entirely different way:

set timezone to 'America/Caracas'; select to_char(now(), 'OF');
SET
to_char
─────────
-004:30
^ What's this extra zero doing here?
(1 row)

Fractional offset time zones should probably be in our regression test
suite for this feature.

It looks like 2d87eedc made adjustments in accounting for padding
negative numbers in several places, but the case of OF is different
than the other places because it also has a sign for positive numbers,
so no adjustment was necessary there. See attached.

--
Thomas Munro
http://www.enterprisedb.com

Attachments:

caracas.patchapplication/octet-stream; name=caracas.patchDownload+40-1
#3Thomas Munro
thomas.munro@gmail.com
In reply to: Thomas Munro (#2)
Re: to_char(OF) is broken

On Thu, Mar 17, 2016 at 3:23 PM, Thomas Munro
<thomas.munro@enterprisedb.com> wrote:

On Thu, Mar 17, 2016 at 3:02 AM, David Fetter <david@fetter.org> wrote:

Folks,

In to_char(), the new-in-9.4 'OF' feature is straight-up broken, to
wit:

set timezone to 'America/Caracas'; select to_char(now(), 'OF');
SET
to_char
─────────
-04:-30
^OK ^NOT OK

Interestingly, in git master as of yesterday, it's broken in an
entirely different way:

set timezone to 'America/Caracas'; select to_char(now(), 'OF');
SET
to_char
─────────
-004:30
^ What's this extra zero doing here?
(1 row)

Fractional offset time zones should probably be in our regression test
suite for this feature.

It looks like 2d87eedc made adjustments in accounting for padding
negative numbers in several places, but the case of OF is different
than the other places because it also has a sign for positive numbers,
so no adjustment was necessary there. See attached.

... and 9.4 has a different bug as you showed (missing abs). Here's a
patch for 9.4 that uses the same code as master for that switch case
(and adds the same regression test).

9.5 doesn't have either of the bugs, but might as well have the same
regression test just to show that.

--
Thomas Munro
http://www.enterprisedb.com

Attachments:

caracas-9.4.patchapplication/octet-stream; name=caracas-9.4.patchDownload+43-3
caracas-9.5.patchapplication/octet-stream; name=caracas-9.5.patchDownload+39-0
#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Thomas Munro (#3)
Re: to_char(OF) is broken

Thomas Munro <thomas.munro@enterprisedb.com> writes:

On Thu, Mar 17, 2016 at 3:23 PM, Thomas Munro
<thomas.munro@enterprisedb.com> wrote:

On Thu, Mar 17, 2016 at 3:02 AM, David Fetter <david@fetter.org> wrote:

Fractional offset time zones should probably be in our regression test
suite for this feature.

It looks like 2d87eedc made adjustments in accounting for padding
negative numbers in several places, but the case of OF is different
than the other places because it also has a sign for positive numbers,
so no adjustment was necessary there. See attached.

... and 9.4 has a different bug as you showed (missing abs). Here's a
patch for 9.4 that uses the same code as master for that switch case
(and adds the same regression test).

9.5 doesn't have either of the bugs, but might as well have the same
regression test just to show that.

This is still not quite right, as it would do the wrong thing for a
zone '-00:30'. I'm not sure that such a zone exists in reality,
but we might as well get it right.

Will fix and commit. Thanks for looking at this!

regards, tom lane

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