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

Commit7428699

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 parent0f0deb7 commit7428699

File tree

8 files changed

+117
-14
lines changed

8 files changed

+117
-14
lines changed

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

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@
6161
#include"utils/array.h"
6262
#include"utils/builtins.h"
6363
#include"utils/fmgroids.h"
64+
#include"utils/guc.h"
6465
#include"utils/hsearch.h"
6566
#include"utils/lsyscache.h"
6667
#include"utils/rel.h"
@@ -2597,11 +2598,15 @@ pg_get_functiondef(PG_FUNCTION_ARGS)
25972598
quote_identifier(configitem));
25982599

25992600
/*
2600-
* Some GUC variable names are 'LIST' type and hence must not
2601-
* be quoted.
2601+
* Variables that are marked GUC_LIST_QUOTE were already fully
2602+
* quoted by flatten_set_variable_args() before they were put
2603+
* into the proconfig array; we mustn't re-quote them or we'll
2604+
* make a mess. Variables that are not so marked should just
2605+
* be emitted as simple string literals. If the variable is
2606+
* not known to guc.c, we'll do the latter; this makes it
2607+
* unsafe to use GUC_LIST_QUOTE for extension variables.
26022608
*/
2603-
if (pg_strcasecmp(configitem,"DateStyle")==0
2604-
||pg_strcasecmp(configitem,"search_path")==0)
2609+
if (GetConfigOptionFlags(configitem, true)&GUC_LIST_QUOTE)
26052610
appendStringInfoString(&buf,pos);
26062611
else
26072612
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
@@ -796,8 +796,8 @@ static const unit_conversion time_unit_conversion_table[] =
796796
*
797797
* 6. Don't forget to document the option (at least in config.sgml).
798798
*
799-
* 7. If it's a newGUC_LIST option you mustedit pg_dumpall.c to ensure
800-
*it is not single quoted at dump time.
799+
* 7. If it's a newGUC_LIST_QUOTE option, you mustadd it to
800+
*variable_is_guc_list_quote() in src/bin/pg_dump/dumputils.c.
801801
*/
802802

803803

@@ -6859,6 +6859,30 @@ GetConfigOptionResetString(const char *name)
68596859
returnNULL;
68606860
}
68616861

6862+
/*
6863+
* Get the GUC flags associated with the given option.
6864+
*
6865+
* If the option doesn't exist, return 0 if missing_ok is true,
6866+
* otherwise throw an ereport and don't return.
6867+
*/
6868+
int
6869+
GetConfigOptionFlags(constchar*name,boolmissing_ok)
6870+
{
6871+
structconfig_generic*record;
6872+
6873+
record=find_option(name, false,WARNING);
6874+
if (record==NULL)
6875+
{
6876+
if (missing_ok)
6877+
return0;
6878+
ereport(ERROR,
6879+
(errcode(ERRCODE_UNDEFINED_OBJECT),
6880+
errmsg("unrecognized configuration parameter \"%s\"",
6881+
name)));
6882+
}
6883+
returnrecord->flags;
6884+
}
6885+
68626886

68636887
/*
68646888
* flatten_set_variable_args

‎src/bin/pg_dump/dumputils.c

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -850,6 +850,29 @@ buildACLQueries(PQExpBuffer acl_subquery, PQExpBuffer racl_subquery,
850850
}
851851
}
852852

853+
/*
854+
* Detect whether the given GUC variable is of GUC_LIST_QUOTE type.
855+
*
856+
* It'd be better if we could inquire this directly from the backend; but even
857+
* if there were a function for that, it could only tell us about variables
858+
* currently known to guc.c, so that it'd be unsafe for extensions to declare
859+
* GUC_LIST_QUOTE variables anyway. Lacking a solution for that, it doesn't
860+
* seem worth the work to do more than have this list, which must be kept in
861+
* sync with the variables actually marked GUC_LIST_QUOTE in guc.c.
862+
*/
863+
bool
864+
variable_is_guc_list_quote(constchar*name)
865+
{
866+
if (pg_strcasecmp(name,"temp_tablespaces")==0||
867+
pg_strcasecmp(name,"session_preload_libraries")==0||
868+
pg_strcasecmp(name,"shared_preload_libraries")==0||
869+
pg_strcasecmp(name,"local_preload_libraries")==0||
870+
pg_strcasecmp(name,"search_path")==0)
871+
return true;
872+
else
873+
return false;
874+
}
875+
853876
/*
854877
* Helper function for dumping "ALTER DATABASE/ROLE SET ..." commands.
855878
*
@@ -887,11 +910,15 @@ makeAlterConfigCommand(PGconn *conn, const char *configitem,
887910
appendPQExpBuffer(buf,"SET %s TO ",fmtId(mine));
888911

889912
/*
890-
* Some GUC variable names are 'LIST' type and hence must not be quoted.
891-
* XXX this list is incomplete ...
913+
* Variables that are marked GUC_LIST_QUOTE were already fully quoted by
914+
* flatten_set_variable_args() before they were put into the setconfig
915+
* array; we mustn't re-quote them or we'll make a mess. Variables that
916+
* are not so marked should just be emitted as simple string literals. If
917+
* the variable is not known to variable_is_guc_list_quote(), we'll do the
918+
* latter; this makes it unsafe to use GUC_LIST_QUOTE for extension
919+
* variables.
892920
*/
893-
if (pg_strcasecmp(mine,"DateStyle")==0
894-
||pg_strcasecmp(mine,"search_path")==0)
921+
if (variable_is_guc_list_quote(mine))
895922
appendPQExpBufferStr(buf,pos);
896923
else
897924
appendStringLiteralConn(buf,pos,conn);

‎src/bin/pg_dump/dumputils.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,8 @@ 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
externvoidmakeAlterConfigCommand(PGconn*conn,constchar*configitem,
6062
constchar*type,constchar*name,
6163
constchar*type2,constchar*name2,

‎src/bin/pg_dump/pg_dump.c

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

1187911879
/*
11880-
* Some GUC variable names are 'LIST' type and hence must not be
11881-
* quoted.
11880+
* Variables that are marked GUC_LIST_QUOTE were already fully quoted
11881+
* by flatten_set_variable_args() before they were put into the
11882+
* proconfig array; we mustn't re-quote them or we'll make a mess.
11883+
* Variables that are not so marked should just be emitted as simple
11884+
* string literals. If the variable is not known to
11885+
* variable_is_guc_list_quote(), we'll do the latter; this makes it
11886+
* unsafe to use GUC_LIST_QUOTE for extension variables.
1188211887
*/
11883-
if (pg_strcasecmp(configitem, "DateStyle") == 0
11884-
|| pg_strcasecmp(configitem, "search_path") == 0)
11888+
if (variable_is_guc_list_quote(configitem))
1188511889
appendPQExpBufferStr(q, pos);
1188611890
else
1188711891
appendStringLiteralAH(q, pos, fout);

‎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
@@ -3149,6 +3149,33 @@ SELECT * FROM hat_data WHERE hat_name IN ('h8', 'h9', 'h7') ORDER BY hat_name;
31493149
DROP RULE hat_upsert ON hats;
31503150
drop table hats;
31513151
drop table hat_data;
3152+
-- test for pg_get_functiondef properly regurgitating SET parameters
3153+
-- Note that the function is kept around to stress pg_dump.
3154+
CREATE FUNCTION func_with_set_params() RETURNS integer
3155+
AS 'select 1;'
3156+
LANGUAGE SQL
3157+
SET search_path TO PG_CATALOG
3158+
SET extra_float_digits TO 2
3159+
SET work_mem TO '4MB'
3160+
SET datestyle to iso, mdy
3161+
SET local_preload_libraries TO "Mixed/Case", 'c:/"a"/path'
3162+
IMMUTABLE STRICT;
3163+
SELECT pg_get_functiondef('func_with_set_params()'::regprocedure);
3164+
pg_get_functiondef
3165+
---------------------------------------------------------------
3166+
CREATE OR REPLACE FUNCTION public.func_with_set_params() +
3167+
RETURNS integer +
3168+
LANGUAGE sql +
3169+
IMMUTABLE STRICT +
3170+
SET search_path TO pg_catalog +
3171+
SET extra_float_digits TO '2' +
3172+
SET work_mem TO '4MB' +
3173+
SET "DateStyle" TO 'iso, mdy' +
3174+
SET local_preload_libraries TO "Mixed/Case", "c:/""a""/path"+
3175+
AS $function$select 1;$function$ +
3176+
3177+
(1 row)
3178+
31523179
-- tests for pg_get_*def with invalid objects
31533180
SELECT pg_get_constraintdef(0);
31543181
pg_get_constraintdef

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1155,6 +1155,19 @@ DROP RULE hat_upsert ON hats;
11551155
droptable hats;
11561156
droptable hat_data;
11571157

1158+
-- test for pg_get_functiondef properly regurgitating SET parameters
1159+
-- Note that the function is kept around to stress pg_dump.
1160+
CREATEFUNCTIONfunc_with_set_params() RETURNSinteger
1161+
AS'select 1;'
1162+
LANGUAGE SQL
1163+
SET search_path TO PG_CATALOG
1164+
SET extra_float_digits TO2
1165+
SET work_mem TO'4MB'
1166+
SET datestyle to iso, mdy
1167+
SET local_preload_libraries TO"Mixed/Case",'c:/"a"/path'
1168+
IMMUTABLE STRICT;
1169+
SELECT pg_get_functiondef('func_with_set_params()'::regprocedure);
1170+
11581171
-- tests for pg_get_*def with invalid objects
11591172
SELECT pg_get_constraintdef(0);
11601173
SELECT pg_get_functiondef(0);

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp