On 2021/09/01 19:04, kuroda.hay...@fujitsu.com wrote:
OK, I split and attached like that. 0001 adds new GUC, and 0002 allows to accept escapes.
Thanks for splitting and updating the patches! Here are the comments for 0001 patch. - /* Use "postgres_fdw" as fallback_application_name. */ + /* Use GUC paramter if set */ + if (pgfdw_application_name && *pgfdw_application_name != '\0') This GUC parameter should be set after the options of foreign server are set so that its setting can override the server-level ones. Isn't it better to comment this? +static bool +check_pgfdw_application_name(char **newval, void **extra, GucSource source) +{ + /* Only allow clean ASCII chars in the application name */ + if (*newval) + pg_clean_ascii(*newval); + return true; Do we really need this check hook? Even without that, any non-ASCII characters in application_name would be replaced with "?" in the foreign PostgreSQL server when it's passed to that. On the other hand, if it's really necessary, application_name set in foreign server object also needs to be processed in the same way. + NULL, + PGC_USERSET, + GUC_IS_NAME, Why is GUC_IS_NAME flag necessary? postgres_fdw.application_name overrides application_name set in foreign server object. Shouldn't this information be documented? Isn't it better to have the regression test for this feature? Regards, -- Fujii Masao Advanced Computing Technology Center Research and Development Headquarters NTT DATA CORPORATION