Re: proposal: function parse_ident

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

In response to

Responses

Browse pgsql-hackers by date

  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