Re: Allow escape in application_name

At Fri, 10 Sep 2021 04:33:53 +0000, "kuroda.hayato@fujitsu.com" <kuroda.hayato@fujitsu.com> wrote in 
> Dear Hou,
> 
> > I found one minor thing in the patch.
> 
> Thanks!
> 
> > Is it better to use Abs(padding) here ?
> > Although some existing codes are using " padding > 0 ? padding : -padding ".
> 
> +1, fixed.
> 
> And I found that some comments were not added above padding calculation,
> so added.

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?


+		/*
+		 * Search application_name and replace it if found.
+		 *
+		 * We search paramters from the end because the later
+		 * one have higher priority.  See also the above comment.
+		 */
+		for (i = n - 1; i >= 0; i--)
+		{
+			if (strcmp(keywords[i], "application_name") == 0)
+			{
+				parse_pgfdw_appname(&buf, values[i]);
+				values[i] = buf.data;
+
+				/*
+				 * If the input format is wrong and the string becomes '\0',
+				 * this parameter is no longer used.
+				 * We conitnue searching application_name.
+				 */
+				if (*values[i] != '\0')
+					break;
+			}
+		}

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;

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center