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

Commit6d06049

Browse files
committed
Adjust postgres_fdw's search path handling.
Set the remote session's search path to exactly "pg_catalog" at sessionstart, then schema-qualify only names that aren't in that schema. Thisgreatly reduces clutter in the generated SQL commands, as seen in theregression test changes. Per discussion.Also, rethink use of FirstNormalObjectId as the "built-in object" cutoff--- FirstBootstrapObjectId is safer, since the former will acceptobjects in information_schema for instance.
1 parentabf5c5c commit6d06049

File tree

3 files changed

+180
-114
lines changed

3 files changed

+180
-114
lines changed

‎contrib/postgres_fdw/connection.c‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ static bool xact_got_connection = false;
6363
/* prototypes of private functions */
6464
staticPGconn*connect_pg_server(ForeignServer*server,UserMapping*user);
6565
staticvoidcheck_conn_params(constchar**keywords,constchar**values);
66+
staticvoidconfigure_remote_session(PGconn*conn);
6667
staticvoidbegin_remote_xact(ConnCacheEntry*entry);
6768
staticvoidpgfdw_xact_callback(XactEventevent,void*arg);
6869
staticvoidpgfdw_subxact_callback(SubXactEventevent,
@@ -237,6 +238,9 @@ connect_pg_server(ForeignServer *server, UserMapping *user)
237238
errdetail("Non-superuser cannot connect if the server does not request a password."),
238239
errhint("Target server's authentication method must be changed.")));
239240

241+
/* Prepare new session for use */
242+
configure_remote_session(conn);
243+
240244
pfree(keywords);
241245
pfree(values);
242246
}
@@ -281,6 +285,31 @@ check_conn_params(const char **keywords, const char **values)
281285
errdetail("Non-superusers must provide a password in the user mapping.")));
282286
}
283287

288+
/*
289+
* Issue SET commands to make sure remote session is configured properly.
290+
*
291+
* We do this just once at connection, assuming nothing will change the
292+
* values later. Since we'll never send volatile function calls to the
293+
* remote, there shouldn't be any way to break this assumption from our end.
294+
* It's possible to think of ways to break it at the remote end, eg making
295+
* a foreign table point to a view that includes a set_config call ---
296+
* but once you admit the possibility of a malicious view definition,
297+
* there are any number of ways to break things.
298+
*/
299+
staticvoid
300+
configure_remote_session(PGconn*conn)
301+
{
302+
constchar*sql;
303+
PGresult*res;
304+
305+
/* Force the search path to contain only pg_catalog (see deparse.c) */
306+
sql="SET search_path = pg_catalog";
307+
res=PQexec(conn,sql);
308+
if (PQresultStatus(res)!=PGRES_COMMAND_OK)
309+
pgfdw_report_error(ERROR,res, true,sql);
310+
PQclear(res);
311+
}
312+
284313
/*
285314
* Start remote transaction or subtransaction, if needed.
286315
*

‎contrib/postgres_fdw/deparse.c‎

Lines changed: 69 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@
1111
* One saving grace is that we only need deparse logic for node types that
1212
* we consider safe to send.
1313
*
14+
* We assume that the remote session's search_path is exactly "pg_catalog",
15+
* and thus we need schema-qualify all and only names outside pg_catalog.
16+
*
1417
* Portions Copyright (c) 2012-2013, PostgreSQL Global Development Group
1518
*
1619
* IDENTIFICATION
@@ -25,6 +28,7 @@
2528
#include"access/htup_details.h"
2629
#include"access/sysattr.h"
2730
#include"access/transam.h"
31+
#include"catalog/pg_namespace.h"
2832
#include"catalog/pg_operator.h"
2933
#include"catalog/pg_proc.h"
3034
#include"catalog/pg_type.h"
@@ -73,6 +77,7 @@ static void deparseParam(StringInfo buf, Param *node, PlannerInfo *root);
7377
staticvoiddeparseArrayRef(StringInfobuf,ArrayRef*node,PlannerInfo*root);
7478
staticvoiddeparseFuncExpr(StringInfobuf,FuncExpr*node,PlannerInfo*root);
7579
staticvoiddeparseOpExpr(StringInfobuf,OpExpr*node,PlannerInfo*root);
80+
staticvoiddeparseOperatorName(StringInfobuf,Form_pg_operatoropform);
7681
staticvoiddeparseDistinctExpr(StringInfobuf,DistinctExpr*node,
7782
PlannerInfo*root);
7883
staticvoiddeparseScalarArrayOpExpr(StringInfobuf,ScalarArrayOpExpr*node,
@@ -321,6 +326,18 @@ foreign_expr_walker(Node *node, foreign_expr_cxt *context)
321326
/*
322327
* Return true if given object is one of PostgreSQL's built-in objects.
323328
*
329+
* We use FirstBootstrapObjectId as the cutoff, so that we only consider
330+
* objects with hand-assigned OIDs to be "built in", not for instance any
331+
* function or type defined in the information_schema.
332+
*
333+
* Our constraints for dealing with types are tighter than they are for
334+
* functions or operators: we want to accept only types that are in pg_catalog
335+
* (else format_type might incorrectly fail to schema-qualify their names),
336+
* and we want to be sure that the remote server will use the same OID as
337+
* we do (since we must transmit a numeric type OID when passing a value of
338+
* the type as a query parameter). Both of these are reasons to reject
339+
* objects created post-bootstrap.
340+
*
324341
* XXX there is a problem with this, which is that the set of built-in
325342
* objects expands over time. Something that is built-in to us might not
326343
* be known to the remote server, if it's of an older version. But keeping
@@ -329,7 +346,7 @@ foreign_expr_walker(Node *node, foreign_expr_cxt *context)
329346
staticbool
330347
is_builtin(Oidoid)
331348
{
332-
return (oid<FirstNormalObjectId);
349+
return (oid<FirstBootstrapObjectId);
333350
}
334351

335352

@@ -563,6 +580,10 @@ deparseRelation(StringInfo buf, Oid relid)
563580
relname=defGetString(def);
564581
}
565582

583+
/*
584+
* Note: we could skip printing the schema name if it's pg_catalog,
585+
* but that doesn't seem worth the trouble.
586+
*/
566587
if (nspname==NULL)
567588
nspname=get_namespace_name(get_rel_namespace(relid));
568589
if (relname==NULL)
@@ -832,17 +853,13 @@ deparseArrayRef(StringInfo buf, ArrayRef *node, PlannerInfo *root)
832853
* Here not only explicit function calls and explicit casts but also implicit
833854
* casts are deparsed to avoid problems caused by different cast settings
834855
* between local and remote.
835-
*
836-
* Function name is always qualified by schema name to avoid problems caused
837-
* by different setting of search_path on remote side.
838856
*/
839857
staticvoid
840858
deparseFuncExpr(StringInfobuf,FuncExpr*node,PlannerInfo*root)
841859
{
842860
HeapTupleproctup;
843861
Form_pg_procprocform;
844862
constchar*proname;
845-
constchar*schemaname;
846863
booluse_variadic;
847864
boolfirst;
848865
ListCell*arg;
@@ -851,7 +868,6 @@ deparseFuncExpr(StringInfo buf, FuncExpr *node, PlannerInfo *root)
851868
if (!HeapTupleIsValid(proctup))
852869
elog(ERROR,"cache lookup failed for function %u",node->funcid);
853870
procform= (Form_pg_proc)GETSTRUCT(proctup);
854-
proname=NameStr(procform->proname);
855871

856872
/* Check if need to print VARIADIC (cf. ruleutils.c) */
857873
if (OidIsValid(procform->provariadic))
@@ -864,11 +880,18 @@ deparseFuncExpr(StringInfo buf, FuncExpr *node, PlannerInfo *root)
864880
else
865881
use_variadic= false;
866882

883+
/* Print schema name only if it's not pg_catalog */
884+
if (procform->pronamespace!=PG_CATALOG_NAMESPACE)
885+
{
886+
constchar*schemaname;
887+
888+
schemaname=get_namespace_name(procform->pronamespace);
889+
appendStringInfo(buf,"%s.",quote_identifier(schemaname));
890+
}
891+
867892
/* Deparse the function name ... */
868-
schemaname=get_namespace_name(procform->pronamespace);
869-
appendStringInfo(buf,"%s.%s(",
870-
quote_identifier(schemaname),
871-
quote_identifier(proname));
893+
proname=NameStr(procform->proname);
894+
appendStringInfo(buf,"%s(",quote_identifier(proname));
872895
/* ... and all the arguments */
873896
first= true;
874897
foreach(arg,node->args)
@@ -887,16 +910,13 @@ deparseFuncExpr(StringInfo buf, FuncExpr *node, PlannerInfo *root)
887910

888911
/*
889912
* Deparse given operator expression into buf.To avoid problems around
890-
* priority of operations, we always parenthesize the arguments. Also we use
891-
* OPERATOR(schema.operator) notation to determine remote operator exactly.
913+
* priority of operations, we always parenthesize the arguments.
892914
*/
893915
staticvoid
894916
deparseOpExpr(StringInfobuf,OpExpr*node,PlannerInfo*root)
895917
{
896918
HeapTupletuple;
897919
Form_pg_operatorform;
898-
constchar*opnspname;
899-
char*opname;
900920
charoprkind;
901921
ListCell*arg;
902922

@@ -905,10 +925,6 @@ deparseOpExpr(StringInfo buf, OpExpr *node, PlannerInfo *root)
905925
if (!HeapTupleIsValid(tuple))
906926
elog(ERROR,"cache lookup failed for operator %u",node->opno);
907927
form= (Form_pg_operator)GETSTRUCT(tuple);
908-
909-
opnspname=quote_identifier(get_namespace_name(form->oprnamespace));
910-
/* opname is not a SQL identifier, so we don't need to quote it. */
911-
opname=NameStr(form->oprname);
912928
oprkind=form->oprkind;
913929

914930
/* Sanity check. */
@@ -927,8 +943,8 @@ deparseOpExpr(StringInfo buf, OpExpr *node, PlannerInfo *root)
927943
appendStringInfoChar(buf,' ');
928944
}
929945

930-
/* Deparsefully qualifiedoperator name. */
931-
appendStringInfo(buf,"OPERATOR(%s.%s)",opnspname,opname);
946+
/* Deparse operator name. */
947+
deparseOperatorName(buf,form);
932948

933949
/* Deparse right operand. */
934950
if (oprkind=='l'||oprkind=='b')
@@ -943,6 +959,34 @@ deparseOpExpr(StringInfo buf, OpExpr *node, PlannerInfo *root)
943959
ReleaseSysCache(tuple);
944960
}
945961

962+
/*
963+
* Print the name of an operator.
964+
*/
965+
staticvoid
966+
deparseOperatorName(StringInfobuf,Form_pg_operatoropform)
967+
{
968+
char*opname;
969+
970+
/* opname is not a SQL identifier, so we should not quote it. */
971+
opname=NameStr(opform->oprname);
972+
973+
/* Print schema name only if it's not pg_catalog */
974+
if (opform->oprnamespace!=PG_CATALOG_NAMESPACE)
975+
{
976+
constchar*opnspname;
977+
978+
opnspname=get_namespace_name(opform->oprnamespace);
979+
/* Print fully qualified operator name. */
980+
appendStringInfo(buf,"OPERATOR(%s.%s)",
981+
quote_identifier(opnspname),opname);
982+
}
983+
else
984+
{
985+
/* Just print operator name. */
986+
appendStringInfo(buf,"%s",opname);
987+
}
988+
}
989+
946990
/*
947991
* Deparse IS DISTINCT FROM.
948992
*/
@@ -960,9 +1004,7 @@ deparseDistinctExpr(StringInfo buf, DistinctExpr *node, PlannerInfo *root)
9601004

9611005
/*
9621006
* Deparse given ScalarArrayOpExpr expression into buf. To avoid problems
963-
* around priority of operations, we always parenthesize the arguments. Also
964-
* we use OPERATOR(schema.operator) notation to determine remote operator
965-
* exactly.
1007+
* around priority of operations, we always parenthesize the arguments.
9661008
*/
9671009
staticvoid
9681010
deparseScalarArrayOpExpr(StringInfobuf,
@@ -971,8 +1013,6 @@ deparseScalarArrayOpExpr(StringInfo buf,
9711013
{
9721014
HeapTupletuple;
9731015
Form_pg_operatorform;
974-
constchar*opnspname;
975-
char*opname;
9761016
Expr*arg1;
9771017
Expr*arg2;
9781018

@@ -982,10 +1022,6 @@ deparseScalarArrayOpExpr(StringInfo buf,
9821022
elog(ERROR,"cache lookup failed for operator %u",node->opno);
9831023
form= (Form_pg_operator)GETSTRUCT(tuple);
9841024

985-
opnspname=quote_identifier(get_namespace_name(form->oprnamespace));
986-
/* opname is not a SQL identifier, so we don't need to quote it. */
987-
opname=NameStr(form->oprname);
988-
9891025
/* Sanity check. */
9901026
Assert(list_length(node->args)==2);
9911027

@@ -995,10 +1031,11 @@ deparseScalarArrayOpExpr(StringInfo buf,
9951031
/* Deparse left operand. */
9961032
arg1=linitial(node->args);
9971033
deparseExpr(buf,arg1,root);
1034+
appendStringInfoChar(buf,' ');
9981035

999-
/* Deparsefully qualifiedoperator name plus decoration. */
1000-
appendStringInfo(buf," OPERATOR(%s.%s) %s (",
1001-
opnspname,opname,node->useOr ?"ANY" :"ALL");
1036+
/* Deparse operator name plus decoration. */
1037+
deparseOperatorName(buf,form);
1038+
appendStringInfo(buf," %s (",node->useOr ?"ANY" :"ALL");
10021039

10031040
/* Deparse right operand. */
10041041
arg2=lsecond(node->args);

0 commit comments

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