Skip to content

Commit ed33e74

Browse files
pramseypramsey
committed
Fixes from Andres Freund patch review
1 parent 3f5b45b commit ed33e74

File tree

4 files changed

+25
-19
lines changed

4 files changed

+25
-19
lines changed

‎contrib/postgres_fdw/deparse.c

+5-5
Original file line numberDiff line numberDiff line change
@@ -382,7 +382,7 @@ foreign_expr_walker(Node *node,
382382
* semantics on remote side.
383383
*/
384384
if (!is_builtin(fe->funcid) &&
385-
!is_shippable(fe->funcid, fpinfo->extensions))
385+
!is_shippable(fe->funcid, ProcedureRelationId, fpinfo->extensions))
386386
return false;
387387

388388
/*
@@ -431,7 +431,7 @@ foreign_expr_walker(Node *node,
431431
* too.)
432432
*/
433433
if (!is_builtin(oe->opno) &&
434-
!is_shippable(oe->opno, fpinfo->extensions))
434+
!is_shippable(oe->opno, OperatorRelationId, fpinfo->extensions))
435435
return false;
436436

437437
/*
@@ -472,7 +472,7 @@ foreign_expr_walker(Node *node,
472472
* Again, only built-in operators can be sent to remote.
473473
*/
474474
if (!is_builtin(oe->opno) &&
475-
!is_shippable(oe->opno, fpinfo->extensions))
475+
!is_shippable(oe->opno, OperatorRelationId, fpinfo->extensions))
476476
return false;
477477

478478
/*
@@ -624,7 +624,7 @@ foreign_expr_walker(Node *node,
624624
*/
625625
if (check_type &&
626626
!is_builtin(exprType(node)) &&
627-
!is_shippable(exprType(node), fpinfo->extensions))
627+
!is_shippable(exprType(node), TypeRelationId, fpinfo->extensions))
628628
return false;
629629

630630
/*
@@ -1445,7 +1445,7 @@ deparseConst(Const *node, deparse_expr_cxt *context)
14451445
* but references to built-in types shouldn't be.
14461446
*/
14471447
appendStringInfo(buf, "::%s",
1448-
is_shippable(node->consttype, fpinfo->extensions) ?
1448+
is_shippable(node->consttype, TypeRelationId, fpinfo->extensions) ?
14491449
format_type_be_qualified(node->consttype) :
14501450
format_type_with_typemod(node->consttype, node->consttypmod));
14511451
}

‎contrib/postgres_fdw/option.c

+4-4
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ postgres_fdw_validator(PG_FUNCTION_ARGS)
128128
}
129129
else if (strcmp(def->defname, "extensions") == 0)
130130
{
131-
/* this must have already-installed extensions */
131+
/* check that the requested extensions are actually installed */
132132
(void) ExtractExtensionList(defGetString(def), false);
133133
}
134134
}
@@ -157,10 +157,10 @@ InitPgFdwOptions(void)
157157
/* cost factors */
158158
{"fdw_startup_cost", ForeignServerRelationId, false},
159159
{"fdw_tuple_cost", ForeignServerRelationId, false},
160-
/* updatable is available on both server and table */
160+
/* updatable option is available on both server and table */
161161
{"updatable", ForeignServerRelationId, false},
162162
{"updatable", ForeignTableRelationId, false},
163-
/* extensions is available on server */
163+
/* "extensions" option is available on server */
164164
{"extensions", ForeignServerRelationId, false},
165165
{NULL, InvalidOid, false}
166166
};
@@ -307,7 +307,7 @@ ExtractConnectionOptions(List *defelems, const char **keywords,
307307
* Parse a comma-separated string and return a List of the Oids of the
308308
* extensions in the string. If an extension provided cannot be looked
309309
* up in the catalog (it hasn't been installed or doesn't exist) then
310-
* throw up an error.
310+
* raise an error.
311311
*/
312312
List *
313313
ExtractExtensionList(char *extensionString, bool populateList)

‎contrib/postgres_fdw/postgres_fdw.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ typedef struct PgFdwRelationInfo
4848
Cost fdw_startup_cost;
4949
Cost fdw_tuple_cost;
5050

51-
/* Optional extensions to support (list of oid) */
51+
/* Optional extensions to support (list of Oids). */
5252
List *extensions;
5353

5454
/* Cached catalog information. */
@@ -78,7 +78,7 @@ extern List *ExtractExtensionList(char *extensionString,
7878
bool populateList);
7979

8080
/* in shippable.c */
81-
extern bool is_shippable(Oid procnumber, List *extension_list);
81+
extern bool is_shippable(Oid objnumber, Oid classnumber, List *extension_list);
8282

8383
/* in deparse.c */
8484
extern void classifyConditions(PlannerInfo *root,

‎contrib/postgres_fdw/shippable.c

+14-8
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ typedef struct
3939
{
4040
/* extension the object appears within, or InvalidOid if none */
4141
Oid objid;
42+
Oid classid;
4243
} ShippableCacheKey;
4344

4445
typedef struct
@@ -89,24 +90,23 @@ InitializeShippableCache(void)
8990
}
9091

9192
/*
92-
* Returns true if given operator/function is part of an extension declared in
93+
* Returns true if given operator/function is part of an extension listed in
9394
* the server options.
9495
*/
9596
static bool
96-
lookup_shippable(Oid objnumber, List *extension_list)
97+
lookup_shippable(Oid objnumber, Oid classnumber, List *extension_list)
9798
{
98-
static int nkeys = 1;
99+
static int nkeys = 2;
99100
ScanKeyData key[nkeys];
100101
HeapTuple tup;
101102
Relation depRel;
102103
SysScanDesc scan;
103104
bool is_shippable = false;
104105

105-
/* Always return false if we don't have any declared extensions */
106+
/* Always return false if the user hasn't set the "extensions" option */
106107
if (extension_list == NIL)
107108
return false;
108109

109-
/* We need this relation to scan */
110110
depRel = heap_open(DependRelationId, RowExclusiveLock);
111111

112112
/*
@@ -115,6 +115,11 @@ lookup_shippable(Oid objnumber, List *extension_list)
115115
* is an extension declared by the user in the options
116116
*/
117117
ScanKeyInit(&key[0],
118+
Anum_pg_depend_classid,
119+
BTEqualStrategyNumber, F_OIDEQ,
120+
ObjectIdGetDatum(classnumber));
121+
122+
ScanKeyInit(&key[1],
118123
Anum_pg_depend_objid,
119124
BTEqualStrategyNumber, F_OIDEQ,
120125
ObjectIdGetDatum(objnumber));
@@ -147,12 +152,12 @@ lookup_shippable(Oid objnumber, List *extension_list)
147152
* part of a declared extension if it is not cached.
148153
*/
149154
bool
150-
is_shippable(Oid objnumber, List *extension_list)
155+
is_shippable(Oid objnumber, Oid classnumber, List *extension_list)
151156
{
152157
ShippableCacheKey key;
153158
ShippableCacheEntry *entry;
154159

155-
/* Always return false if we don't have any declared extensions */
160+
/* Always return false if the user hasn't set the "extensions" option */
156161
if (extension_list == NIL)
157162
return false;
158163

@@ -164,6 +169,7 @@ is_shippable(Oid objnumber, List *extension_list)
164169
memset(&key, 0, sizeof(key));
165170

166171
key.objid = objnumber;
172+
key.classid = classnumber;
167173

168174
entry = (ShippableCacheEntry *)
169175
hash_search(ShippableCacheHash,
@@ -180,7 +186,7 @@ is_shippable(Oid objnumber, List *extension_list)
180186
* In the future we could additionally have a whitelist of functions
181187
* declared one at a time.
182188
*/
183-
bool shippable = lookup_shippable(objnumber, extension_list);
189+
bool shippable = lookup_shippable(objnumber, classnumber, extension_list);
184190

185191
entry = (ShippableCacheEntry *)
186192
hash_search(ShippableCacheHash,

0 commit comments

Comments
 (0)