RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name)
Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>
From: "kuroda.hayato@fujitsu.com" <kuroda.hayato@fujitsu.com>
To: 'Fujii Masao' <masao.fujii@oss.nttdata.com>
Cc: "'pgsql-hackers@lists.postgresql.org'" <pgsql-hackers@lists.postgresql.org>, 'Tom Lane' <tgl@sss.pgh.pa.us>
Date: 2021-09-02T09:27:09Z
Lists: pgsql-hackers
Attachments
- v06_0001_add_application_name_GUC.patch (application/octet-stream) patch v6
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
Commits
-
postgres_fdw: Add regression test for postgres_fdw.application_name GUC.
- 353aa01687b5 15.0 landed
-
postgres_fdw: Allow postgres_fdw.application_name to include escape sequences.
- 6e0cb3dec10e 15.0 landed
-
doc: Add note about postgres_fdw.application_name.
- 58e2e6eb67fe 15.0 landed
-
postgres_fdw: Revert unstable tests for postgres_fdw.application_name.
- 98dbef90eb29 15.0 landed
-
postgres_fdw: Allow application_name of remote connection to be set via GUC.
- 449ab6350526 15.0 landed