Re: Allow escape in application_name

At Sun, 7 Nov 2021 13:35:39 +0900, Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com> wrote in
>
>
> On 2021/11/05 12:17, Kyotaro Horiguchi wrote:
> If possible, I'd like to see this change as a separate patch
> and commt it first because this is the description for
> the existing parameter postgres_fdw.application_name.

Fair enough.

> >> I'd like to hear more opinions about this from others.
> >> But *if* there is actually no use case, I'd like not to add
> >> the feature, to make the code simpler.
> > I think padding is useful because it alingns the significant content
> > of log lines by equating the length of the leading fixed
> > In short, I'm for to removing it by +0.7.
>
> So our current consensus is to remove the padding part
> from postgres_fdw.application_name.

I think so.

> >> + case 'u':
> >> + Assert(MyProcPort != NULL);
> >>
> >> Isn't it enough to perform this assertion check only once
> >> at the top of parse_pgfdw_appname()?
> > Yeah, in either way, we should treat them in the same way.
> >
> >>> We can force parse_pgfdw_appname() not to be called if MyProcPort does
> >>> not exist,
> >>> but I don't think it is needed now.
> >>
> >> Yes.
> > (I assume you said "it is needed now".) I'm not sure how to force
> > that but if it means a NULL MyProcPort cuases a crash, I think
> > crashing server is needlessly too aggressive as the penatly.
>
> I said "Yes" for Kuroda-san's comment "I don't think it is
> needed now". So I meant that "it is NOT needed now".
> Sorry for unclear comment..
>
> His idea was to skip calling parse_pgfdw_appname() if
> MyProcPort is NULL, so as to prevent parse_pgfdw_appname()
> from seeing NULL pointer of MyProcPort. But he thought
> it's not necessary now, and I agree with him because
> the idea would cause more confusing behavior.
>
>
> > It seems to me that postgres-fdw asumes a valid user id, but doesn't
> > make no use of databsae, server port, and process id. What I thought
> > here is that making it an assertion is too much. So just ignoring the
> > replacement is also fine to me.
> > That being said, if we are eager not to have unused code paths, it is
> > reasonable enough. I don't object strongly to replace it with an
> > assertion.
>
> So no one strongly objects to the addition of assertion?

It seems to me so.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center