Date: Sun, 19 Nov 2017 17:47:35 +0300
From: Arthur Zakirov <a.zakirov@postgrespro.ru>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: amul sul <sulamul@gmail.com>, Pavel Stehule <pavel.stehule@gmail.com>,
	Robert Haas <robertmhaas@gmail.com>,
	"David G. Johnston" <david.g.johnston@gmail.com>,
	Alex Ignatov <a.ignatov@postgrespro.ru>,
	Bruce Momjian <bruce@momjian.us>, amul sul <sul_amul@yahoo.co.in>,
	PostgreSQL-development <pgsql-hackers@postgresql.org>
Subject: Re: [HACKERS] Bug in to_timestamp().
Message-ID: <20171119144734.GA12415@arthur.localdomain>
References: 
 <CA+TgmoaFQZeqmAo3CdivNu70coB=9eM5OroKYtNcTuMaC1kuUQ@mail.gmail.com>
 <32684.1466704957@sss.pgh.pa.us>
 <734e50cd-c563-4afd-d1c8-80f244b3265e@postgrespro.ru>
 <CAFj8pRDGxe=cdcZ7pur=2WeW9O3MQCZTBEAOJfQ7OLQJaYXDvw@mail.gmail.com>
 <b2a39359-3282-b402-f4a3-057aae500ee7@postgrespro.ru>
 <29142.1475097531@sss.pgh.pa.us>
 <CAKNkYny-0YJZzz=a9E==UBuiEoMA!FV=n6tik7oLbDVtahiW7tA@mail.gmail.com>
 <8744.1478284597@sss.pgh.pa.us>
 <CAKNkYnxCqwC+aXZFXent27SU5=nonR6JPxJDxfnf9YiXW71PKA@mail.gmail.com>
 <29637.1511045306@sss.pgh.pa.us>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <29637.1511045306@sss.pgh.pa.us>
User-Agent: Mutt/1.9.1 (2017-09-22)
Precedence: bulk

On Sat, Nov 18, 2017 at 05:48:26PM -0500, Tom Lane wrote:
> This patch needs a rebase over the formatting.c fixes that have gone
> in over the last couple of days.
> 
> Looking at the rejects, I notice that in your changes to parse_format(),
> you seem to be making it rely even more heavily on remembering state about
> the previous input.  I recommend against that --- it was broken before,
> and it's a pretty fragile approach.  Backslashes are not that hard to
> deal with in-line.

I can continue to work on a better approach. Though, the patch was made a long time
ago I'll refresh my memory. The main intent was to fix the following
behaviour:

=# SELECT TO_TIMESTAMP('2016-06-13 15:43:36', 'YYYY/MM/DD  HH24:MI:SS');
to_timestamp 
------------------------
2016-06-13 05:43:36-07       <— incorrect time

> select to_timestamp('97/Feb/16', 'YY:Mon:DD')
> select to_timestamp('97/Feb/16', 'YY Mon DD')
> select to_timestamp('97 Feb 16', 'YY/Mon/DD')
> 
> (Well, Oracle thinks these mean 2097 where we think 1997, but the point is
> you don't get an error.)  I see from your regression test additions that
> you want to make some of these throw an error, and I think that any such
> proposal is dead in the water.  Nobody is going to consider it an
> improvement if it both breaks working PG apps and disagrees with Oracle.
> 
> 			regards, tom lane

If I understand correctly, these queries don't throw an error and the patch tried to follow Oracle's rules.
Only queries with FX field throw an error. For example:

=# SELECT to_timestamp('97/Feb/16', 'FXYY:Mon:DD');
ERROR:  unexpected character "/", expected ":"

From Oracle's documentation [1]:

> FX - Requires exact matching between the character data and the format model.

I agree that compatibility breaking is not good and a fu
ure patch may only try to fix wrong output date and time as in Amul's first email.

1 - https://docs.oracle.com/cd/B19306_01/server.102/b14200/sql_elements004.htm#i34924

-- 
Arthur Zakirov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company