RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name) - Mailing list pgsql-hackers

From kuroda.hayato@fujitsu.com
Subject RE: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name)
Date
Msg-id TYAPR01MB5866A3CAE041F7B05E2EE3A9F5CE9@TYAPR01MB5866.jpnprd01.prod.outlook.com
Whole thread Raw
In response to Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name)  (Fujii Masao <masao.fujii@oss.nttdata.com>)
Responses Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name)  (Fujii Masao <masao.fujii@oss.nttdata.com>)
List 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

pgsql-hackers by date:

Previous
From: Alvaro Herrera
Date:
Subject: Re: Column Filtering in Logical Replication
Next
From: Thomas Munro
Date:
Subject: Re: stat() vs ERROR_DELETE_PENDING, round N + 1