Dear Fujii-san, Thanks for 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.. you mean that this is not strtoXXX, right? If so we should go back to process_padding().... Horiguchi-san, do you have any idea? And I added pg_restrict keywords for compiler optimizations. > >> 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? I agreed that (2) and (3) breaks the rule which should override server option. Hence I chose (4), values[i] substituted to buf.data in any case. Attached is the latest version. Best Regards, Hayato Kuroda FUJITSU LIMITED
v16_0002_allow_escapes.patch
Description: v16_0002_allow_escapes.patch
