Re: [HACKERS] surprising to_timestamp behavior

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Jeevan Chalke <jeevan(dot)chalke(at)enterprisedb(dot)com>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Postgres-Bugs <pgsql-bugs(at)postgresql(dot)org>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [HACKERS] surprising to_timestamp behavior
Date: 2014-01-20 18:53:33
Message-ID: 32524.1390244013@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs pgsql-hackers

Jeevan Chalke <jeevan(dot)chalke(at)enterprisedb(dot)com> writes:
>> I went to review this, and found that there's not actually a patch
>> attached ...

> Attached. Sorry for that.

This looks good to me except for one thing: if the upcoming node is a
DCH_FX action (ie, turn on fx_mode), I don't think we want to skip
whitespace. The original coding had that bug too, but it was only
exposed if the FX prefix was immediately preceded by whitespace in
the format.

It's a bit hard to think of useful cases where this would make a
difference, because it turns out that from_char_parse_int contains
an internal skip-whitespace action that's applied regardless of FX
mode, so it really only matters for non-integer field types. I
kinda think that maybe now we should remove that extra skip, or at
least count the skipped whitespace against the field width, but
did not experiment to see what the results would be.

Committed with that change and some cosmetic adjustments.

regards, tom lane

In response to

Browse pgsql-bugs by date

  From Date Subject
Next Message Boszormenyi Zoltan 2014-01-20 22:20:22 Re: BUG #8893: Precompiler ECPG doesn't check Var-List in Select corrrect.
Previous Message Tom Lane 2014-01-20 15:36:49 Re: BUG #8893: Precompiler ECPG doesn't check Var-List in Select corrrect.

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2014-01-20 20:15:35 Re: Trigger information for auto_explain.
Previous Message Jaime Casanova 2014-01-20 18:52:50 Re: Trigger information for auto_explain.