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