On 2021/09/21 15:08, kuroda.hay...@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
