Dear Fujii-san, Thank you for reviewing!
> 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? I added the following comments: ```diff - /* Use "postgres_fdw" as fallback_application_name. */ + /* + * Use pgfdw_application_name as application_name. + * + * Note that this check must be behind constructing generic options + * because pgfdw_application_name has higher priority. + */ ``` > 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. I added check_hook because I want to make sure that input for parse function has only ascii characters. But in this phase we don't have such a function, so removed. (Actually I'm not sure it is really needed, so I will check until next phase) > Why is GUC_IS_NAME flag necessary? I thought again and I noticed even if extremely long string is specified it will be truncated at server-side. This check is duplicated so removed. Maybe such a check is a user-responsibility. > postgres_fdw.application_name overrides application_name set in foreign server > object. > Shouldn't this information be documented? I added descriptions to postgres_fdw.sgml. > Isn't it better to have the regression test for this feature? +1, added. In that test SET the parameter and check pg_stat_activity. Attached is the fixed version. 0002 will be rebased later. Best Regards, Hayato Kuroda FUJITSU LIMITED
v06_0001_add_application_name_GUC.patch
Description: v06_0001_add_application_name_GUC.patch