Re: Allow escape in application_name

On 2021/12/07 18:48, kuroda.hayato@fujitsu.com wrote:
> Yeah, I followed your suggestion. But we deiced to keep codes clean,
> hence I removed the if-statements.

+1 because neither application_name, user_name nor database_name should be NULL for current usage. But if it's better
tocheck whether they are NULL or not for some reasons or future usage, e.g., background worker not logging into any
specifieddatabase may set application_name to '%d' and connect to a foreign server in the future, let's change the code
laterwith comments.
 

Thanks for updating the patch!

Regarding 0001 patch, IMO it's better to explain that postgres_fdw.application_name can be any string of any length and
containeven non-ASCII characters, unlike application_name. So how about using the following description, instead?
 

-----------------
<varname>postgres_fdw.application_name</varname> can be any string of any length and contain even non-ASCII characters.
Howeverwhen it's passed to and used as <varname>application_name</varname> in a foreign server, note that it will be
truncatedto less than <symbol>NAMEDATALEN</symbol> characters and any characters other than printable ASCII ones in it
willbe replaced with question marks (<literal>?</literal>).
 
-----------------

+        int            i;
+        for (i = n - 1; i >= 0; i--)

As I told before, it's better to simplify this to "for (int i = n - 1; i >= 0; i--)".

Seems you removed the calls to pfree() from the patch. That's because the memory context used for the replaced
application_namestring will be released soon? Or it's better to call pfree()?
 

+      Same as <xref linkend="guc-log-line-prefix"/>, this is a
+      <function>printf</function>-style string. Unlike <xref linkend="guc-log-line-prefix"/>,
+      paddings are not allowed. Accepted escapes are as follows:

Isn't it better to explain escape sequences in postgres_fdw.application_name more explicitly, as follows?

-----------------
<literal>%</literal> characters begin <quote>escape sequences</quote> that are replaced with status information as
outlinedbelow. Unrecognized escapes are ignored. Other characters are copied straight to the application name. Note
thatit's not allowed to specify a plus/minus sign or a numeric literal after the <literal>%</literal> and before the
option,for alignment and padding.
 
-----------------

+         <entry><literal>%u</literal></entry>
+         <entry>Local user name</entry>

Average users can understand what "Local" here means?

+      postgres_fdw.application_name is set to application_name of a pgfdw
+      connection after placeholder conversion, thus the resulting string is
+      subject to the same restrictions alreadby mentioned above.

This description doesn't need to be added if 0001 patch is applied, is it? Because 0001 patch adds very similar
descriptioninto the docs.
 

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION