Re: Allow escape in application_name

On 2021/09/14 13:42, kuroda.hayato@fujitsu.com wrote:
> Dear Horiguchi-san,
> 
> Thank you for giving comments!
> 
>> Thanks for the new version.  I don't object for reusing
>> process_log_prefix_padding, but I still find it strange that the
>> function with the name 'process_padding' is in string.c.  If we move
>> it to string.c, I'd like to name it "pg_fast_strtoi" or something and
>> change the interface to int pg_fast_strtoi(const char *p, char
>> **endptr) that is (roughly) compatible to strtol.  What do (both) you
>> think about this?
> 
> I agree that this interface might be confused.
> I changed its name and interface. How do you think?
> Actually I cannot distinguish the name is good or not,
> but I could not think of anything else...

The name using the word "strtoint" sounds confusing to me
because the behavior of the function is different from strtoint() or
pg_strtoint32(), etc. Otherwise we can easily misunderstand that
pg_fast_strtoint() can be used as alternative of strtoint() or
pg_strtoint32(). I have no better idea for the name, though..


> 
>> I didn't fully checked in what case parse_pgfdw_appname gives "" as
>> result, I feel that we should use the original value in that
>> case. That is,
>>
>>>   parse_pgfdw_appname(&buf, vaues[i]);
>>>
>>>   /* use the result if any, otherwise use the original string */
>>>   if (buf.data[0] != 0)
>>>      values[i] = buf.data;
>>>
>>>   /* break if it results in non-empty string */
>>>   if (values[0][0] != 0)
>>>      break;

Agreed. It's strange to use the application_name of the server
object in that case. There seems to be four options:

(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

(2) and (3) look strange to me because we expect that
postgres_fdw.application_name should override application_name
of the server object and fallback_application_mame.

Also reporting invalid escape sequence in application_name as it is,
i.e., (1), looks strange to me.

So (4) looks most intuitive and similar behavior to log_line_prefix.
Thought?

Regards,

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