Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Commit8132f0f

Browse files
committed
Fix mishandling of quoted-list GUC values in pg_dump and ruleutils.c.
Code that prints out the contents of setconfig or proconfig arrays inSQL format needs to handle GUC_LIST_QUOTE variables differently fromother ones, because for those variables, flatten_set_variable_args()already applied a layer of quoting. The value can therefore safelybe printed as-is, and indeed must be, or flatten_set_variable_args()will muck it up completely on reload. For all other GUC variables,it's necessary and sufficient to quote the value as a SQL literal.We'd recognized the need for this long ago, but mis-analyzed theneed slightly, thinking that all GUC_LIST_INPUT variables neededthe special treatment. That's actually wrong, since a valid valueof a LIST variable might include characters that need quoting,although no existing variables accept such values.More to the point, we hadn't made any particular effort to keep thevarious places that deal with this up-to-date with the set of variablesthat actually need special treatment, meaning that we'd do the wrongthing with, for example, temp_tablespaces values. This affects dumpingof SET clauses attached to functions, as well as ALTER DATABASE/ROLE SETcommands.In ruleutils.c we can fix it reasonably honestly by exporting a guc.cfunction that allows discovering the flags for a given GUC variable.But pg_dump doesn't have easy access to that, so continue the old methodof having a hard-wired list of affected variable names. At least we canfix it to have just one list not two, and update the list to matchcurrent reality.A remaining problem with this is that it only works for built-inGUC variables. pg_dump's list obvious knows nothing of third-partyextensions, and even the "ask guc.c" method isn't bulletproof sincethe relevant extension might not be loaded. There's no obvioussolution to that, so for now, we'll just have to discourage extensionauthors from inventing custom GUCs that need GUC_LIST_QUOTE.This has been busted for a long time, so back-patch to all supportedbranches.Michael Paquier and Tom Lane, reviewed by Kyotaro Horiguchi andPavel StehuleDiscussion:https://postgr.es/m/20180111064900.GA51030@paquier.xyz
1 parent36e1d45 commit8132f0f

File tree

9 files changed

+117
-13
lines changed

9 files changed

+117
-13
lines changed

‎src/backend/utils/adt/ruleutils.c‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@
5858
#include"utils/array.h"
5959
#include"utils/builtins.h"
6060
#include"utils/fmgroids.h"
61+
#include"utils/guc.h"
6162
#include"utils/hsearch.h"
6263
#include"utils/lsyscache.h"
6364
#include"utils/rel.h"
@@ -2197,11 +2198,15 @@ pg_get_functiondef(PG_FUNCTION_ARGS)
21972198
quote_identifier(configitem));
21982199

21992200
/*
2200-
* Some GUC variable names are 'LIST' type and hence must not
2201-
* be quoted.
2201+
* Variables that are marked GUC_LIST_QUOTE were already fully
2202+
* quoted by flatten_set_variable_args() before they were put
2203+
* into the proconfig array; we mustn't re-quote them or we'll
2204+
* make a mess. Variables that are not so marked should just
2205+
* be emitted as simple string literals. If the variable is
2206+
* not known to guc.c, we'll do the latter; this makes it
2207+
* unsafe to use GUC_LIST_QUOTE for extension variables.
22022208
*/
2203-
if (pg_strcasecmp(configitem,"DateStyle")==0
2204-
||pg_strcasecmp(configitem,"search_path")==0)
2209+
if (GetConfigOptionFlags(configitem, true)&GUC_LIST_QUOTE)
22052210
appendStringInfoString(&buf,pos);
22062211
else
22072212
simple_quote_literal(&buf,pos);

‎src/backend/utils/misc/guc.c‎

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -769,8 +769,8 @@ static const unit_conversion time_unit_conversion_table[] =
769769
*
770770
* 6. Don't forget to document the option (at least in config.sgml).
771771
*
772-
* 7. If it's a newGUC_LIST option you mustedit pg_dumpall.c to ensure
773-
*it is not single quoted at dump time.
772+
* 7. If it's a newGUC_LIST_QUOTE option, you mustadd it to
773+
*variable_is_guc_list_quote() in src/bin/pg_dump/dumputils.c.
774774
*/
775775

776776

@@ -6689,6 +6689,30 @@ GetConfigOptionResetString(const char *name)
66896689
returnNULL;
66906690
}
66916691

6692+
/*
6693+
* Get the GUC flags associated with the given option.
6694+
*
6695+
* If the option doesn't exist, return 0 if missing_ok is true,
6696+
* otherwise throw an ereport and don't return.
6697+
*/
6698+
int
6699+
GetConfigOptionFlags(constchar*name,boolmissing_ok)
6700+
{
6701+
structconfig_generic*record;
6702+
6703+
record=find_option(name, false,WARNING);
6704+
if (record==NULL)
6705+
{
6706+
if (missing_ok)
6707+
return0;
6708+
ereport(ERROR,
6709+
(errcode(ERRCODE_UNDEFINED_OBJECT),
6710+
errmsg("unrecognized configuration parameter \"%s\"",
6711+
name)));
6712+
}
6713+
returnrecord->flags;
6714+
}
6715+
66926716

66936717
/*
66946718
* flatten_set_variable_args

‎src/bin/pg_dump/dumputils.c‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -853,3 +853,26 @@ buildACLQueries(PQExpBuffer acl_subquery, PQExpBuffer racl_subquery,
853853
printfPQExpBuffer(init_racl_subquery,"NULL");
854854
}
855855
}
856+
857+
/*
858+
* Detect whether the given GUC variable is of GUC_LIST_QUOTE type.
859+
*
860+
* It'd be better if we could inquire this directly from the backend; but even
861+
* if there were a function for that, it could only tell us about variables
862+
* currently known to guc.c, so that it'd be unsafe for extensions to declare
863+
* GUC_LIST_QUOTE variables anyway. Lacking a solution for that, it doesn't
864+
* seem worth the work to do more than have this list, which must be kept in
865+
* sync with the variables actually marked GUC_LIST_QUOTE in guc.c.
866+
*/
867+
bool
868+
variable_is_guc_list_quote(constchar*name)
869+
{
870+
if (pg_strcasecmp(name,"temp_tablespaces")==0||
871+
pg_strcasecmp(name,"session_preload_libraries")==0||
872+
pg_strcasecmp(name,"shared_preload_libraries")==0||
873+
pg_strcasecmp(name,"local_preload_libraries")==0||
874+
pg_strcasecmp(name,"search_path")==0)
875+
return true;
876+
else
877+
return false;
878+
}

‎src/bin/pg_dump/dumputils.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,4 +56,6 @@ extern void buildACLQueries(PQExpBuffer acl_subquery, PQExpBuffer racl_subquery,
5656
constchar*acl_column,constchar*acl_owner,
5757
constchar*obj_kind,boolbinary_upgrade);
5858

59+
externboolvariable_is_guc_list_quote(constchar*name);
60+
5961
#endif/* DUMPUTILS_H */

‎src/bin/pg_dump/pg_dump.c‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11799,11 +11799,15 @@ dumpFunc(Archive *fout, FuncInfo *finfo)
1179911799
appendPQExpBuffer(q, "\n SET %s TO ", fmtId(configitem));
1180011800

1180111801
/*
11802-
* Some GUC variable names are 'LIST' type and hence must not be
11803-
* quoted.
11802+
* Variables that are marked GUC_LIST_QUOTE were already fully quoted
11803+
* by flatten_set_variable_args() before they were put into the
11804+
* proconfig array; we mustn't re-quote them or we'll make a mess.
11805+
* Variables that are not so marked should just be emitted as simple
11806+
* string literals. If the variable is not known to
11807+
* variable_is_guc_list_quote(), we'll do the latter; this makes it
11808+
* unsafe to use GUC_LIST_QUOTE for extension variables.
1180411809
*/
11805-
if (pg_strcasecmp(configitem, "DateStyle") == 0
11806-
|| pg_strcasecmp(configitem, "search_path") == 0)
11810+
if (variable_is_guc_list_quote(configitem))
1180711811
appendPQExpBufferStr(q, pos);
1180811812
else
1180911813
appendStringLiteralAH(q, pos, fout);

‎src/bin/pg_dump/pg_dumpall.c‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1719,10 +1719,15 @@ makeAlterConfigCommand(PGconn *conn, const char *arrayitem,
17191719
appendPQExpBuffer(buf,"SET %s TO ",fmtId(mine));
17201720

17211721
/*
1722-
* Some GUC variable names are 'LIST' type and hence must not be quoted.
1722+
* Variables that are marked GUC_LIST_QUOTE were already fully quoted by
1723+
* flatten_set_variable_args() before they were put into the setconfig
1724+
* array; we mustn't re-quote them or we'll make a mess. Variables that
1725+
* are not so marked should just be emitted as simple string literals. If
1726+
* the variable is not known to variable_is_guc_list_quote(), we'll do the
1727+
* latter; this makes it unsafe to use GUC_LIST_QUOTE for extension
1728+
* variables.
17231729
*/
1724-
if (pg_strcasecmp(mine,"DateStyle")==0
1725-
||pg_strcasecmp(mine,"search_path")==0)
1730+
if (variable_is_guc_list_quote(mine))
17261731
appendPQExpBufferStr(buf,pos+1);
17271732
else
17281733
appendStringLiteralConn(buf,pos+1,conn);

‎src/include/utils/guc.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -349,6 +349,7 @@ extern void EmitWarningsOnPlaceholders(const char *className);
349349
externconstchar*GetConfigOption(constchar*name,boolmissing_ok,
350350
boolrestrict_superuser);
351351
externconstchar*GetConfigOptionResetString(constchar*name);
352+
externintGetConfigOptionFlags(constchar*name,boolmissing_ok);
352353
externvoidProcessConfigFile(GucContextcontext);
353354
externvoidInitializeGUCOptions(void);
354355
externboolSelectConfigFiles(constchar*userDoption,constchar*progname);

‎src/test/regress/expected/rules.out‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3059,6 +3059,33 @@ SELECT * FROM hat_data WHERE hat_name IN ('h8', 'h9', 'h7') ORDER BY hat_name;
30593059
DROP RULE hat_upsert ON hats;
30603060
drop table hats;
30613061
drop table hat_data;
3062+
-- test for pg_get_functiondef properly regurgitating SET parameters
3063+
-- Note that the function is kept around to stress pg_dump.
3064+
CREATE FUNCTION func_with_set_params() RETURNS integer
3065+
AS 'select 1;'
3066+
LANGUAGE SQL
3067+
SET search_path TO PG_CATALOG
3068+
SET extra_float_digits TO 2
3069+
SET work_mem TO '4MB'
3070+
SET datestyle to iso, mdy
3071+
SET local_preload_libraries TO "Mixed/Case", 'c:/"a"/path'
3072+
IMMUTABLE STRICT;
3073+
SELECT pg_get_functiondef('func_with_set_params()'::regprocedure);
3074+
pg_get_functiondef
3075+
---------------------------------------------------------------
3076+
CREATE OR REPLACE FUNCTION public.func_with_set_params() +
3077+
RETURNS integer +
3078+
LANGUAGE sql +
3079+
IMMUTABLE STRICT +
3080+
SET search_path TO pg_catalog +
3081+
SET extra_float_digits TO '2' +
3082+
SET work_mem TO '4MB' +
3083+
SET "DateStyle" TO 'iso, mdy' +
3084+
SET local_preload_libraries TO "Mixed/Case", "c:/""a""/path"+
3085+
AS $function$select 1;$function$ +
3086+
3087+
(1 row)
3088+
30623089
-- tests for pg_get_*def with invalid objects
30633090
SELECT pg_get_constraintdef(0);
30643091
pg_get_constraintdef

‎src/test/regress/sql/rules.sql‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1145,6 +1145,19 @@ DROP RULE hat_upsert ON hats;
11451145
droptable hats;
11461146
droptable hat_data;
11471147

1148+
-- test for pg_get_functiondef properly regurgitating SET parameters
1149+
-- Note that the function is kept around to stress pg_dump.
1150+
CREATEFUNCTIONfunc_with_set_params() RETURNSinteger
1151+
AS'select 1;'
1152+
LANGUAGE SQL
1153+
SET search_path TO PG_CATALOG
1154+
SET extra_float_digits TO2
1155+
SET work_mem TO'4MB'
1156+
SET datestyle to iso, mdy
1157+
SET local_preload_libraries TO"Mixed/Case",'c:/"a"/path'
1158+
IMMUTABLE STRICT;
1159+
SELECT pg_get_functiondef('func_with_set_params()'::regprocedure);
1160+
11481161
-- tests for pg_get_*def with invalid objects
11491162
SELECT pg_get_constraintdef(0);
11501163
SELECT pg_get_functiondef(0);

0 commit comments

Comments
 (0)

[8]ページ先頭

©2009-2025 Movatter.jp