From: Tom Lane <tgl@sss.pgh.pa.us>
To: Artur Zakirov <a.zakirov@postgrespro.ru>
Cc: amul sul <sulamul@gmail.com>
Cc: Pavel Stehule <pavel.stehule@gmail.com>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: David G. Johnston <david.g.johnston@gmail.com>
Cc: Alex Ignatov <a.ignatov@postgrespro.ru>
Cc: Bruce Momjian <bruce@momjian.us>
Cc: amul sul <sul_amul@yahoo.co.in>
Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>
Subject: Re: Bug in to_timestamp().
Date: Fri, 04 Nov 2016 14:36:37 -0400
Message-ID: <8744.1478284597@sss.pgh.pa.us> (raw)
In-Reply-To: <CAKNkYny-0YJZzz=a9E==UBuiEoMAFV=n6tik7oLbDVtahiW7tA@mail.gmail.com>
References: <1873520224.1784572.1465833145330.JavaMail.yahoo.ref@mail.yahoo.com>
<1873520224.1784572.1465833145330.JavaMail.yahoo@mail.yahoo.com>
<f64563b3-5c3c-6f28-3f73-41b2159417bc@postgrespro.ru>
<884791796.213185.1466667686999.JavaMail.yahoo@mail.yahoo.com>
<20160623133001.GF21246@momjian.us>
<1877e60a-a970-18b5-453c-e342f96150e6@postgrespro.ru>
<CAKFQuwZKtS0Ad=a1E9_mkdYwLMq+JA3YjmhCvjJ1JF+3BU8a0g@mail.gmail.com>
<d469b177-fd4d-5a5b-769e-620957ec1a64@postgrespro.ru>
<CAKFQuwb2gp8n2ai_agyJod5BTYxL7ek0AjB9zifDSceK6QntnQ@mail.gmail.com>
<CA+TgmoaWTgbtd2RTwVvySMVoOcHmWq66rCCxtVywOOX3FJy4tQ@mail.gmail.com>
<31915.1466703655@sss.pgh.pa.us>
<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>
<CAAJ_b96+G2F_FYvEmwb9b+0TQd9L! aMeWNFVhZSMxXCqHO1m8KQ@mail.gmail.com>
<CAKNkYny-0YJZzz=a9E==UBuiEoMAFV=n6tik7oLbDVtahiW7tA@mail.gmail.com>
List-Unsubscribe: <mailto:majordomo@postgresql.org?body=unsub%20pgsql-hackers>
Artur Zakirov <a.zakirov@postgrespro.ru> writes:
> I attached new version of the patch, which fix is_char_separator()
> declaration too.
I did some experimenting using
http://rextester.com/l/oracle_online_compiler
It appears that Oracle will consider a single space in the pattern
to match zero or more spaces in the input, as all of these produce
the expected result:
SELECT to_timestamp('2000JUN', 'YYYY MON') FROM dual
SELECT to_timestamp('2000 JUN', 'YYYY MON') FROM dual
SELECT to_timestamp('2000 JUN', 'YYYY MON') FROM dual
Also, a space in the pattern will match a single separator character
in the input, but not multiple separators:
SELECT to_timestamp('2000-JUN', 'YYYY MON') FROM dual
-- ok
SELECT to_timestamp('2000--JUN', 'YYYY MON') FROM dual
ORA-01843: not a valid month
And you can have whitespace along with that single separator:
SELECT to_timestamp('2000 + JUN', 'YYYY MON') FROM dual
-- ok
SELECT to_timestamp('2000 + JUN', 'YYYY MON') FROM dual
-- ok
SELECT to_timestamp('2000 ++ JUN', 'YYYY MON') FROM dual
ORA-01843: not a valid month
You can have leading whitespace, but not leading separators:
SELECT to_timestamp(' 2000 JUN', 'YYYY MON') FROM dual
-- ok
SELECT to_timestamp('/2000 JUN', 'YYYY MON') FROM dual
ORA-01841: (full) year must be between -4713 and +9999, and not be 0
These all work:
SELECT to_timestamp('2000 JUN', 'YYYY/MON') FROM dual
SELECT to_timestamp('2000JUN', 'YYYY/MON') FROM dual
SELECT to_timestamp('2000/JUN', 'YYYY/MON') FROM dual
SELECT to_timestamp('2000-JUN', 'YYYY/MON') FROM dual
but not
SELECT to_timestamp('2000//JUN', 'YYYY/MON') FROM dual
ORA-01843: not a valid month
SELECT to_timestamp('2000--JUN', 'YYYY/MON') FROM dual
ORA-01843: not a valid month
which makes it look a lot like Oracle treats separator characters in the
pattern the same as spaces (but I haven't checked their documentation to
confirm that).
The proposed patch doesn't seem to me to be trying to follow
these Oracle behaviors, but I think there is very little reason for
changing any of this stuff unless we move it closer to Oracle.
Some other nitpicking:
* I think the is-separator function would be better coded like
static bool
is_separator_char(const char *str)
{
/* ASCII printable character, but not letter or digit */
return (*str > 0x20 && *str < 0x7F &&
!(*str >= 'A' && *str <= 'Z') &&
!(*str >= 'a' && *str <= 'z') &&
!(*str >= '0' && *str <= '9'));
}
The previous way is neither readable nor remarkably efficient, and it
knows much more about the ASCII character set than it needs to.
* Don't forget the cast to unsigned char when using isspace() or other
<ctype.h> functions.
* I do not see the reason for throwing an error here:
+ /* Previous character was a backslash */
+ if (in_backslash)
+ {
+ /* After backslash should go non-space character */
+ if (isspace(*str))
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("invalid escape sequence")));
+ in_backslash = false;
Why shouldn't backslash-space be a valid quoting sequence?
I'll set this back to Waiting on Author.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, a.zakirov@postgrespro.ru, sulamul@gmail.com, pavel.stehule@gmail.com, robertmhaas@gmail.com, david.g.johnston@gmail.com, a.ignatov@postgrespro.ru, bruce@momjian.us, sul_amul@yahoo.co.in
Subject: Re: Bug in to_timestamp().
In-Reply-To: <8744.1478284597@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox