On 2021/12/17 12:06, Kyotaro Horiguchi wrote:
At Fri, 17 Dec 2021 02:42:25 +0000, "kuroda.hay...@fujitsu.com"
<kuroda.hay...@fujitsu.com> wrote in
Dear Fujii-san,
Sorry I forgot replying your messages.
if (strcmp(keywords[i], "application_name") == 0 &&
values[i] != NULL && *(values[i]) != '\0')
I'm not sure but do we have a case that values[i] becomes NULL
even if keywords[i] is "application_name"?
No for now, I guess. But isn't it safer to check that, too? I also could not
find strong
reason why that check should be dropped. But you'd like to drop that?
No, I agreed the new checking. I'm just afraid of my code missing.
FWIW, I don't understand why we care of the case where the function
itself changes its mind to set values[i] to null
Whether we add this check or not, the behavior is the same, i.e., that setting
value is not used. But by adding the check we can avoid unnecessary call of
process_pgfdw_appname() when the value is NULL. OTOH, of course we can also
remove the check. So I'm ok to remove that if you're thinking it's more
consistent to remove that.
while we ignore the
possibility that some module at remote is modified so that some global
variables to be NULL. I don't mind wheter we care for NULLs or not
but I think we should be consistent at least in a single patch.
Probably you're mentioning that we got rid of something like the following code from
process_pgfdw_appname(). In this case whether we add the check or not can affect the
behavior (i.e., escape sequence is replace with "[unknown]" or not). So ISTM
that the situations are similar but not the same.
+ if (appname == NULL || *appname == '\0')
+ appname = "[unknown]";
Probably now it's good chance to revisit this issue. As I commented at [1], at least for
me it's intuitive to use empty string rather than "[unknown]" when appname or
username, etc was NULL or '\0'. To implement this behavior, I argued to remove the check
itself. But there are several options:
#1. use "[unknown]"
#2. add the check but not use "[unknown]"
#3. don't add the check (i.e., what the current patch does)
For now, I'm ok to choose #2 or #3.
[1]
https://postgr.es/m/0dbe50c3-c528-74b1-c577-035a4a68f...@oss.nttdata.com
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
