On 2021/09/03 14:56, kuroda.hay...@fujitsu.com wrote:
Dear Fujii-san,
Thank you for your great works. Attached is the latest version.
Thanks a lot!
I set four testcases:
(1) Sets neither GUC nor server option
(2) Sets server option, but not GUC
(3) Sets GUC but not server option
(4) Sets both GUC and server option
OK.
I confirmed it almost works fine, but I found that
fallback_application_name will be never used in our test enviroment.
It is caused because our test runner pg_regress sets PGAPPNAME to "pg_regress"
and
libpq prefer the environment variable to fallback_appname.
(I tried to control it by \setenv, but failed...)
make check uses "pg_regress", but I guess that make installcheck uses
"postgres_fdw". So the patch would cause make installcheck to fail.
I think that the case (1) is not so important, so can be removed. Thought?
Attached is the updated version of the patch. I removed the test
for case (1). And I arranged the regression tests so that they are based
on debug_discard_caches, to simplify them. Also I added and updated
some comments and docs. Could you review this version?
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
diff --git a/contrib/postgres_fdw/connection.c
b/contrib/postgres_fdw/connection.c
index 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.out
index e3ee30f1aa..6a7d83c81f 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 ft1 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.c
index 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.h
index 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.sql
index 30b5175da5..2fad091a5e 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 ft1 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.sgml
index 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>
