On Tue, Jul 21, 2015 at 11:29 AM, Paul Ramsey <[email protected]> wrote:
> On July 21, 2015 at 11:22:12 AM, Tom Lane ([email protected]) wrote:
>
> No, *not* populated first-time-through, because that won't handle any of
> the CREATE, DROP, or UPGRADE cases. It's also doing a lot of work you
> might never need. I was thinking of "populate on demand", that is, first
> time you need to know whether function X is shippable, you find that out
> and then cache the answer (whether it be positive or negative).
>
>
> Roger that. Off to the races..
Attached, reworked with a local cache. I felt a little dirty sticking
the cache entry right in postgres_fdw.c, so I broke out all my
nonsense into shippable.c.
Thanks!
P
diff --git a/contrib/postgres_fdw/Makefile b/contrib/postgres_fdw/Makefile
index d2b98e1..3543312 100644
--- a/contrib/postgres_fdw/Makefile
+++ b/contrib/postgres_fdw/Makefile
@@ -1,7 +1,7 @@
# contrib/postgres_fdw/Makefile
MODULE_big = postgres_fdw
-OBJS = postgres_fdw.o option.o deparse.o connection.o $(WIN32RES)
+OBJS = postgres_fdw.o option.o deparse.o connection.o shippable.o $(WIN32RES)
PGFILEDESC = "postgres_fdw - foreign data wrapper for PostgreSQL"
PG_CPPFLAGS = -I$(libpq_srcdir)
diff --git a/contrib/postgres_fdw/deparse.c b/contrib/postgres_fdw/deparse.c
index 81cb2b4..d6fff30 100644
--- a/contrib/postgres_fdw/deparse.c
+++ b/contrib/postgres_fdw/deparse.c
@@ -229,6 +229,9 @@ foreign_expr_walker(Node *node,
Oid collation;
FDWCollateState state;
+ /* Access extension metadata from fpinfo on baserel */
+ PgFdwRelationInfo *fpinfo = (PgFdwRelationInfo
*)(glob_cxt->foreignrel->fdw_private);
+
/* Need do nothing for empty subexpressions */
if (node == NULL)
return true;
@@ -361,7 +364,7 @@ foreign_expr_walker(Node *node,
* can't be sent to remote because it might
have incompatible
* semantics on remote side.
*/
- if (!is_builtin(fe->funcid))
+ if (!is_builtin(fe->funcid) &&
!is_shippable(fe->funcid, fpinfo))
return false;
/*
@@ -407,7 +410,7 @@ foreign_expr_walker(Node *node,
* (If the operator is, surely its underlying
function is
* too.)
*/
- if (!is_builtin(oe->opno))
+ if (!is_builtin(oe->opno) &&
!is_shippable(oe->opno, fpinfo))
return false;
/*
@@ -445,7 +448,7 @@ foreign_expr_walker(Node *node,
/*
* Again, only built-in operators can be sent
to remote.
*/
- if (!is_builtin(oe->opno))
+ if (!is_builtin(oe->opno) &&
!is_shippable(oe->opno, fpinfo))
return false;
/*
@@ -591,7 +594,7 @@ foreign_expr_walker(Node *node,
* If result type of given expression is not built-in, it can't be sent
to
* remote because it might have incompatible semantics on remote side.
*/
- if (check_type && !is_builtin(exprType(node)))
+ if (check_type && !is_builtin(exprType(node)) &&
!is_shippable(exprType(node), fpinfo))
return false;
/*
@@ -1404,8 +1407,7 @@ deparseConst(Const *node, deparse_expr_cxt *context)
}
if (needlabel)
appendStringInfo(buf, "::%s",
-
format_type_with_typemod(node->consttype,
-
node->consttypmod));
+ format_type_be_qualified(node->consttype));
}
/*
diff --git a/contrib/postgres_fdw/option.c b/contrib/postgres_fdw/option.c
index 7547ec2..9aeaf1a 100644
--- a/contrib/postgres_fdw/option.c
+++ b/contrib/postgres_fdw/option.c
@@ -15,6 +15,7 @@
#include "postgres_fdw.h"
#include "access/reloptions.h"
+#include "catalog/pg_foreign_data_wrapper.h"
#include "catalog/pg_foreign_server.h"
#include "catalog/pg_foreign_table.h"
#include "catalog/pg_user_mapping.h"
@@ -124,6 +125,10 @@ postgres_fdw_validator(PG_FUNCTION_ARGS)
errmsg("%s requires a
non-negative numeric value",
def->defname)));
}
+ else if (strcmp(def->defname, "extensions") == 0)
+ {
+ extractExtensionList(defGetString(def), NULL);
+ }
}
PG_RETURN_VOID();
@@ -153,6 +158,9 @@ InitPgFdwOptions(void)
/* updatable is available on both server and table */
{"updatable", ForeignServerRelationId, false},
{"updatable", ForeignTableRelationId, false},
+ /* extensions is available on both wrapper and server */
+ {"extensions", ForeignServerRelationId, false},
+ {"extensions", ForeignDataWrapperRelationId, false},
{NULL, InvalidOid, false}
};
diff --git a/contrib/postgres_fdw/postgres_fdw.c
b/contrib/postgres_fdw/postgres_fdw.c
index e4d799c..42d7e25 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -47,39 +47,6 @@ PG_MODULE_MAGIC;
/* Default CPU cost to process 1 row (above and beyond cpu_tuple_cost). */
#define DEFAULT_FDW_TUPLE_COST 0.01
-/*
- * FDW-specific planner information kept in RelOptInfo.fdw_private for a
- * foreign table. This information is collected by postgresGetForeignRelSize.
- */
-typedef struct PgFdwRelationInfo
-{
- /* baserestrictinfo clauses, broken down into safe and unsafe subsets.
*/
- List *remote_conds;
- List *local_conds;
-
- /* Bitmap of attr numbers we need to fetch from the remote server. */
- Bitmapset *attrs_used;
-
- /* Cost and selectivity of local_conds. */
- QualCost local_conds_cost;
- Selectivity local_conds_sel;
-
- /* Estimated size and cost for a scan with baserestrictinfo quals. */
- double rows;
- int width;
- Cost startup_cost;
- Cost total_cost;
-
- /* Options extracted from catalogs. */
- bool use_remote_estimate;
- Cost fdw_startup_cost;
- Cost fdw_tuple_cost;
-
- /* Cached catalog information. */
- ForeignTable *table;
- ForeignServer *server;
- UserMapping *user; /* only set in
use_remote_estimate mode */
-} PgFdwRelationInfo;
/*
* Indexes of FDW-private information stored in fdw_private lists.
@@ -397,6 +364,7 @@ postgresGetForeignRelSize(PlannerInfo *root,
/* Look up foreign-table catalog info. */
fpinfo->table = GetForeignTable(foreigntableid);
fpinfo->server = GetForeignServer(fpinfo->table->serverid);
+ fpinfo->wrapper = GetForeignDataWrapper(fpinfo->server->fdwid);
/*
* Extract user-settable option values. Note that per-table setting of
@@ -405,7 +373,15 @@ postgresGetForeignRelSize(PlannerInfo *root,
fpinfo->use_remote_estimate = false;
fpinfo->fdw_startup_cost = DEFAULT_FDW_STARTUP_COST;
fpinfo->fdw_tuple_cost = DEFAULT_FDW_TUPLE_COST;
+ fpinfo->extensions = NIL;
+ foreach(lc, fpinfo->wrapper->options)
+ {
+ DefElem *def = (DefElem *) lfirst(lc);
+
+ if (strcmp(def->defname, "extensions") == 0)
+ extractExtensionList(defGetString(def),
&(fpinfo->extensions));
+ }
foreach(lc, fpinfo->server->options)
{
DefElem *def = (DefElem *) lfirst(lc);
@@ -416,6 +392,8 @@ postgresGetForeignRelSize(PlannerInfo *root,
fpinfo->fdw_startup_cost = strtod(defGetString(def),
NULL);
else if (strcmp(def->defname, "fdw_tuple_cost") == 0)
fpinfo->fdw_tuple_cost = strtod(defGetString(def),
NULL);
+ else if (strcmp(def->defname, "extensions") == 0)
+ extractExtensionList(defGetString(def),
&(fpinfo->extensions));
}
foreach(lc, fpinfo->table->options)
{
diff --git a/contrib/postgres_fdw/postgres_fdw.h
b/contrib/postgres_fdw/postgres_fdw.h
index 3835ddb..8e16ee9 100644
--- a/contrib/postgres_fdw/postgres_fdw.h
+++ b/contrib/postgres_fdw/postgres_fdw.h
@@ -20,6 +20,44 @@
#include "libpq-fe.h"
+/*
+ * FDW-specific planner information kept in RelOptInfo.fdw_private for a
+ * foreign table. This information is collected by postgresGetForeignRelSize.
+ */
+typedef struct PgFdwRelationInfo
+{
+ /* baserestrictinfo clauses, broken down into safe and unsafe subsets.
*/
+ List *remote_conds;
+ List *local_conds;
+
+ /* Bitmap of attr numbers we need to fetch from the remote server. */
+ Bitmapset *attrs_used;
+
+ /* Cost and selectivity of local_conds. */
+ QualCost local_conds_cost;
+ Selectivity local_conds_sel;
+
+ /* Estimated size and cost for a scan with baserestrictinfo quals. */
+ double rows;
+ int width;
+ Cost startup_cost;
+ Cost total_cost;
+
+ /* Options extracted from catalogs. */
+ bool use_remote_estimate;
+ Cost fdw_startup_cost;
+ Cost fdw_tuple_cost;
+
+ /* Optional extensions to support (list of oid) */
+ List *extensions;
+
+ /* Cached catalog information. */
+ ForeignDataWrapper *wrapper;
+ ForeignTable *table;
+ ForeignServer *server;
+ UserMapping *user; /* only set in
use_remote_estimate mode */
+} PgFdwRelationInfo;
+
/* in postgres_fdw.c */
extern int set_transmission_modes(void);
extern void reset_transmission_modes(int nestlevel);
@@ -38,6 +76,11 @@ extern int ExtractConnectionOptions(List *defelems,
const char **keywords,
const char **values);
+/* in shippable.c */
+extern bool extractExtensionList(char *extensionString,
+ List **extensionOids);
+extern bool is_shippable(Oid procnumber, PgFdwRelationInfo *fpinfo);
+
/* in deparse.c */
extern void classifyConditions(PlannerInfo *root,
RelOptInfo *baserel,
diff --git a/contrib/postgres_fdw/shippable.c b/contrib/postgres_fdw/shippable.c
new file mode 100644
index 0000000..29745ed
--- /dev/null
+++ b/contrib/postgres_fdw/shippable.c
@@ -0,0 +1,235 @@
+/*-------------------------------------------------------------------------
+ *
+ * shippable.c
+ * Non-built-in objects cache management and utilities.
+ *
+ * Is a non-built-in shippable to the remote server? Only if
+ * the object is in an extension declared by the user in the
+ * OPTIONS of the wrapper or the server.
+ *
+ * Portions Copyright (c) 1996-2015, PostgreSQL Global Development Group
+ *
+ * IDENTIFICATION
+ * contrib/postgres_fdw/shippable.c
+ *
+ *-------------------------------------------------------------------------
+ */
+
+#include "postgres.h"
+
+#include "postgres_fdw.h"
+
+#include "access/genam.h"
+#include "access/heapam.h"
+#include "access/htup_details.h"
+#include "catalog/dependency.h"
+#include "catalog/indexing.h"
+#include "catalog/pg_depend.h"
+#include "commands/extension.h"
+#include "utils/builtins.h"
+#include "utils/fmgroids.h"
+#include "utils/hsearch.h"
+#include "utils/rel.h"
+#include "utils/snapmgr.h"
+
+/* Hash table for informations about remote objects we'll call */
+static HTAB *ShippableCacheHash = NULL;
+
+/* objid is the lookup key, must appear first */
+typedef struct
+{
+ Oid objid;
+} ShippableCacheKey;
+
+typedef struct
+{
+ ShippableCacheKey key; /* lookup key - must be first */
+ bool shippable; /* extension the object appears within, or
InvalidOid if none */
+} ShippableCacheEntry;
+
+/*
+ * InitializeShippableCache
+ * Initialize the cache of functions we can ship to remote server.
+ */
+static void
+InitializeShippableCache(void)
+{
+ HASHCTL ctl;
+
+ /* Initialize the hash table. */
+ MemSet(&ctl, 0, sizeof(ctl));
+ ctl.keysize = sizeof(ShippableCacheKey);
+ ctl.entrysize = sizeof(ShippableCacheEntry);
+ ShippableCacheHash =
+ hash_create("Shippable cache", 256, &ctl, HASH_ELEM);
+}
+
+/*
+ * Returns true if given operator/function is part of an extension declared in
the
+ * server options.
+ */
+static bool
+lookup_shippable(Oid objnumber, List *extension_list)
+{
+ static int nkeys = 1;
+ ScanKeyData key[nkeys];
+ HeapTuple tup;
+ Relation depRel;
+ SysScanDesc scan;
+ int nresults = 0;
+
+ /* Always return false if we don't have any declared extensions */
+ if (!extension_list)
+ return false;
+
+ /* We need this relation to scan */
+ depRel = heap_open(DependRelationId, RowExclusiveLock);
+
+ /* Scan the system dependency table for all entries this object */
+ /* depends on, then iterate through and see if one of them */
+ /* is an extension declared by the user in the options */
+ ScanKeyInit(&key[0],
+ Anum_pg_depend_objid,
+ BTEqualStrategyNumber, F_OIDEQ,
+ ObjectIdGetDatum(objnumber));
+
+ scan = systable_beginscan(depRel, DependDependerIndexId, true,
+
GetCatalogSnapshot(depRel->rd_id), nkeys, key);
+
+ while (HeapTupleIsValid(tup = systable_getnext(scan)))
+ {
+ Form_pg_depend foundDep = (Form_pg_depend) GETSTRUCT(tup);
+
+ if (foundDep->deptype == DEPENDENCY_EXTENSION)
+ {
+ ListCell *ext;
+
+ foreach(ext, extension_list)
+ {
+ Oid extension_oid = (Oid) lfirst(ext);
+ if (foundDep->refobjid == extension_oid)
+ {
+ nresults++;
+ }
+ }
+ }
+ if (nresults > 0) break;
+ }
+
+ systable_endscan(scan);
+ relation_close(depRel, RowExclusiveLock);
+
+ return nresults > 0;
+}
+
+/*
+ * is_shippable
+ * Is this object (proc/op/type) shippable to foreign server?
+ * Check cache first, then look-up whether (proc/op/type) is
+ * part of a declared extension if it is not cached.
+ */
+bool
+is_shippable(Oid objnumber, PgFdwRelationInfo *fpinfo)
+{
+ ShippableCacheKey key;
+ ShippableCacheEntry *entry;
+
+ /* Always return false if we don't have any declared extensions */
+ if (!fpinfo->extensions)
+ return false;
+
+ /* Find existing cache, if any. */
+ if (!ShippableCacheHash)
+ InitializeShippableCache();
+
+ /* Zero out the key */
+ memset(&key, 0, sizeof(key));
+
+ key.objid = objnumber;
+
+ entry = (ShippableCacheEntry *)
+ hash_search(ShippableCacheHash,
+ (void *) &key,
+ HASH_FIND,
+ NULL);
+
+ /* Not found in ShippableCacheHash cache. Construct new entry. */
+ if (!entry)
+ {
+ /* Right now "shippability" is exclusively a function of
whether */
+ /* the obj (proc/op/type) is in an extension declared by the
user. In the future */
+ /* we could additionally have a whitelist of functions declared
one */
+ /* at a time. */
+ bool shippable = lookup_shippable(objnumber,
fpinfo->extensions);
+
+ entry = (ShippableCacheEntry *)
+ hash_search(ShippableCacheHash,
+ (void *) &key,
+ HASH_ENTER,
+ NULL);
+
+ entry->shippable = shippable;
+ }
+
+ if (!entry)
+ return false;
+ else
+ return entry->shippable;
+}
+
+/*
+ * extractExtensionList
+ * Parse a comma-separated string and fill out the list
+ * argument with the Oids of the extensions in the string.
+ * If an extenstion provided cannot be looked up in the
+ * catalog (it hasn't been installed or doesn't exist)
+ * then throw an error.
+ */
+bool
+extractExtensionList(char *extensionString, List **extensionOids)
+{
+ List *extlist;
+ ListCell *l, *o;
+
+ if (!SplitIdentifierString(extensionString, ',', &extlist))
+ {
+ list_free(extlist);
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("unable to parse extension list \"%s\"",
+ extensionString)));
+ }
+
+ foreach(l, extlist)
+ {
+ const char *extension_name = (const char *) lfirst(l);
+ Oid extension_oid = get_extension_oid(extension_name, true);
+ if (extension_oid == InvalidOid)
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("the \"%s\" extension must be installed
locally before it can be used on a remote server",
+ extension_name)));
+ }
+ /* Option validation calls this function with NULL in the */
+ /* extensionOids parameter, to just do existence/syntax */
+ /* checking of the option */
+ else if (extensionOids)
+ {
+ bool found = false;
+ /* Only add this extension Oid to the list */
+ /* if we don't already have it in the list */
+ foreach(o, *extensionOids)
+ {
+ Oid oid = (Oid) lfirst(o);
+ if (oid == extension_oid)
+ found = true;
+ }
+ if (!found)
+ *extensionOids = lappend_oid(*extensionOids,
extension_oid);
+ }
+ }
+
+ list_free(extlist);
+ return true;
+}
diff --git a/doc/src/sgml/postgres-fdw.sgml b/doc/src/sgml/postgres-fdw.sgml
index 14b12e3..7b3d8c7 100644
--- a/doc/src/sgml/postgres-fdw.sgml
+++ b/doc/src/sgml/postgres-fdw.sgml
@@ -373,6 +373,37 @@
foreign tables, see <xref linkend="sql-createforeigntable">.
</para>
</sect3>
+
+ <sect3>
+ <title>Extension Options</title>
+
+ <para>
+ By default only built-in operators and functions will be sent from the
+ local to the foreign server. This may be overridden using the following
option:
+ </para>
+
+ <variablelist>
+
+ <varlistentry>
+ <term><literal>extensions</literal></term>
+ <listitem>
+ <para>
+ This option allows you to declare what extensions you expect are
+ installed on the foreign server, using a comma-separated list of
+ extension names. The extensions are also expected to be installed
+ on the local server too.
+ </para>
+<programlisting>
+CREATE SERVER foreign_server
+ FOREIGN DATA WRAPPER postgres_fdw
+ OPTIONS (host '127.0.0.1', port '5432', dbname 'my_db', extensions
'cube, seg');
+</programlisting>
+ </listitem>
+ </varlistentry>
+
+ </variablelist>
+ </sect3>
+
</sect2>
<sect2>
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers