On Thu, Jan 11, 2018 at 10:42:38AM -0500, Tom Lane wrote: > Michael Paquier <michael.paqu...@gmail.com> writes: >> guc.c already holds a find_option() >> which can be used to retrieve the flags of a parameter. What about using >> that and filtering by GUC_LIST_INPUT? This requires exposing the >> function, and I am not sure what people think about that. > > Don't like exposing find_option directly, but I think it would make > sense to provide an API along the lines of > int GetConfigOptionFlags(const char *name, bool missing_ok).
OK, I can live with that. What do you think about the attached? I'll be happy to produce patches for back-branches as necessary. When an option is not found, I have made the function return 0 as value for the flags, which is consistent with flatten_set_variable_args(). To make things behave more consistently with GUC_LIST_QUOTE GUCs, it seems to me that those should not be quoted as well (ALTER SYSTEM shares the same compatibility). And attached is a patch. While reviewing more the code, I have noticed the same code pattern in pg_dump.c and pg_dumpall.c. In the patch attached, I have corrected things so as all parameters marked as GUC_LIST_QUOTE are correctly not quoted because we have no generic solution to know from frontends what's a GUC type (it would make sense to me to expose a text[] with this information in pg_settings). However, let's be honest, it does not make sense to update all those parameters because who is really going to use them for functions! Two things that make sense to me are just wal_consistency_checking and synchronous_standby_names for developers which could use it to tune WAL generated within a set of SQL or plpgsql functions. As it is easier to delete than add code here, I have added all of them to ease the production of a committable version. For correctness, still having them may make sense. -- Michael
diff --git a/src/backend/utils/adt/ruleutils.c
b/src/backend/utils/adt/ruleutils.c
index 9cdbb06add..7914f4dd88 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -61,6 +61,7 @@
#include "utils/array.h"
#include "utils/builtins.h"
#include "utils/fmgroids.h"
+#include "utils/guc.h"
#include "utils/hsearch.h"
#include "utils/lsyscache.h"
#include "utils/rel.h"
@@ -2561,6 +2562,7 @@ pg_get_functiondef(PG_FUNCTION_ARGS)
{
char *configitem = TextDatumGetCString(d);
char *pos;
+ int flags;
pos = strchr(configitem, '=');
if (pos == NULL)
@@ -2571,11 +2573,11 @@ 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.
*/
- if (pg_strcasecmp(configitem, "DateStyle") == 0
- || pg_strcasecmp(configitem,
"search_path") == 0)
+ flags = GetConfigOptionFlags(configitem, false);
+ if ((flags & GUC_LIST_INPUT) != 0)
appendStringInfoString(&buf, pos);
else
simple_quote_literal(&buf, pos);
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 72f6be329e..5a39c96242 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -8099,6 +8099,30 @@ GetConfigOptionByName(const char *name, const char
**varname, bool missing_ok)
return _ShowOption(record, true);
}
+/*
+ * Return "flags" for a given GUC variable. If missing_ok is true, ignore
+ * any errors and return 0 to the caller instead.
+ */
+int
+GetConfigOptionFlags(const char *name, bool missing_ok)
+{
+ struct config_generic *record;
+
+ record = find_option(name, false, ERROR);
+
+ if (record == NULL)
+ {
+ if (missing_ok)
+ return 0;
+
+ ereport(ERROR,
+ (errcode(ERRCODE_UNDEFINED_OBJECT),
+ errmsg("unrecognized configuration parameter
\"%s\"", name)));
+ }
+
+ return record->flags;
+}
+
/*
* Return GUC variable value by variable number; optionally return canonical
* form of name. Return value is palloc'd.
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 27628a397c..f9b9de8894 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -11777,11 +11777,20 @@ 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!
*/
- 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, "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/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index 3dd2c3871e..2fd43b039e 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -1728,10 +1728,20 @@ makeAlterConfigCommand(PGconn *conn, const char
*arrayitem,
appendPQExpBuffer(buf, "SET %s TO ", fmtId(mine));
/*
- * 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!
*/
- 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, "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 + 1);
else
appendStringLiteralConn(buf, pos + 1, conn);
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index 77daa5a539..b02e65cf59 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -368,6 +368,7 @@ extern int set_config_option(const char *name, const char
*value,
extern void AlterSystemSetConfigFile(AlterSystemStmt *setstmt);
extern char *GetConfigOptionByName(const char *name, const char **varname,
bool missing_ok);
+extern int GetConfigOptionFlags(const char *name, bool missing_ok);
extern void GetConfigOptionByNum(int varnum, const char **values, bool
*noshow);
extern int GetNumConfigOptions(void);
diff --git a/src/test/regress/expected/rules.out
b/src/test/regress/expected/rules.out
index f1c1b44d6f..c7ebb29b65 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -3226,6 +3226,30 @@ SELECT pg_get_partkeydef(0);
(1 row)
+-- test for pg_get_functiondef with list parameters which should not be
+-- quoted.
+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)
+
+DROP FUNCTION func_with_set_params();
-- test rename for a rule defined on a partitioned table
CREATE TABLE parted_table (a int) PARTITION BY LIST (a);
CREATE TABLE parted_table_1 PARTITION OF parted_table FOR VALUES IN (1);
diff --git a/src/test/regress/sql/rules.sql b/src/test/regress/sql/rules.sql
index 0ded0f01d2..40a6914e69 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.
+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);
+DROP FUNCTION func_with_set_params();
+
-- test rename for a rule defined on a partitioned table
CREATE TABLE parted_table (a int) PARTITION BY LIST (a);
CREATE TABLE parted_table_1 PARTITION OF parted_table FOR VALUES IN (1);
signature.asc
Description: PGP signature
