Re: Allow escape in application_name

On 2021/09/21 15:08, kuroda.hayato@fujitsu.com wrote:
> Dear Fujii-san, Horiguchi-san,
> 
> Based on your advice, I made a patch
> that communize two parsing functions into one.
> new internal function parse_format_string() was added.
> (This name may be too generic...)
> log_line_prefix() and parse_pgfdw_appname() become just the wrapper function.
> My prerpimary checking was passed for server and postgres_fdw,
> but could you review that?

Yes. Thanks for updating the patch!

-------------------
elog.c:2785:14: warning: expression which evaluates to zero treated as a null pointer constant of type 'char *'
[-Wnon-literal-null-conversion]
                         *endptr = '\0';
                                   ^~~~
1 warning generated.
-------------------

I got the above compiler warning.


+ * Note: StringInfo datatype cannot be accepted
+ * because elog.h should not include postgres-original header file.

How about moving the function to guc.c from elog.c because it's for
the parameters, i.e., log_line_prefix and postgres_fdw.application_name?
This allows us to use StringInfo in the function?


+                parse_pgfdw_appname(buf, values[i]);
+                /*
+                 * Note that appname may becomes an empty string
+                 * if an input string has wrong format.
+                 */
+                values[i] = *buf;

If postgres_fdw.application_name contains only invalid escape characters like
"%b", parse_pgfdw_appname() returns an empty string. We discussed
there are four options to handle this case and we concluded (4) is better.
Right? But ISTM that the patch uses (2).

> (1) Use the original postgres_fdw.application_name like "%b"
> (2) Use the application_name of the server object (if set)
> (3) Use fallback_application_name
> (4) Use empty string as application_name

Regards,

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