From: | Jim Nasby <Jim(dot)Nasby(at)BlueTreble(dot)com> |
---|---|
To: | pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: proposal: function parse_ident |
Date: | 2016-02-10 18:26:03 |
Message-ID: | 20160210182603.1320.3850.pgcf@coridan.postgresql.org |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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.
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.
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.
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 new status of this patch is: Waiting on Author
From | Date | Subject | |
---|---|---|---|
Next Message | Jim Nasby | 2016-02-10 18:29:49 | Re: proposal: function parse_ident |
Previous Message | Pavel Stehule | 2016-02-10 18:04:28 | Re: custom function for converting human readable sizes to bytes |