From: | Artur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru> |
---|---|
To: | Robert Haas <robertmhaas(at)gmail(dot)com>, amul sul <sul_amul(at)yahoo(dot)co(dot)in> |
Cc: | Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com>, Alex Ignatov <a(dot)ignatov(at)postgrespro(dot)ru>, Bruce Momjian <bruce(at)momjian(dot)us>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Bug in to_timestamp(). |
Date: | 2016-08-24 17:42:25 |
Message-ID: | 03e76bef-75eb-4d70-40d6-890d42f4dc8e@postgrespro.ru |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Sorry. I did not get last two mails from Amul. Don't know why. So I
reply to another mail.
> Documented as working case, but unfortunatly it does not :
>
> postgres=# SELECT to_timestamp('2000----JUN', 'YYYY MON');
> ERROR: invalid value "---" for "MON"
> DETAIL: The given value did not match any of the allowed values for this field.
Indeed! Fixed it. Now this query executes without error. Added this case
to tests.
> NODE_TYPE_CHAR assumption in else block seems incorrect. What if we have space after double quote? It will again have incorrect output without any error, see below:
>
> postgres=# SELECT to_timestamp('Year: 1976, Month: May, Day: 16',
> postgres(# '" Year:" YYYY, "Month:" FMMonth, "Day:" DD');
> to_timestamp
> ------------------------------
> 0006-05-16 00:00:00-07:52:58
> (1 row)
>
> I guess, we might need NODE_TYPE_SEPARATOR and NODE_TYPE_SPACE check as well?
Agree. Fixed and added to tests.
> Unnecessary hunk?
> We should not touch any code unless it necessary to implement proposed feature, otherwise it will add unnecessary diff to the patch and eventually extra burden to review the code. Similarly hunk in the patch @ line # 313 - 410, nothing to do with to_timestamp behaviour improvement, IIUC.
>
> If you think this changes need to be in, please submit separate cleanup-patch.
Fixed it. It was a typo.
About lines # 313 - 410. It is necessary to avoid bugs. I wrote aboud it
in
https://www.postgresql.org/message-id/b2a39359-3282-b402-f4a3-057aae500ee7%40postgrespro.ru
:
> - now DCH_cache_getnew() is called after parse_format(). Because now
> parse_format() can raise an error and in the next attempt
> DCH_cache_search() could return broken cache entry.
For example, you can test incorrect inputs for to_timestamp(). Try to
execute such query several times.
--
Artur Zakirov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company
Attachment | Content-Type | Size |
---|---|---|
0001-to-timestamp-format-checking-v3.patch | text/x-patch | 18.3 KB |
0002-to-timestamp-validation-v2.patch | text/x-patch | 6.3 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Kevin Grittner | 2016-08-24 18:00:40 | Re: [COMMITTERS] pgsql: Modify BufferGetPage() to prepare for "snapshot too old" feature |
Previous Message | Alvaro Herrera | 2016-08-24 17:40:17 | Re: [COMMITTERS] pgsql: Modify BufferGetPage() to prepare for "snapshot too old" feature |