|From:||Artur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>|
|To:||amul sul <sulamul(at)gmail(dot)com>|
|Cc:||Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Artur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, "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>, amul sul <sul_amul(at)yahoo(dot)co(dot)in>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>|
|Subject:||Re: Bug in to_timestamp().|
|Views:||Raw Message | Whole Thread | Download mbox|
2016-09-29 13:54 GMT+05:00 amul sul <sulamul(at)gmail(dot)com>:
> On Thu, Sep 29, 2016 at 2:48 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> > I started looking at your 0001-to-timestamp-format-checking-v4.patch
> > and this point immediately jumped out at me. Currently the code relies
> > ... without any documentation ... on no elog being thrown out of
> > parse_format(). That's at the very least trouble waiting to happen.
> > There's a hack to deal with errors from within the NUMDesc_prepare
> > subroutine, but it's a pretty ugly and underdocumented hack. And what
> > you had here was randomly different from that solution, too.
> > After a bit of thought it seemed to me that a much cleaner fix is to add
> > a "valid" flag to the cache entries, which we can leave clear until we
> > have finished parsing the new format string. That avoids adding extra
> > data copying as you suggested, removes the need for PG_TRY, and just
> > generally seems cleaner and more bulletproof.
> > I've pushed a patch that does it that way. The 0001 patch will need
> > to be rebased over that (might just require removal of some hunks,
> > not sure).
> > I also pushed 0002-to-timestamp-validation-v2.patch with some revisions
> > (it'd broken acceptance of BC dates, among other things, but I think
> > I fixed everything).
Thank you for committing the 0002 part of the patch! I wanted to fix
cache functions too, but wasn't sure about that.
> > Since you told us earlier that you'd be on vacation through the end of
> > September, I'm assuming that nothing more will happen on this patch during
> > this commitfest, so I will mark the CF entry Returned With Feedback.
> Behalf of Artur I've rebased patch, removed hunk dealing with broken
> cache entries by copying it, which is no longer required after 83bed06
> Commitfest status left untouched, I am not sure how to deal with
> "Returned With Feedback" patch. Is there any chance that, this can be
> considered again for committer review?
Thank you for fixing the patch!
Today I have access to the internet and able to fix and test the
patch. I've looked at your 0001-to-timestamp-format-checking-v5.patch.
It seems nice to me. I suppose it is necessary to fix
static bool is_char_separator(char *str);
static bool is_char_separator(const char *str);
Because now in parse_format() *str argument is const.
I attached new version of the patch, which fix is_char_separator()
Sorry for confusing!
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company
|Next Message||Heikki Linnakangas||2016-09-29 10:17:53||Re: less expensive pg_buffercache on big shmem|
|Previous Message||Heikki Linnakangas||2016-09-29 09:49:27||Re: Tuplesort merge pre-reading|