Hi Daniel,
Thanks for taking care of this patch.
On Sep 25, 2025, at 15:46, Daniel Gustafsson <daniel@yesql.se> wrote:
On 23 Sep 2025, at 09:50, Chao Li <li.evan.chao@gmail.com> wrote:
Again, found an incorrect function comment of stringToNodeInternal(). Wrong function name is put to the comment, I guess it was from a copy/paste error.
I think this is intentional, and the proposed change would not improve it. The
comment refers to the externally visible symbols stringToNodeWithLocations
(when enabled) and stringToNode The stringToNodeInternal function is just an
implementation detail of it, hence the comment further down on the actual
stringToNode() function being "Externally visible entry points".
I searched for similar cases:
This one just uses the right function name in comment:
/*
* CommitTransactionCommandInternal - a function doing an iteration of work
* regarding handling the commit transaction command. In the case of
* subtransactions more than one iterations could be required. Returns
* true when no more iterations required, false otherwise.
*/
static bool
CommitTransactionCommandInternal(void)
{
This one say “body of” the actual function:
/*
* Body of createTrgmNFA, exclusive of regex compilation/freeing.
*/
static TRGM *
createTrgmNFAInternal(regex_t *regex, TrgmPackedGraph **graph,
MemoryContext rcontext)
{
This one just doesn’t mention function name in the comment:
/*
* Split internal page and insert new data.
*
* Returns new temp pages to *newlpage and *newrpage.
* The original buffer is left untouched.
*/
static void
dataSplitPageInternal(GinBtree btree, Buffer origbuf,
This one does the same ways as stringToNode:
/*
* PQescapeBytea - converts from binary string to the
* minimal encoding necessary to include the string in an SQL
* INSERT statement with a bytea type column as the target.
*
* We can use either hex or escape (traditional) encoding.
* In escape mode, the following transformations are applied:
* '\0' == ASCII 0 == \000
* '\'' == ASCII 39 == ''
* '\\' == ASCII 92 == \\
* anything < 0x20, or > 0x7e ---> \ooo
* (where ooo is an octal expression)
*
* If not std_strings, all backslashes sent to the output are doubled.
*/
static unsigned char *
PQescapeByteaInternal(PGconn *conn,
And this one doesn’t have a function comment:
static int
PQsendQueryInternal(PGconn *conn, const char *query, bool newQuery)
There are 5 different cases, showing that there is not a unique way for what function name should be put to xxInternal() functions’ comment.
Is it deserve to take this opportunity to make all of them in a consistent format?
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
