Dear Fujii-san, Thank you for reviewing!
> postgres_fdw gets out of the loop after processing appname even
> when buf.data is '\0'. Is this expected behavior? Because of this,
> when postgres_fdw.application_name = '%b', unprocessed appname
> of the server object is used.
In this case, I expected to use fallback_application_name instead of appname,
but I missed the case where appname was set as a server option.
Maybe we should do continue when buf.data is \0.
> + char *endptr = NULL;
> + padding = (int)strtol(p, &endptr, 10);
>
> strtol() seems to work differently from process_log_prefix_padding(),
> for example, when the input string is "%-p".
Yeah, and I found that "%+5p" is another example.
Our padding function does not understand plus sign
(and maybe this is the specification of printf()), but strtol() reads.
I did not fix here because I lost my way. Do we treats these cases
and continue to use strtol(), or use process_padding()?
> Could you tell me why the statement checking
> application_name should be wrapped in a function?
> Instead, we can just execute something like the following?
>
> SELECT COUNT(*) FROM pg_stat_activity WHERE application_name =
> current_setting('application_name') || current_user || current_database() ||
> pg_backend_pid() || '%';
I did not know current_setting() function...
The function was replaced as you expected.
> When log_line_prefix() processes "%a", "%u" or "%d" characters in
> log_line_prefix, it checks whether MyProcPort is NULL or not.
> Likewise shouldn't parse_pgfdw_appname() do the same thing?
> For now it's ok not to check that because only process having
> MyProcPort can use postgres_fdw. But in the future the process
> not having that may use postgres_fdw, for example, 2PC resolver
> process that Sawada-san is proposing to add to support automatic
> 2PC among multiple remote servers.
Sure, I did not consider about other patches. I added checks.
> + You can use escape sequences for <literal>application_name</literal>
> even if
> + it is set as a connection or a user mapping option.
> + Please refer the later section.
>
> I was thinking that application_name cannot be set in a user mapping.
I confirmed codes and found that is_valid_option() returns false
if libpq options are set. So removed.
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
v12_0002_allow_escapes.patch
Description: v12_0002_allow_escapes.patch
