Re: [HACKERS] Bug in to_timestamp().

On 09/22/2018 10:00 PM, Alexander Korotkov wrote: On Thu, Sep 20, 2018 at 3:52 PM Alexander Korotkov wrote: On Thu, Sep 20, 2018 at 6:09 AM amul sul wrote: Agreed, thanks for working on this. Pushed, thanks. Please, find attached patch improving documentation about letters/digits in

Re: [HACKERS] Bug in to_timestamp().

On Thu, Sep 20, 2018 at 3:52 PM Alexander Korotkov wrote: > > On Thu, Sep 20, 2018 at 6:09 AM amul sul wrote: > > Agreed, thanks for working on this. > > Pushed, thanks. Please, find attached patch improving documentation about letters/digits in to_date()/to_timestamp() template string. I

Re: [HACKERS] Bug in to_timestamp().

On Thu, Sep 20, 2018 at 6:09 AM amul sul wrote: > On Thu, Sep 20, 2018, 3:22 AM Alexander Korotkov > wrote: >> It's hard to understand whether it was expected, because it wasn't >> properly documented. More important that it's the same behavior we >> have before cf984672, and purpose of

Re: [HACKERS] Bug in to_timestamp().

On Thu, Sep 20, 2018, 3:22 AM Alexander Korotkov wrote: > On Wed, Sep 19, 2018 at 1:38 PM amul sul wrote: > > On Wed, Sep 19, 2018 at 3:51 PM amul sul wrote: > > > > > > On Wed, Sep 19, 2018 at 2:57 PM Alexander Korotkov > > [...] > > > > > > With this patch, to_date and to_timestamp behaving

Re: [HACKERS] Bug in to_timestamp().

On Wed, Sep 19, 2018 at 1:38 PM amul sul wrote: > On Wed, Sep 19, 2018 at 3:51 PM amul sul wrote: > > > > On Wed, Sep 19, 2018 at 2:57 PM Alexander Korotkov > [...] > > > > With this patch, to_date and to_timestamp behaving differently, see this: > > > > edb=# SELECT to_date('18 12 2011',

Re: [HACKERS] Bug in to_timestamp().

On Wed, Sep 19, 2018 at 3:51 PM amul sul wrote: > > On Wed, Sep 19, 2018 at 2:57 PM Alexander Korotkov [...] > > With this patch, to_date and to_timestamp behaving differently, see this: > > edb=# SELECT to_date('18 12 2011', 'xDDxMMx'); > to_date > > 18-DEC-11

Re: [HACKERS] Bug in to_timestamp().

On Wed, Sep 19, 2018 at 2:57 PM Alexander Korotkov wrote: > > On Tue, Sep 18, 2018 at 4:42 PM Alexander Korotkov > wrote: > > But, I found related issue in cf9846724. Before it was: > > > > # select to_timestamp('2018 01 01', '9MM9DD'); > > to_timestamp > > >

Re: [HACKERS] Bug in to_timestamp().

On Tue, Sep 18, 2018 at 4:42 PM Alexander Korotkov wrote: > But, I found related issue in cf9846724. Before it was: > > # select to_timestamp('2018 01 01', '9MM9DD'); > to_timestamp > > 2018-01-01 00:00:00+03 > (1 row) > > But after it becomes so. > > # select

Re: [HACKERS] Bug in to_timestamp().

On Tue, Sep 18, 2018 at 2:08 PM Prabhat Sahu wrote: > > Few more findings on to_timestamp() test with HEAD. > > postgres[3493]=# select to_timestamp('15-07-1984 23:30:32',' dd- mm- > hh24: mi: ss'); >to_timestamp > --- > 1984-07-15 23:30:32+05:30 > (1 row)

Re: [HACKERS] Bug in to_timestamp().

Hi All, Few more findings on to_timestamp() test with HEAD. postgres[3493]=# select to_timestamp('15-07-1984 23:30:32',' dd- mm- hh24: mi: ss'); to_timestamp --- 1984-07-15 23:30:32+05:30 (1 row) postgres[3493]=# select to_timestamp('15-07-*1984*

Re: [HACKERS] Bug in to_timestamp().

Alexander Korotkov writes: > I've closed commitfest entry. I think we can add new commitfest entry if > required. Regarding FX part, it easy to extract it as separate patch, but > it's hard to find consensus. I think there are at least three possible > decisions. > 1) Change FX mode to

Re: [HACKERS] Bug in to_timestamp().

On Tue, Sep 11, 2018 at 6:18 PM Arthur Zakirov wrote: > On Sun, Sep 09, 2018 at 09:52:46PM +0300, Alexander Korotkov wrote: > > So, pushed! Thanks to every thread participant for review and feedback. > > Great! Should we close the commitfest entry? There is FX part of the > patch though. But it

Re: [HACKERS] Bug in to_timestamp().

On Sun, Sep 09, 2018 at 09:52:46PM +0300, Alexander Korotkov wrote: > So, pushed! Thanks to every thread participant for review and feedback. Great! Should we close the commitfest entry? There is FX part of the patch though. But it seems that nobody is happy with it. It could be done with a

Re: [HACKERS] Bug in to_timestamp().

On Thu, Sep 6, 2018 at 3:58 PM Alexander Korotkov wrote: > On Thu, Sep 6, 2018 at 2:40 PM Alexander Korotkov > wrote: > > > > On Wed, Sep 5, 2018 at 7:28 PM amul sul wrote: > > > On Wed, Sep 5, 2018, 6:35 PM Alexander Korotkov < > a.korot...@postgrespro.ru> wrote: > > >> On Wed, Sep 5, 2018 at

Re: [HACKERS] Bug in to_timestamp().

On Thu, Sep 6, 2018 at 2:40 PM Alexander Korotkov wrote: > > On Wed, Sep 5, 2018 at 7:28 PM amul sul wrote: > > On Wed, Sep 5, 2018, 6:35 PM Alexander Korotkov > > wrote: > >> On Wed, Sep 5, 2018 at 3:10 PM amul sul wrote: > >> > On Wed, Sep 5, 2018 at 3:05 PM Alexander Korotkov > >> >

Re: [HACKERS] Bug in to_timestamp().

On Wed, Sep 5, 2018 at 7:28 PM amul sul wrote: > On Wed, Sep 5, 2018, 6:35 PM Alexander Korotkov > wrote: >> On Wed, Sep 5, 2018 at 3:10 PM amul sul wrote: >> > On Wed, Sep 5, 2018 at 3:05 PM Alexander Korotkov >> > wrote: >> > > On Wed, Sep 5, 2018 at 1:22 AM David G. Johnston >> > > wrote:

Re: [HACKERS] Bug in to_timestamp().

On Wed, Sep 5, 2018, 6:35 PM Alexander Korotkov wrote: > On Wed, Sep 5, 2018 at 3:10 PM amul sul wrote: > > On Wed, Sep 5, 2018 at 3:05 PM Alexander Korotkov > > wrote: > > > On Wed, Sep 5, 2018 at 1:22 AM David G. Johnston > > > wrote: > > > > From those results the question is how important

Re: [HACKERS] Bug in to_timestamp().

On Wed, Sep 5, 2018 at 3:05 PM Alexander Korotkov wrote: > > On Wed, Sep 5, 2018 at 1:22 AM David G. Johnston > wrote: > > On Mon, Sep 3, 2018 at 2:30 PM, Alexander Korotkov > > wrote: > >> > >> The current version of patch doesn't really distinguish spaces and > >> delimiters in format string

Re: [HACKERS] Bug in to_timestamp().

On Wed, Sep 5, 2018 at 1:22 AM David G. Johnston wrote: > On Mon, Sep 3, 2018 at 2:30 PM, Alexander Korotkov > wrote: >> >> The current version of patch doesn't really distinguish spaces and >> delimiters in format string in non-FX mode. So, spaces and delimiters >> are forming single group.

Re: [HACKERS] Bug in to_timestamp().

On Mon, Sep 3, 2018 at 2:30 PM, Alexander Korotkov < a.korot...@postgrespro.ru> wrote: > The current version of patch doesn't really distinguish spaces and > delimiters in format string in non-FX mode. So, spaces and delimiters > are forming single group. For me Oracle behavior is ridiculous at

Re: [HACKERS] Bug in to_timestamp().

Hi! Sorry for very long reply. On Thu, Aug 16, 2018 at 11:44 PM David G. Johnston wrote: > If the new behavior is an error I don't really have a problem since the need > to fix one's queries will be obvious. > > "So length of last group of spaces/separators in the pattern should be > greater

Re: [HACKERS] Bug in to_timestamp().

On Thu, Aug 16, 2018 at 3:53 AM, Alexander Korotkov < a.korot...@postgrespro.ru> wrote: > Hi, David! > > You can notice that: > > 1) We identified downside of changes in to_timestamp() function and > documented them [1]. > 2) We found 4 more differences between between patches behavior and >

Re: [HACKERS] Bug in to_timestamp().

On Thu, Aug 16, 2018 at 4:57 PM Liudmila Mantrova wrote: > On 08/14/2018 06:38 PM, Alexander Korotkov wrote: > > On Thu, Aug 2, 2018 at 9:06 PM Alexander Korotkov > > wrote: > > BTW, I've also revised documentation and regression tests. Patch is > > attached. > > > Please consider some further

Re: [HACKERS] Bug in to_timestamp().

On 08/14/2018 06:38 PM, Alexander Korotkov wrote: On Thu, Aug 2, 2018 at 9:06 PM Alexander Korotkov wrote: On Thu, Aug 2, 2018 at 6:17 PM Alexander Korotkov wrote: After some experiments I found that when you mix spaces and separators between two fields, then Oracle takes into account only

Re: [HACKERS] Bug in to_timestamp().

Hi, David! You can notice that: 1) We identified downside of changes in to_timestamp() function and documented them [1]. 2) We found 4 more differences between between patches behavior and Oracle behavior [2][3]. One of them was assumed to be ridiculous and wasn't implemented. So, I think this

Re: [HACKERS] Bug in to_timestamp().

On Tue, Aug 14, 2018 at 06:38:56PM +0300, Alexander Korotkov wrote: > BTW, I've also revised documentation and regression tests. Patch is attached. I looked at the patch. It applies without errors. The document looks good. It compiles. The code looks good too. It compiles and tests are passed.

Re: [HACKERS] Bug in to_timestamp().

On Thu, Aug 2, 2018 at 9:06 PM Alexander Korotkov wrote: > On Thu, Aug 2, 2018 at 6:17 PM Alexander Korotkov > wrote: > > After some experiments I found that when you mix spaces and separators > > between two fields, then Oracle takes into account only length of last > > group of

Re: [HACKERS] Bug in to_timestamp().

On Thu, Aug 02, 2018 at 06:17:01PM +0300, Alexander Korotkov wrote: > So, if number of spaces/separators between fields in input string is > more than in format string and list space/separator skipped is minus > sign, then it decides to glue that minus sign to TZH. I think this > behavior is nice

Re: [HACKERS] Bug in to_timestamp().

On Thu, Aug 2, 2018 at 6:17 PM Alexander Korotkov wrote: > After some experiments I found that when you mix spaces and separators > between two fields, then Oracle takes into account only length of last > group of spaces/separators. > > # SELECT to_timestamp('2018- -01 02', ' ---

Re: [HACKERS] Bug in to_timestamp().

On Wed, Aug 1, 2018 at 3:17 PM Arthur Zakirov wrote: > On Mon, Jul 23, 2018 at 05:21:43PM +0300, Alexander Korotkov wrote: > > Thank you, Arthur. These examples shows downside of this patch, where > > users may be faced with incompatibility. But it's good that this > > situation can be handled

Re: [HACKERS] Bug in to_timestamp().

Hello, Alexander, On Mon, Jul 23, 2018 at 05:21:43PM +0300, Alexander Korotkov wrote: > Thank you, Arthur. These examples shows downside of this patch, where > users may be faced with incompatibility. But it's good that this > situation can be handled by altering format string. I think these >

Re: [HACKERS] Bug in to_timestamp().

On Mon, Jul 23, 2018 at 5:12 PM Arthur Zakirov wrote: > On Mon, Jul 23, 2018 at 04:30:43PM +0300, Arthur Zakirov wrote: > > I looked for some tradeoffs of the patch. I think it could be parsing > > strings like the following input strings: > > > > SELECT TO_TIMESTAMP('2011年5月1日', '-MM-DD'); >

Re: [HACKERS] Bug in to_timestamp().

On Mon, Jul 23, 2018 at 04:30:43PM +0300, Arthur Zakirov wrote: > I looked for some tradeoffs of the patch. I think it could be parsing > strings like the following input strings: > > SELECT TO_TIMESTAMP('2011年5月1日', '-MM-DD'); > SELECT TO_TIMESTAMP('2011y5m1d', '-MM-DD'); > > HEAD

Re: [HACKERS] Bug in to_timestamp().

On Sun, Jul 22, 2018 at 08:22:23AM -0700, David G. Johnston wrote: > My re-read of the thread the other day left me with a feeling of > contentment that this was an acceptable change but I also get the feeling > like I'm missing the downside trade-off too...I was hoping your review > would help in

Re: [HACKERS] Bug in to_timestamp().

On Sun, Jul 22, 2018 at 6:22 PM David G. Johnston wrote: > On Sat, Jul 21, 2018 at 2:34 PM, Alexander Korotkov > wrote: >> >> So, as I get from this thread, current patch brings these function very >> close to Oracle behavior. The only divergence found yet is related to >> handling of

Re: [HACKERS] Bug in to_timestamp().

On Sat, Jul 21, 2018 at 2:34 PM, Alexander Korotkov < a.korot...@postgrespro.ru> wrote: > So, as I get from this thread, current patch brings these function very > close to Oracle behavior. The only divergence found yet is related to > handling of unmatched tails of input strings (Oracle throws

Re: [HACKERS] Bug in to_timestamp().

On Sun, Jul 8, 2018 at 12:15 AM Alexander Korotkov < a.korot...@postgrespro.ru> wrote: > On Sat, Jul 7, 2018 at 12:31 AM Tom Lane wrote: > > Alexander Korotkov writes: > > > I tool a look at this patch. It looks good for me. It applies > > > cleanly on last master, builds without warnings,

Re: [HACKERS] Bug in to_timestamp().

On Sat, Jul 7, 2018 at 12:31 AM Tom Lane wrote: > Alexander Korotkov writes: > > I tool a look at this patch. It looks good for me. It applies > > cleanly on last master, builds without warnings, passes tests. > > Functionality seems to be well-covered by documentation and regression > >

Re: [HACKERS] Bug in to_timestamp().

Alexander Korotkov writes: > I tool a look at this patch. It looks good for me. It applies > cleanly on last master, builds without warnings, passes tests. > Functionality seems to be well-covered by documentation and regression > tests. So, I'm going to commit it if no objections. AFAIR, the

Re: [HACKERS] Bug in to_timestamp().

On Fri, Mar 2, 2018 at 3:52 PM Arthur Zakirov wrote: > > On Fri, Mar 02, 2018 at 01:44:53PM +0100, Dmitry Dolgov wrote: > > Well, I'm not sure that it's completely unrelated, but I see your point - it > > makes sense to discuss this and move forward in small steps. So, if there > > are > > no

Re: [HACKERS] Bug in to_timestamp().

On Fri, Mar 02, 2018 at 01:44:53PM +0100, Dmitry Dolgov wrote: > Well, I'm not sure that it's completely unrelated, but I see your point - it > makes sense to discuss this and move forward in small steps. So, if there are > no objections I'll mark this patch as ready. Thank you! -- Arthur

Re: [HACKERS] Bug in to_timestamp().

> On 2 March 2018 at 10:33, Arthur Zakirov wrote: > > The fix you proposed isn't related with the initial report [1] by Amul > Sul, IMHO. The patch tries to fix that behaviour and additionally tries > to be more smart in handling and matching separator characters within

Re: [HACKERS] Bug in to_timestamp().

On Fri, Mar 02, 2018 at 12:58:57AM +0100, Dmitry Dolgov wrote: > > On 18 February 2018 at 18:49, Dmitry Dolgov <9erthali...@gmail.com> wrote: > > > On 17 February 2018 at 10:02, Arthur Zakirov > wrote: > > > ... > > > I think it could be fixed by another patch. But I'm

Re: [HACKERS] Bug in to_timestamp().

> On 18 February 2018 at 18:49, Dmitry Dolgov <9erthali...@gmail.com> wrote: > > On 17 February 2018 at 10:02, Arthur Zakirov wrote: > > > On Wed, Feb 14, 2018 at 05:23:53PM +0100, Dmitry Dolgov wrote: > > > > > > SELECT to_timestamp('2000 + JUN', ' /') FROM dual > > >

Re: [HACKERS] Bug in to_timestamp().

> On 17 February 2018 at 10:02, Arthur Zakirov wrote: > > I think it could be fixed by another patch. But I'm not sure that it > will be accepted as well as this patch :). Why do you think there should be another patch for it?

Re: [HACKERS] Bug in to_timestamp().

On Wed, Feb 14, 2018 at 05:23:53PM +0100, Dmitry Dolgov wrote: > * On line 52 there is a removed empty line, without any other changes around Agree, it is unnecessary change. There was the macro there before. > * On line 177 there is a new commented out line of code. I assume it's not > an >

Re: [HACKERS] Bug in to_timestamp().

> On 12 February 2018 at 12:49, Arthur Zakirov wrote: > > Yes, I somehow missed it. I changed the behaviour of separator > characters in format string. They are greedy now and eat whitespaces > before a separator character in an input string. I suppose that there >

Re: [HACKERS] Bug in to_timestamp().

Hello, On Fri, Feb 09, 2018 at 04:31:14PM +0100, Dmitry Dolgov wrote: > I went through this thread, and want to summarize a bit: > > From what I see this patch addresses most important concerns that were > mentioned in the thread, i.e. to make `to_timestamp` less confusing and be > close to

Re: [HACKERS] Bug in to_timestamp().

> On 7 February 2018 at 22:51, Dmitry Dolgov <9erthali...@gmail.com> wrote: >> On 6 February 2018 at 10:17, Arthur Zakirov wrote: >> It is strange. I still can apply both v9 [1] and v10 [2] via 'git >> apply'. And Patch Tester [3] says that it is applied. But maybe >> it

Re: [HACKERS] Bug in to_timestamp().

> On 6 February 2018 at 10:17, Arthur Zakirov wrote: > It is strange. I still can apply both v9 [1] and v10 [2] via 'git > apply'. And Patch Tester [3] says that it is applied. But maybe > it is because of my git (git version 2.16.1). > > You can try also 'patch -p1':

Re: [HACKERS] Bug in to_timestamp().

On Fri, Feb 02, 2018 at 09:54:45PM +0100, Dmitry Dolgov wrote: > For some reason I can't apply it clean to the latest master: > > (Stripping trailing CRs from patch; use --binary to disable.) > patching file doc/src/sgml/func.sgml > (Stripping trailing CRs from patch; use --binary to

Re: [HACKERS] Bug in to_timestamp().

> On 1 February 2018 at 11:24, Arthur Zakirov wrote: > > I fixed those messages, but in a different manner. I think that an > unexpected character is unknown and neither space nor separator. And > better to say that was expected space/separator character. Sounds good,

Re: [HACKERS] Bug in to_timestamp().

On Wed, Jan 31, 2018 at 11:53 AM, Dmitry Dolgov <9erthali...@gmail.com> wrote: > Why not write `is_separator_char` using `isprint`, `isalpha`, `isdigit` from > ctype.h? Something like: > > return isprint(*str) && !isalpha(*str) && !isdigit(*str) > > From what I see in the source code they do

Re: [HACKERS] Bug in to_timestamp().

On Wed, Jan 31, 2018 at 05:53:29PM +0100, Dmitry Dolgov wrote: > Thanks for working on that! I haven't followed this thread before, and after a > quick review I have few side questions. Thank you for your comments! > Why not write `is_separator_char` using `isprint`, `isalpha`, `isdigit` from >

Re: [HACKERS] Bug in to_timestamp().

> On 12 January 2018 at 13:58, Arthur Zakirov wrote: > > I attached new version of the patch. Now (expected output): > Thanks for working on that! I haven't followed this thread before, and after a quick review I have few side questions. Why not write

Re: [HACKERS] Bug in to_timestamp().

Hello, On Fri, Jan 12, 2018 at 02:48:40PM +1300, Thomas Munro wrote: > > I'm guessing that commit 11b623dd0a2c385719ebbbdd42dd4ec395dcdc9d had > something to do with the following failure, when your patch is > applied: > > horology ... FAILED > Thank you a lot for

Re: [HACKERS] Bug in to_timestamp().

On Thu, Nov 23, 2017 at 4:23 AM, Arthur Zakirov wrote: > I've attached new version of the patch. It is a little bit simpler now than > the previous one. > The patch doesn't handle backslashes now, since there was a commit which > fixes quoted-substring handling

Re: [HACKERS] Bug in to_timestamp().

On Thu, Nov 30, 2017 at 10:39:56AM -0500, Robert Haas wrote: > > So is it now the case that all of the regression test cases in this > patch produce the same results as they would on Oracle? > Yes, exactly. All new cases give the same result as in Oracle, except text of error messages. --

Re: [HACKERS] Bug in to_timestamp().

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

Re: [HACKERS] Bug in to_timestamp().

Artur Zakirov writes: > [ 0001-to-timestamp-format-checking-v7.patch ] 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