Re: proposal: function parse_ident

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

In response to

Responses

Browse pgsql-hackers by date

  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