2016-09-29 13:54 GMT+05:00 amul sul <sula...@gmail.com>: > > On Thu, Sep 29, 2016 at 2:48 AM, Tom Lane <t...@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
0001-to-timestamp-format-checking-v6.patch
Description: Binary data
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
