On Fri, Mar 16, 2018 at 10:25:35AM +0900, Michael Paquier wrote: >> But, I suppose it is a bit too big. > > That's of course not backpatchable.
So in this jungle attached is my counter-proposal. As the same code pattern is repeated in three places, we could as well refactor the whole into a common routine, say in src/common/guc_options.c or similar. Perhaps just on HEAD and not back-branches as this is always annoying for packagers on Windows using custom scripts. Per the lack of complains only doing something on HEAD, with only a subset of the parameters which make sense for CREATE FUNCTION is what makes the most sense in my opinion. As mentioned in this bug, the handling of empty values gets kind of tricky as in this case proconfig stores a set of quotes and not an actual value: https://www.postgresql.org/message-id/152049236165.23137.5241258464332317...@wrigleys.postgresql.org -- Michael
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index b58ee3c387..ccd7863c6c 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -2596,11 +2596,23 @@ pg_get_functiondef(PG_FUNCTION_ARGS)
quote_identifier(configitem));
/*
- * Some GUC variable names are 'LIST' type and hence must not
- * be quoted.
+ * Some GUC variable names are of type GUC_LIST_INPUT and
+ * hence must not be quoted.
+ * XXX: Keep this list in sync with what is defined in guc.c and
+ * any modules using list parameters!
*/
- if (pg_strcasecmp(configitem, "DateStyle") == 0
- || pg_strcasecmp(configitem, "search_path") == 0)
+ if (pg_strcasecmp(configitem, "DateStyle") == 0 ||
+ pg_strcasecmp(configitem, "listen_addresses") == 0 ||
+ pg_strcasecmp(configitem, "local_preload_libraries") == 0 ||
+ pg_strcasecmp(configitem, "log_destination") == 0 ||
+ pg_strcasecmp(configitem, "plpgsql.extra_errors") == 0 ||
+ pg_strcasecmp(configitem, "plpgsql.extra_warnings") == 0 ||
+ pg_strcasecmp(configitem, "search_path") == 0 ||
+ pg_strcasecmp(configitem, "session_preload_libraries") == 0 ||
+ pg_strcasecmp(configitem, "shared_preload_libraries") == 0 ||
+ pg_strcasecmp(configitem, "synchronous_standby_names") == 0 ||
+ pg_strcasecmp(configitem, "temp_tablespaces") == 0 ||
+ pg_strcasecmp(configitem, "wal_consistency_checking") == 0)
appendStringInfoString(&buf, pos);
else
simple_quote_literal(&buf, pos);
diff --git a/src/bin/pg_dump/dumputils.c b/src/bin/pg_dump/dumputils.c
index 7f5bb1343e..c39096ebff 100644
--- a/src/bin/pg_dump/dumputils.c
+++ b/src/bin/pg_dump/dumputils.c
@@ -887,11 +887,23 @@ makeAlterConfigCommand(PGconn *conn, const char *configitem,
appendPQExpBuffer(buf, "SET %s TO ", fmtId(mine));
/*
- * Some GUC variable names are 'LIST' type and hence must not be quoted.
- * XXX this list is incomplete ...
+ * Some GUC variable names are of type GUC_LIST_INPUT and hence
+ * must not be quoted.
+ * XXX: Keep this list in sync with what is defined in guc.c and
+ * any modules using list parameters!
*/
- if (pg_strcasecmp(mine, "DateStyle") == 0
- || pg_strcasecmp(mine, "search_path") == 0)
+ if (pg_strcasecmp(mine, "DateStyle") == 0 ||
+ pg_strcasecmp(mine, "listen_addresses") == 0 ||
+ pg_strcasecmp(mine, "local_preload_libraries") == 0 ||
+ pg_strcasecmp(mine, "log_destination") == 0 ||
+ pg_strcasecmp(mine, "plpgsql.extra_errors") == 0 ||
+ pg_strcasecmp(mine, "plpgsql.extra_warnings") == 0 ||
+ pg_strcasecmp(mine, "search_path") == 0 ||
+ pg_strcasecmp(mine, "session_preload_libraries") == 0 ||
+ pg_strcasecmp(mine, "shared_preload_libraries") == 0 ||
+ pg_strcasecmp(mine, "synchronous_standby_names") == 0 ||
+ pg_strcasecmp(mine, "temp_tablespaces") == 0 ||
+ pg_strcasecmp(mine, "wal_consistency_checking") == 0)
appendPQExpBufferStr(buf, pos);
else
appendStringLiteralConn(buf, pos, conn);
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 566cbf2cda..1ec9d4b0fd 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -11877,11 +11877,23 @@ dumpFunc(Archive *fout, FuncInfo *finfo)
appendPQExpBuffer(q, "\n SET %s TO ", fmtId(configitem));
/*
- * Some GUC variable names are 'LIST' type and hence must not be
- * quoted.
+ * Some GUC variable names are of type GUC_LIST_INPUT and hence
+ * must not be quoted.
+ * XXX: Keep this list in sync with what is defined in guc.c and
+ * any modules using list parameters!
*/
- if (pg_strcasecmp(configitem, "DateStyle") == 0
- || pg_strcasecmp(configitem, "search_path") == 0)
+ if (pg_strcasecmp(configitem, "DateStyle") == 0 ||
+ pg_strcasecmp(configitem, "listen_addresses") == 0 ||
+ pg_strcasecmp(configitem, "local_preload_libraries") == 0 ||
+ pg_strcasecmp(configitem, "log_destination") == 0 ||
+ pg_strcasecmp(configitem, "plpgsql.extra_errors") == 0 ||
+ pg_strcasecmp(configitem, "plpgsql.extra_warnings") == 0 ||
+ pg_strcasecmp(configitem, "search_path") == 0 ||
+ pg_strcasecmp(configitem, "session_preload_libraries") == 0 ||
+ pg_strcasecmp(configitem, "shared_preload_libraries") == 0 ||
+ pg_strcasecmp(configitem, "synchronous_standby_names") == 0 ||
+ pg_strcasecmp(configitem, "temp_tablespaces") == 0 ||
+ pg_strcasecmp(configitem, "wal_consistency_checking") == 0)
appendPQExpBufferStr(q, pos);
else
appendStringLiteralAH(q, pos, fout);
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index 5e0597e091..a416ef274f 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -3228,6 +3228,30 @@ SELECT pg_get_partkeydef(0);
(1 row)
+-- test for pg_get_functiondef with list parameters which should not be
+-- quoted and various inputs: non-quoted list and quoted list.
+-- Note that the function is kept around to stress pg_dump.
+CREATE FUNCTION func_with_set_params() RETURNS integer
+ AS 'select 1;'
+ LANGUAGE SQL
+ SET search_path TO 'pg_catalog'
+ SET wal_consistency_checking TO heap, heap2
+ SET session_preload_libraries TO 'foo, bar'
+ IMMUTABLE STRICT;
+SELECT pg_get_functiondef('func_with_set_params'::regproc);
+ pg_get_functiondef
+----------------------------------------------------------
+ CREATE OR REPLACE FUNCTION public.func_with_set_params()+
+ RETURNS integer +
+ LANGUAGE sql +
+ IMMUTABLE STRICT +
+ SET search_path TO pg_catalog +
+ SET wal_consistency_checking TO heap, heap2 +
+ SET session_preload_libraries TO "foo, bar" +
+ AS $function$select 1;$function$ +
+
+(1 row)
+
-- test rename for a rule defined on a partitioned table
CREATE TABLE rules_parted_table (a int) PARTITION BY LIST (a);
CREATE TABLE rules_parted_table_1 PARTITION OF rules_parted_table FOR VALUES IN (1);
diff --git a/src/test/regress/sql/rules.sql b/src/test/regress/sql/rules.sql
index 60212129a2..32ac34e616 100644
--- a/src/test/regress/sql/rules.sql
+++ b/src/test/regress/sql/rules.sql
@@ -1170,6 +1170,18 @@ SELECT pg_get_function_arg_default(0, 0);
SELECT pg_get_function_arg_default('pg_class'::regclass, 0);
SELECT pg_get_partkeydef(0);
+-- test for pg_get_functiondef with list parameters which should not be
+-- quoted and various inputs: non-quoted list and quoted list.
+-- Note that the function is kept around to stress pg_dump.
+CREATE FUNCTION func_with_set_params() RETURNS integer
+ AS 'select 1;'
+ LANGUAGE SQL
+ SET search_path TO 'pg_catalog'
+ SET wal_consistency_checking TO heap, heap2
+ SET session_preload_libraries TO 'foo, bar'
+ IMMUTABLE STRICT;
+SELECT pg_get_functiondef('func_with_set_params'::regproc);
+
-- test rename for a rule defined on a partitioned table
CREATE TABLE rules_parted_table (a int) PARTITION BY LIST (a);
CREATE TABLE rules_parted_table_1 PARTITION OF rules_parted_table FOR VALUES IN (1);
signature.asc
Description: PGP signature
