RE: Allow escape in application_name

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