diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index 1b2486c270..8b755c4f0c 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -58,6 +58,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" @@ -2197,11 +2198,15 @@ pg_get_functiondef(PG_FUNCTION_ARGS) quote_identifier(configitem)); /* - * Some GUC variable names are 'LIST' type and hence must not - * be quoted. + * Variables that are marked GUC_LIST_QUOTE were already fully + * quoted by flatten_set_variable_args() before they were put + * into the proconfig array; we mustn't re-quote them or we'll + * make a mess. Variables that are not so marked should just + * be emitted as simple string literals. If the variable is + * not known to guc.c, we'll do the latter; this makes it + * unsafe to use GUC_LIST_QUOTE for extension variables. */ - if (pg_strcasecmp(configitem, "DateStyle") == 0 - || pg_strcasecmp(configitem, "search_path") == 0) + if (GetConfigOptionFlags(configitem, true) & GUC_LIST_QUOTE) 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 9ecbe4a9ba..0b403a3272 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -769,8 +769,8 @@ static const unit_conversion time_unit_conversion_table[] = * * 6. Don't forget to document the option (at least in config.sgml). * - * 7. If it's a new GUC_LIST option you must edit pg_dumpall.c to ensure - * it is not single quoted at dump time. + * 7. If it's a new GUC_LIST_QUOTE option, you must add it to + * variable_is_guc_list_quote() in src/bin/pg_dump/dumputils.c. */ @@ -6689,6 +6689,30 @@ GetConfigOptionResetString(const char *name) return NULL; } +/* + * Get the GUC flags associated with the given option. + * + * If the option doesn't exist, return 0 if missing_ok is true, + * otherwise throw an ereport and don't return. + */ +int +GetConfigOptionFlags(const char *name, bool missing_ok) +{ + struct config_generic *record; + + record = find_option(name, false, WARNING); + if (record == NULL) + { + if (missing_ok) + return 0; + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_OBJECT), + errmsg("unrecognized configuration parameter \"%s\"", + name))); + } + return record->flags; +} + /* * flatten_set_variable_args diff --git a/src/bin/pg_dump/dumputils.c b/src/bin/pg_dump/dumputils.c index abd0579d5d..008f284835 100644 --- a/src/bin/pg_dump/dumputils.c +++ b/src/bin/pg_dump/dumputils.c @@ -853,3 +853,26 @@ buildACLQueries(PQExpBuffer acl_subquery, PQExpBuffer racl_subquery, printfPQExpBuffer(init_racl_subquery, "NULL"); } } + +/* + * Detect whether the given GUC variable is of GUC_LIST_QUOTE type. + * + * It'd be better if we could inquire this directly from the backend; but even + * if there were a function for that, it could only tell us about variables + * currently known to guc.c, so that it'd be unsafe for extensions to declare + * GUC_LIST_QUOTE variables anyway. Lacking a solution for that, it doesn't + * seem worth the work to do more than have this list, which must be kept in + * sync with the variables actually marked GUC_LIST_QUOTE in guc.c. + */ +bool +variable_is_guc_list_quote(const char *name) +{ + if (pg_strcasecmp(name, "temp_tablespaces") == 0 || + pg_strcasecmp(name, "session_preload_libraries") == 0 || + pg_strcasecmp(name, "shared_preload_libraries") == 0 || + pg_strcasecmp(name, "local_preload_libraries") == 0 || + pg_strcasecmp(name, "search_path") == 0) + return true; + else + return false; +} diff --git a/src/bin/pg_dump/dumputils.h b/src/bin/pg_dump/dumputils.h index e2241cf40e..e6c4fcc15b 100644 --- a/src/bin/pg_dump/dumputils.h +++ b/src/bin/pg_dump/dumputils.h @@ -56,4 +56,6 @@ extern void buildACLQueries(PQExpBuffer acl_subquery, PQExpBuffer racl_subquery, const char *acl_column, const char *acl_owner, const char *obj_kind, bool binary_upgrade); +extern bool variable_is_guc_list_quote(const char *name); + #endif /* DUMPUTILS_H */ diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index 41d0a1afee..2e6225049c 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -11799,11 +11799,15 @@ 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. + * Variables that are marked GUC_LIST_QUOTE were already fully quoted + * by flatten_set_variable_args() before they were put into the + * proconfig array; we mustn't re-quote them or we'll make a mess. + * Variables that are not so marked should just be emitted as simple + * string literals. If the variable is not known to + * variable_is_guc_list_quote(), we'll do the latter; this makes it + * unsafe to use GUC_LIST_QUOTE for extension variables. */ - if (pg_strcasecmp(configitem, "DateStyle") == 0 - || pg_strcasecmp(configitem, "search_path") == 0) + if (variable_is_guc_list_quote(configitem)) 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 3fc7b024c8..3554f51ec5 100644 --- a/src/bin/pg_dump/pg_dumpall.c +++ b/src/bin/pg_dump/pg_dumpall.c @@ -1719,10 +1719,15 @@ 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. + * Variables that are marked GUC_LIST_QUOTE were already fully quoted by + * flatten_set_variable_args() before they were put into the setconfig + * array; we mustn't re-quote them or we'll make a mess. Variables that + * are not so marked should just be emitted as simple string literals. If + * the variable is not known to variable_is_guc_list_quote(), we'll do the + * latter; this makes it unsafe to use GUC_LIST_QUOTE for extension + * variables. */ - if (pg_strcasecmp(mine, "DateStyle") == 0 - || pg_strcasecmp(mine, "search_path") == 0) + if (variable_is_guc_list_quote(mine)) 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 68c9110304..2b8a0b933d 100644 --- a/src/include/utils/guc.h +++ b/src/include/utils/guc.h @@ -349,6 +349,7 @@ extern void EmitWarningsOnPlaceholders(const char *className); extern const char *GetConfigOption(const char *name, bool missing_ok, bool restrict_superuser); extern const char *GetConfigOptionResetString(const char *name); +extern int GetConfigOptionFlags(const char *name, bool missing_ok); extern void ProcessConfigFile(GucContext context); extern void InitializeGUCOptions(void); extern bool SelectConfigFiles(const char *userDoption, const char *progname); diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out index 4be33aac84..3f9592f2b5 100644 --- a/src/test/regress/expected/rules.out +++ b/src/test/regress/expected/rules.out @@ -3059,6 +3059,33 @@ SELECT * FROM hat_data WHERE hat_name IN ('h8', 'h9', 'h7') ORDER BY hat_name; DROP RULE hat_upsert ON hats; drop table hats; drop table hat_data; +-- test for pg_get_functiondef properly regurgitating SET parameters +-- 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 extra_float_digits TO 2 + SET work_mem TO '4MB' + SET datestyle to iso, mdy + SET local_preload_libraries TO "Mixed/Case", 'c:/"a"/path' + IMMUTABLE STRICT; +SELECT pg_get_functiondef('func_with_set_params()'::regprocedure); + 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 extra_float_digits TO '2' + + SET work_mem TO '4MB' + + SET "DateStyle" TO 'iso, mdy' + + SET local_preload_libraries TO "Mixed/Case", "c:/""a""/path"+ + AS $function$select 1;$function$ + + +(1 row) + -- tests for pg_get_*def with invalid objects SELECT pg_get_constraintdef(0); pg_get_constraintdef diff --git a/src/test/regress/sql/rules.sql b/src/test/regress/sql/rules.sql index 90dc9ceaf4..14b2276bf1 100644 --- a/src/test/regress/sql/rules.sql +++ b/src/test/regress/sql/rules.sql @@ -1145,6 +1145,19 @@ DROP RULE hat_upsert ON hats; drop table hats; drop table hat_data; +-- test for pg_get_functiondef properly regurgitating SET parameters +-- 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 extra_float_digits TO 2 + SET work_mem TO '4MB' + SET datestyle to iso, mdy + SET local_preload_libraries TO "Mixed/Case", 'c:/"a"/path' + IMMUTABLE STRICT; +SELECT pg_get_functiondef('func_with_set_params()'::regprocedure); + -- tests for pg_get_*def with invalid objects SELECT pg_get_constraintdef(0); SELECT pg_get_functiondef(0);