Dear Fujii-san,
Thank you for reviewing! I'll post the latest version.
> Currently StringInfo variable is defined in connect_pg_server() and passed to
> parse_pgfdw_appname(). But there is no strong reason why the variable needs to
> be defined connect_pg_server(). Right? If so how about refactoring
> parse_pgfdw_appname() so that it defines StringInfoData variable and returns the
> processed character string? You can see such coding at, for example,
> escape_param_str() in dblink.c.
> If this refactoring is done, probably we can get rid of "#include "lib/stringinfo.h""
> line from connection.c because connect_pg_server() no longer needs to use
> StringInfo.
I refactored that. Thanks!
> + int i;
> <snip>
> + for (i = n - 1; i >= 0; i--)
>
> I'm tempted to simplify these into "for (int i = n - 1; i >= 0; i--)".
>
> + /*
> + * Search application_name and replace it if found.
> + *
> + * We search paramters from the end because the later
> + * one have higher priority. See also the above comment.
> + */
>
> How about updating these comments into the following?
>
> -----------------------
> Search the parameter arrays to find application_name setting,
> and replace escape sequences in it with status information if found.
> The arrays are searched backwards because the last value
> is used if application_name is repeatedly set.
Prefer yours. Fixed.
> + if (strcmp(buf->data, "") != 0)
>
> Is this condition the same as "data[0] == '\0'"?
Maybe "data[0] != '\0'", but fixed.
> + * parse postgres_fdw.application_name and set escaped string.
> + * This function is almost same as log_line_prefix(), but
> + * accepted escape sequence is different and this cannot understand
> + * padding integer.
> + *
> + * Note that argument buf must be initialized.
>
> How about updating these comments into the following?
> I thought that using the term "parse" was a bit confusing because what the
> function actually does is to replace escape seequences in application_name.
> Probably it's also better to rename the function, e.g., to process_pgfdw_appname .
>
> -----------------------------
> Replace escape sequences beginning with % character in the given
> application_name with status information, and return it.
> -----------------------------
Thank you for suggestions. I confused the word "parse."
Fixed and renamed.
> + Same as <xref linkend="guc-log-line-prefix"/>, this is a
> + <function>printf</function>-style string. Accepted escapes are
> + bit different from <xref linkend="guc-log-line-prefix"/>,
> + but padding can be used like as it.
>
> This description needs to be updated.
I missed there. Fixed.
Best Regards,
Hayato Kuroda
FUJITSU LIMITED