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

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to