2016-09-29 13:54 GMT+05:00 amul sul <sulamul@gmail.com>:
>
> On Thu, Sep 29, 2016 at 2:48 AM, Tom Lane <tgl@sss.pgh.pa.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
> commit.
>
> 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
is_char_separator() declaration.
from:
static bool is_char_separator(char *str);
to:
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()
declaration too.
Sorry for confusing!
--
Artur Zakirov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company
