On Wed, Jul 22, 2015 at 12:19 PM, Paul Ramsey <pram...@cleverelephant.ca> wrote:
>
> I’ll have a look at doing invalidation for the case of changes to the FDW
> wrappers and servers.

Here's an updated patch that clears the cache on changes to foreign
wrappers and servers.
In testing it I came across an unrelated issue which could make it
hard for users to manage the options on their wrappers/servers

fdw=# ALTER SERVER foreign_server OPTIONS ( extensions 'postgis' );
ALTER SERVER
fdw=# ALTER SERVER foreign_server OPTIONS ( extensions 'postgis,seg' );
ERROR:  option "extensions" provided more than once

Once set, an option seems to be effectively immutable.
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..50c0ea2
--- /dev/null
+++ b/contrib/postgres_fdw/shippable.c
@@ -0,0 +1,268 @@
+/*-------------------------------------------------------------------------
+ *
+ * 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/inval.h"
+#include "utils/rel.h"
+#include "utils/snapmgr.h"
+#include "utils/syscache.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;
+
+/*
+ * InvalidateShippableCacheCallback
+ *             Flush all cache entries when pg_foreign_data_wrapper
+ *      or pg_foreign_server is updated.
+ */
+static void
+InvalidateShippableCacheCallback(Datum arg, int cacheid, uint32 hashvalue)
+{
+       HASH_SEQ_STATUS status;
+       ShippableCacheEntry *entry;
+
+       hash_seq_init(&status, ShippableCacheHash);
+       while ((entry = (ShippableCacheEntry *) hash_seq_search(&status)) != 
NULL)
+       {
+               if (hash_search(ShippableCacheHash,
+                                               (void *) &entry->key,
+                                               HASH_REMOVE,
+                                               NULL) == NULL)
+                       elog(ERROR, "hash table corrupted");
+       }
+}
+
+/*
+ * 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);
+
+       /* Watch for invalidation events. */
+       CacheRegisterSyscacheCallback(FOREIGNDATAWRAPPEROID,
+                                                                 
InvalidateShippableCacheCallback,
+                                                                 (Datum) 0);
+
+       CacheRegisterSyscacheCallback(FOREIGNSERVEROID,
+                                                                 
InvalidateShippableCacheCallback,
+                                                                 (Datum) 0);
+}
+
+/*
+ * 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 (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to