From: | "kuroda(dot)hayato(at)fujitsu(dot)com" <kuroda(dot)hayato(at)fujitsu(dot)com> |
---|---|
To: | 'Fujii Masao' <masao(dot)fujii(at)oss(dot)nttdata(dot)com> |
Cc: | "'pgsql-hackers(at)lists(dot)postgresql(dot)org'" <pgsql-hackers(at)lists(dot)postgresql(dot)org>, 'Tom Lane' <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Subject: | RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) |
Date: | 2021-09-02 09:27:09 |
Message-ID: | TYAPR01MB5866A3CAE041F7B05E2EE3A9F5CE9@TYAPR01MB5866.jpnprd01.prod.outlook.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
Attachment | Content-Type | Size |
---|---|---|
v06_0001_add_application_name_GUC.patch | application/octet-stream | 8.3 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Thomas Munro | 2021-09-02 10:28:09 | Re: stat() vs ERROR_DELETE_PENDING, round N + 1 |
Previous Message | Alvaro Herrera | 2021-09-02 08:56:39 | Re: Column Filtering in Logical Replication |