From: Fujii Masao <masao.fujii@oss.nttdata.com>
To: kuroda.hayato@fujitsu.com <kuroda.hayato@fujitsu.com>
Cc: 'pgsql-hackers@lists.postgresql.org' <pgsql-hackers@lists.postgresql.org>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Subject: Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name)
Date: Mon, 6 Sep 2021 20:52:45 +0900
Message-ID: <42ea526c-aa4f-bfe2-5547-6fcf8ac6469c@oss.nttdata.com> (raw)
In-Reply-To: <TYAPR01MB58666157D4FF274822A76B3BF5D29@TYAPR01MB5866.jpnprd01.prod.outlook.com>
References: <TYCPR01MB5870D1E8B949DAF6D3B84E02F5F29@TYCPR01MB5870.jpnprd01.prod.outlook.com>
<ed1c8ef2-16ac-4e56-d302-b1ff430baeec@oss.nttdata.com>
<TYAPR01MB58661635D3884E3B863ED01CF5C79@TYAPR01MB5866.jpnprd01.prod.outlook.com>
<TYAPR01MB5866EF9624D2DFA8A74E5E94F5CC9@TYAPR01MB5866.jpnprd01.prod.outlook.com>
<8f985ba0-2e79-ab67-4fa4-d35024188047@oss.nttdata.com>
<TYAPR01MB5866B697888D5568E6D1546BF5CD9@TYAPR01MB5866.jpnprd01.prod.outlook.com>
<0d5df322-c9ad-308e-982b-a3d3275bcdc0@oss.nttdata.com>
<TYAPR01MB5866A3CAE041F7B05E2EE3A9F5CE9@TYAPR01MB5866.jpnprd01.prod.outlook.com>
<2dde164d-bc49-b3ca-0cc1-1c5fe40f0886@oss.nttdata.com>
<TYAPR01MB58669D67A505AD8995BBD2D6F5CF9@TYAPR01MB5866.jpnprd01.prod.outlook.com>
<21aa7786-fce9-5dbf-6b75-519015319724@oss.nttdata.com>
<TYAPR01MB58666157D4FF274822A76B3BF5D29@TYAPR01MB5866.jpnprd01.prod.outlook.com>
On 2021/09/06 10:32, kuroda.hayato@fujitsu.com wrote:
> I think we should SELECT ft6 because foreign server 'loopback'
> doesn't have application_name server option.
Yes. I forgot to update that... Thanks for the review!
Attached is the updated version of the patch.
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
Attachments:v09_0001_add_application_name_GUC.patch (text/plain, 10.0K) downloaddiff --git a/contrib/postgres_fdw/connection.c b/contrib/postgres_fdw/connection.cindex 82aa14a65d..705f60a3ae 100644--- a/contrib/postgres_fdw/connection.c+++ b/contrib/postgres_fdw/connection.c@@ -353,10 +353,11 @@ connect_pg_server(ForeignServer *server, UserMapping *user)
/*
* Construct connection params from generic options of ForeignServer
* and UserMapping. (Some of them might not be libpq options, in
- * which case we'll just waste a few array slots.) Add 3 extra slots- * for fallback_application_name, client_encoding, end marker.+ * which case we'll just waste a few array slots.) Add 4 extra slots+ * for application_name, fallback_application_name, client_encoding,+ * end marker.
*/
- n = list_length(server->options) + list_length(user->options) + 3;+ n = list_length(server->options) + list_length(user->options) + 4;
keywords = (const char **) palloc(n * sizeof(char *));
values = (const char **) palloc(n * sizeof(char *));
@@ -366,7 +367,23 @@ connect_pg_server(ForeignServer *server, UserMapping *user)
n += ExtractConnectionOptions(user->options,
keywords + n, values + n);
- /* Use "postgres_fdw" as fallback_application_name. */+ /*+ * Use pgfdw_application_name as application_name if set.+ *+ * PQconnectdbParams() processes the parameter arrays from start to+ * end. If any key word is repeated, the last value is used. Therefore+ * note that pgfdw_application_name must be added to the arrays after+ * options of ForeignServer are, so that it can override+ * application_name set in ForeignServer.+ */+ if (pgfdw_application_name && *pgfdw_application_name != '\0')+ {+ keywords[n] = "application_name";+ values[n] = pgfdw_application_name;+ n++;+ }++ /* Use "postgres_fdw" as fallback_application_name */
keywords[n] = "fallback_application_name";
values[n] = "postgres_fdw";
n++;
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.outindex e3ee30f1aa..39befa394a 100644--- a/contrib/postgres_fdw/expected/postgres_fdw.out+++ b/contrib/postgres_fdw/expected/postgres_fdw.out@@ -10761,3 +10761,82 @@ ERROR: invalid value for integer option "fetch_size": 100$%$#$#
CREATE FOREIGN TABLE inv_bsz (c1 int )
SERVER loopback OPTIONS (batch_size '100$%$#$#');
ERROR: invalid value for integer option "batch_size": 100$%$#$#
+-- ===================================================================+-- test postgres_fdw.application_name GUC+-- ===================================================================+-- Turn debug_discard_caches off for this test to make that+-- the remote connection is alive when checking its application_name.+-- For each test, close all the existing cached connections manually and+-- establish connection with new setting of application_name.+SET debug_discard_caches = 0;+-- If appname is set as GUC but not as options of server object,+-- the GUC setting is used as application_name of remote connection.+SET postgres_fdw.application_name TO 'fdw_guc_appname';+SELECT 1 FROM postgres_fdw_disconnect_all();+ ?column? +----------+ 1+(1 row)++SELECT 1 FROM ft6 LIMIT 1;+ ?column? +----------+ 1+(1 row)++SELECT application_name FROM pg_stat_activity+ WHERE application_name IN ('loopback2', 'fdw_guc_appname');+ application_name +------------------+ fdw_guc_appname+(1 row)++-- If appname is set as options of server object but not as GUC,+-- appname of server object is used.+RESET postgres_fdw.application_name;+ALTER SERVER loopback2 OPTIONS (ADD application_name 'loopback2');+SELECT 1 FROM postgres_fdw_disconnect_all();+ ?column? +----------+ 1+(1 row)++SELECT 1 FROM ft6 LIMIT 1;+ ?column? +----------+ 1+(1 row)++SELECT application_name FROM pg_stat_activity+ WHERE application_name IN ('loopback2', 'fdw_guc_appname');+ application_name +------------------+ loopback2+(1 row)++-- If appname is set both as GUC and as options of server object,+-- the GUC setting overrides appname of server object and is used.+SET postgres_fdw.application_name TO 'fdw_guc_appname';+SELECT 1 FROM postgres_fdw_disconnect_all();+ ?column? +----------+ 1+(1 row)++SELECT 1 FROM ft6 LIMIT 1;+ ?column? +----------+ 1+(1 row)++SELECT application_name FROM pg_stat_activity+ WHERE application_name IN ('loopback2', 'fdw_guc_appname');+ application_name +------------------+ fdw_guc_appname+(1 row)++--Clean up+ALTER SERVER loopback2 OPTIONS (DROP application_name);+RESET postgres_fdw.application_name;+RESET debug_discard_caches;diff --git a/contrib/postgres_fdw/option.c b/contrib/postgres_fdw/option.cindex c574ca2cf3..5bb1af4084 100644--- a/contrib/postgres_fdw/option.c+++ b/contrib/postgres_fdw/option.c@@ -1,7 +1,7 @@
/*-------------------------------------------------------------------------
*
* option.c
- * FDW option handling for postgres_fdw+ * FDW and GUC option handling for postgres_fdw
*
* Portions Copyright (c) 2012-2021, PostgreSQL Global Development Group
*
@@ -45,6 +45,13 @@ static PgFdwOption *postgres_fdw_options;
*/
static PQconninfoOption *libpq_options;
+/*+ * GUC parameters+ */+char *pgfdw_application_name = NULL;++void _PG_init(void);+
/*
* Helper functions
*/
@@ -435,3 +442,29 @@ ExtractExtensionList(const char *extensionsString, bool warnOnMissing)
list_free(extlist);
return extensionOids;
}
++/*+ * Module load callback+ */+void+_PG_init(void)+{+ /*+ * Unlike application_name GUC, don't set GUC_IS_NAME flag nor check_hook+ * to allow postgres_fdw.application_name to be any string more than+ * NAMEDATALEN characters and to include non-ASCII characters. Instead,+ * remote server truncates application_name of remote connection to less+ * than NAMEDATALEN and replaces any non-ASCII characters in it with a '?'+ * character.+ */+ DefineCustomStringVariable("postgres_fdw.application_name",+ "Sets the application name to be used on the remote server.",+ NULL,+ &pgfdw_application_name,+ NULL,+ PGC_USERSET,+ 0,+ NULL,+ NULL,+ NULL);+}diff --git a/contrib/postgres_fdw/postgres_fdw.h b/contrib/postgres_fdw/postgres_fdw.hindex ca83306af9..90b72e9ec5 100644--- a/contrib/postgres_fdw/postgres_fdw.h+++ b/contrib/postgres_fdw/postgres_fdw.h@@ -158,6 +158,7 @@ extern int ExtractConnectionOptions(List *defelems,
const char **values);
extern List *ExtractExtensionList(const char *extensionsString,
bool warnOnMissing);
+extern char *pgfdw_application_name;
/* in deparse.c */
extern void classifyConditions(PlannerInfo *root,
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sqlindex 30b5175da5..20749868d3 100644--- a/contrib/postgres_fdw/sql/postgres_fdw.sql+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql@@ -3422,3 +3422,42 @@ CREATE FOREIGN TABLE inv_fsz (c1 int )
-- Invalid batch_size option
CREATE FOREIGN TABLE inv_bsz (c1 int )
SERVER loopback OPTIONS (batch_size '100$%$#$#');
++-- ===================================================================+-- test postgres_fdw.application_name GUC+-- ===================================================================+-- Turn debug_discard_caches off for this test to make that+-- the remote connection is alive when checking its application_name.+-- For each test, close all the existing cached connections manually and+-- establish connection with new setting of application_name.+SET debug_discard_caches = 0;++-- If appname is set as GUC but not as options of server object,+-- the GUC setting is used as application_name of remote connection.+SET postgres_fdw.application_name TO 'fdw_guc_appname';+SELECT 1 FROM postgres_fdw_disconnect_all();+SELECT 1 FROM ft6 LIMIT 1;+SELECT application_name FROM pg_stat_activity+ WHERE application_name IN ('loopback2', 'fdw_guc_appname');++-- If appname is set as options of server object but not as GUC,+-- appname of server object is used.+RESET postgres_fdw.application_name;+ALTER SERVER loopback2 OPTIONS (ADD application_name 'loopback2');+SELECT 1 FROM postgres_fdw_disconnect_all();+SELECT 1 FROM ft6 LIMIT 1;+SELECT application_name FROM pg_stat_activity+ WHERE application_name IN ('loopback2', 'fdw_guc_appname');++-- If appname is set both as GUC and as options of server object,+-- the GUC setting overrides appname of server object and is used.+SET postgres_fdw.application_name TO 'fdw_guc_appname';+SELECT 1 FROM postgres_fdw_disconnect_all();+SELECT 1 FROM ft6 LIMIT 1;+SELECT application_name FROM pg_stat_activity+ WHERE application_name IN ('loopback2', 'fdw_guc_appname');++--Clean up+ALTER SERVER loopback2 OPTIONS (DROP application_name);+RESET postgres_fdw.application_name;+RESET debug_discard_caches;diff --git a/doc/src/sgml/postgres-fdw.sgml b/doc/src/sgml/postgres-fdw.sgmlindex 0075bc3dbb..bf95da9721 100644--- a/doc/src/sgml/postgres-fdw.sgml+++ b/doc/src/sgml/postgres-fdw.sgml@@ -905,6 +905,31 @@ postgres=# SELECT postgres_fdw_disconnect_all();
</para>
</sect2>
+ <sect2>+ <title>Configuration Parameters</title>++ <variablelist>+ <varlistentry>+ <term>+ <varname>postgres_fdw.application_name</varname> (<type>string</type>)+ <indexterm>+ <primary><varname>postgres_fdw.application_name</varname> configuration parameter</primary>+ </indexterm>+ </term>+ <listitem>+ <para>+ Specifies a value for <xref linkend="guc-application-name"/>+ configuration parameter used when <filename>postgres_fdw</filename>+ establishes a connection to a foreign server. This overrides+ <varname>application_name</varname> option of the server object.+ Note that change of this parameter doesn't affect any existing+ connections until they are re-established.+ </para>+ </listitem>+ </varlistentry>+ </variablelist>+ </sect2>+
<sect2>
<title>Examples</title>
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: masao.fujii@oss.nttdata.com, kuroda.hayato@fujitsu.com, pgsql-hackers@lists.postgresql.org, tgl@sss.pgh.pa.us
Subject: Re: Allow escape in application_name (was: [postgres_fdw] add local pid to fallback_application_name)
In-Reply-To: <42ea526c-aa4f-bfe2-5547-6fcf8ac6469c@oss.nttdata.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox