RE: Allow escape in application_name

Dear Horiguchi-san,

Thank you for reviewing! I attached the fixed version.

> Is "the previous comment" "the comment above"?

Yeah, fixed.

> +        for (i = n -1; i >= 0; i--)
>
> You might want a space between - and 1.

Fixed.

> +parse_application_name(StringInfo buf, const char *name)
>
> The name seems a bit too generic as it is a function only for
> postgres_fdw.

Indeed. I renamed to parse_pgfdw_appname().

> +        /* must be a '%', so skip to the next char */
> +        p++;
> +        if (*p == '\0')
> +            break;                /* format error -
> ignore it */
>
> I'm surprised by finding that undefined %-escapes and stray % behave
> differently between archive_command and log_line_prefix. I understand
> this behaves like the latter.

Indeed. pgarch_archiveXlog() treats undefined escapes as nothing special,
but log_line_prefix() stop parsing immediately.
They have no description about it in docs.
I will not treat it in this thread and follow log_line_prefix(),
but I agree it is strange.

> +                    const char *username =
> MyProcPort->user_name;
>
> I'm not sure but even if user_name doesn't seem to be NULL, don't we
> want to do the same thing with %u of log_line_prefix for safety?
> Namely, setting [unknown] if user_name is NULL or "". The same can be
> said for %d.

I think they will be never NULL in current implementation,
but your suggestion is better. Checks were added in %a, %u and %d.

> + * process_padding --- helper function for processing the format
> + * string in log_line_prefix
>
> Since this is no longer a static helper function for a specific
> function, the name and the comment should be more descriptive.
>
> That being said, in the first place the function seems reducible
> almost to a call to atol. By a quick measurement the function is about
> 38% faster (0.024us/call(the function) vs 0.039us/call(strtol) so I'm
> not sure we want to replace process_log_prefix_padding(), but we don't
> need to reuse the function in parse_application_name since it is
> called only once per connection.

My first impression is that they use the function
because they want to know the end of sequence and do error-handling,
but I agree this can be replaced by strtol().
I changed the function to strtol() and restored process_log_prefix_padding().
How do you think? Does it follow your suggestion?

Best Regards,
Hayato Kuroda
FUJITSU LIMITED