From: | Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> |
---|---|
To: | Jim Nasby <Jim(dot)Nasby(at)bluetreble(dot)com> |
Cc: | PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: proposal: function parse_ident |
Date: | 2016-02-11 07:27:22 |
Message-ID: | CAFj8pRDgfWdXOJfe8+rOZmBDM2YiV35vHgnCJdRckgE3EtZ49w@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
2016-02-10 19:26 GMT+01:00 Jim Nasby <Jim(dot)Nasby(at)bluetreble(dot)com>:
> The following review has been posted through the commitfest application:
> make installcheck-world: tested, passed
> Implements feature: tested, passed
> Spec compliant: not tested
> Documentation: not tested
>
> NEEDS CATVERSION BUMP
>
> I editorialized the docs and some of the comments. In particular, I
> documented behavior of not truncating, and recommended casting to name[] if
> user cares about that. Added a unit test to verify that works. BTW, I saw
> mention in the thread about not truncated spaces, but the function *does*
> truncate them, unless they're inside quotes, where they're legitimate.
>
ok
>
> Also added test for invalid characters.
>
> I think "strict" would be more in line with other uses in code. There are
> currently no other occurrences of 'strictmode' in the code. There are loads
> of references to 'strict', but I didn't go through all of them to see if
> any were used as externally visible function parameter names.
>
I am sorry, I don't understand to this point. You unlike the name of
parameter "strictmode" ? Have you any proposal? Maybe "restrictive" ?
>
> qualname_str is used in exactly 1 place. Either it should be gotten rid
> of, or all the uses of text_to_cstring(qualname) should be changed to
> qualname_str.
>
fixed, qualname_str is used everywhere
>
> I think the code would have been clearer if instead of the big if (*nextp
> == '\"') it did the same "inquote" looping that is done elsewhere, but I
> don't have a strong opinion on it.
>
The almost all code +/- is related to human readable error messages. We can
move some code to separate static functions - read_quoted_ident,
read_unquoted_ident, but there will be some magic about parameters, and the
code will not be much better, than it is now.
>
> The new status of this patch is: Waiting on Author
>
Thank you for your work on documentation.
I am sending updated version of this patch.
Regards
Pavel
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>
Attachment | Content-Type | Size |
---|---|---|
parse_ident-to-text-array-08.patch | text/x-patch | 15.2 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Artur Zakirov | 2016-02-11 08:14:50 | Re: Mac OS: invalid byte sequence for encoding "UTF8" |
Previous Message | Vitaly Burovoy | 2016-02-11 06:44:28 | Re: custom function for converting human readable sizes to bytes |