Thread: to_char(OF) is broken
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
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
Attachment
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
Attachment
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