Dear Fujii-san, Thank you for reviewing! I'll post the latest version.
> Currently StringInfo variable is defined in connect_pg_server() and passed to > parse_pgfdw_appname(). But there is no strong reason why the variable needs to > be defined connect_pg_server(). Right? If so how about refactoring > parse_pgfdw_appname() so that it defines StringInfoData variable and returns > the > processed character string? You can see such coding at, for example, > escape_param_str() in dblink.c. > If this refactoring is done, probably we can get rid of "#include > "lib/stringinfo.h"" > line from connection.c because connect_pg_server() no longer needs to use > StringInfo. I refactored that. Thanks! > + int i; > <snip> > + for (i = n - 1; i >= 0; i--) > > I'm tempted to simplify these into "for (int i = n - 1; i >= 0; i--)". > > + /* > + * 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. > + */ > > How about updating these comments into the following? > > ----------------------- > Search the parameter arrays to find application_name setting, > and replace escape sequences in it with status information if found. > The arrays are searched backwards because the last value > is used if application_name is repeatedly set. Prefer yours. Fixed. > + if (strcmp(buf->data, "") != 0) > > Is this condition the same as "data[0] == '\0'"? Maybe "data[0] != '\0'", but fixed. > + * parse postgres_fdw.application_name and set escaped string. > + * This function is almost same as log_line_prefix(), but > + * accepted escape sequence is different and this cannot understand > + * padding integer. > + * > + * Note that argument buf must be initialized. > > How about updating these comments into the following? > I thought that using the term "parse" was a bit confusing because what the > function actually does is to replace escape seequences in application_name. > Probably it's also better to rename the function, e.g., to > process_pgfdw_appname . > > ----------------------------- > Replace escape sequences beginning with % character in the given > application_name with status information, and return it. > ----------------------------- Thank you for suggestions. I confused the word "parse." Fixed and renamed. > + Same as <xref linkend="guc-log-line-prefix"/>, this is a > + <function>printf</function>-style string. Accepted escapes are > + bit different from <xref linkend="guc-log-line-prefix"/>, > + but padding can be used like as it. > > This description needs to be updated. I missed there. Fixed. Best Regards, Hayato Kuroda FUJITSU LIMITED
