Re: Allow escape in application_name

On 2021/10/15 17:45, kuroda.hayato@fujitsu.com wrote:
> Dear Horiguchi-san,
> 
>> I'm not sure. All of it is a if-story.  z/OS's isdigit is defined as
>> "Test for a decimal digit, as defined in the digit locale source file
>> and in the digit class of the LC_CTYPE category of the current
>> locale.", which I read it as "it accepts multibyte strings" but of
>> course I'm not sure that's correct.
>>
>> On CentOS8, and for Japanese letters, both iswdigit(L'ー') and
>> iswdigit(L'０') return false so, assuming the same character
>> classfication scheme that is implementation dependent, I guess z/OS's
>> isdigit would behave the same way with CentOS's isdigit.
>>
>> Hoever, regardless of the precise behavior,
>>
>>> But, of cause I agreed this is the crazy case.
>>
>> Yes, I meant that that doesn't matter.
> 
> Hmm.. of cause I want to do because I believe this is corner case,
> but we should avoid regression...
> If no one can say it completely some existing methods probably should be used.
> Therefore, I would like to suggest using the export and support functions again.

On second thought, do we really need padding support for application_name
in postgres_fdw? I just thought that when I read the description
"Padding can be useful to aid human readability in log files." in the docs
about log_line_prefix.


+            case 'a':
+                if (MyProcPort)
+                {

I commented that the check of MyProcPort is necessary because background
worker not having MyProcPort may access to the remote server. The example
of such process is the resolver process that Sawada-san was proposing for
atomic commit feature. But the proposal was withdrawn and for now
there seems no such process. If this is true, we can safely remove the check
of MyProcPort? If so, something like Assert(MyProcPort != NULL) may need
to be added, instead.


+                    if (username == NULL || *username == '\0')
+                        username = "[unknown]";

If user name or database name is not set, the name is replaced with
"[unknown]". log_line_prefix needs this because log message may be
output when they have not been set yet, e.g., at early stage of backend
startup. But I wonder if application_name in postgres_fdw actually
need that.. Thought?


+                    if (appname == NULL || *appname == '\0')
+                        appname = "[unknown]";

Do we really want to replace the application name with "[unknown]"
when application_name in the local server is not set? At least for me,
it's intuitive to replace it with empty string in that case,
in postgres_fdw application_name.


The patch now fails to be applied to the master. Could you rebase it?


Regards,

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