On 2016/04/05 0:23, Tom Lane wrote:
> Amit Langote <[email protected]> writes:
>> On Mon, Apr 4, 2016 at 11:24 PM, Tom Lane <[email protected]> wrote:
>>> A related issue, now that I've seen this example, is that altering
>>> FDW-level or server-level options won't cause a replan either. I'm
>>> not sure there's any very good fix for that. Surely we don't want
>>> to try to identify all tables belonging to the FDW or server and
>>> issue relcache invals on all of them.
>
>> Hm, some kind of PlanInvalItem-based solution could work maybe?
>
> Hm, so we'd expect that whenever an FDW consulted the options while
> making a plan, it'd have to record a plan dependency on them? That
> would be a clean fix maybe, but I'm worried that third-party FDWs
> would fail to get the word about needing to do this.
I would imagine that that level of granularity may be a little too much; I
mean tracking dependencies at the level of individual FDW/foreign
table/foreign server options. I think it should really be possible to do
the entire thing in core instead of requiring this to be made a concern of
FDW authors. How about the attached that teaches
extract_query_dependencies() to add a foreign table and associated foreign
data wrapper and foreign server to invalItems. Also, it adds plan cache
callbacks for respective caches.
One thing that I observed that may not be all that surprising is that we
may need a similar mechanism for postgres_fdw's connection cache, which
doesn't drop connections using older server connection info after I alter
them. I was trying to test my patch by altering dbaname option of a
foreign server but that was silly, ;). Although, I did confirm that the
patch works by altering use_remote_estimates server option. I could not
really test for FDW options though.
Thoughts?
Thanks,
Amit
diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
index dd2b9ed..e7ddc14 100644
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -16,7 +16,9 @@
#include "postgres.h"
#include "access/transam.h"
+#include "catalog/pg_class.h"
#include "catalog/pg_type.h"
+#include "foreign/foreign.h"
#include "nodes/makefuncs.h"
#include "nodes/nodeFuncs.h"
#include "optimizer/pathnode.h"
@@ -150,6 +152,12 @@ static List *set_returning_clause_references(PlannerInfo *root,
static bool fix_opfuncids_walker(Node *node, void *context);
static bool extract_query_dependencies_walker(Node *node,
PlannerInfo *context);
+static void record_plan_foreign_table_dependency(PlannerInfo *root,
+ Oid ftid);
+static void record_plan_foreign_data_wrapper_dependency(PlannerInfo *root,
+ Oid fdwid);
+static void record_plan_foreign_server_dependency(PlannerInfo *root,
+ Oid serverid);
/*****************************************************************************
*
@@ -2652,6 +2660,54 @@ record_plan_function_dependency(PlannerInfo *root, Oid funcid)
}
/*
+ * record_plan_foreign_table_dependency
+ * Mark the current plan as depending on a particular foreign table.
+ */
+static void
+record_plan_foreign_table_dependency(PlannerInfo *root, Oid ftid)
+{
+ PlanInvalItem *inval_item = makeNode(PlanInvalItem);
+
+ inval_item->cacheId = FOREIGNTABLEREL;
+ inval_item->hashValue = GetSysCacheHashValue1(FOREIGNTABLEREL,
+ ObjectIdGetDatum(ftid));
+
+ root->glob->invalItems = lappend(root->glob->invalItems, inval_item);
+}
+
+/*
+ * record_plan_foreign_data_wrapper_dependency
+ * Mark the current plan as depending on a particular FDW.
+ */
+static void
+record_plan_foreign_data_wrapper_dependency(PlannerInfo *root, Oid fdwid)
+{
+ PlanInvalItem *inval_item = makeNode(PlanInvalItem);
+
+ inval_item->cacheId = FOREIGNDATAWRAPPEROID;
+ inval_item->hashValue = GetSysCacheHashValue1(FOREIGNDATAWRAPPEROID,
+ ObjectIdGetDatum(fdwid));
+
+ root->glob->invalItems = lappend(root->glob->invalItems, inval_item);
+}
+
+/*
+ * record_plan_foreign_server_dependency
+ * Mark the current plan as depending on a particular FDW.
+ */
+static void
+record_plan_foreign_server_dependency(PlannerInfo *root, Oid serverid)
+{
+ PlanInvalItem *inval_item = makeNode(PlanInvalItem);
+
+ inval_item->cacheId = FOREIGNSERVEROID;
+ inval_item->hashValue = GetSysCacheHashValue1(FOREIGNSERVEROID,
+ ObjectIdGetDatum(serverid));
+
+ root->glob->invalItems = lappend(root->glob->invalItems, inval_item);
+}
+
+/*
* extract_query_dependencies
* Given a not-yet-planned query or queries (i.e. a Query node or list
* of Query nodes), extract dependencies just as set_plan_references
@@ -2723,6 +2779,22 @@ extract_query_dependencies_walker(Node *node, PlannerInfo *context)
if (rte->rtekind == RTE_RELATION)
context->glob->relationOids =
lappend_oid(context->glob->relationOids, rte->relid);
+
+ if (rte->relkind == RELKIND_FOREIGN_TABLE)
+ {
+ ForeignTable *ftable;
+ ForeignServer *fserver;
+
+ ftable = GetForeignTable(rte->relid);
+ fserver = GetForeignServer(ftable->serverid);
+
+ record_plan_foreign_table_dependency(context,
+ ftable->relid);
+ record_plan_foreign_server_dependency(context,
+ ftable->serverid);
+ record_plan_foreign_data_wrapper_dependency(context,
+ fserver->fdwid);
+ }
}
/* And recurse into the query's subexpressions */
diff --git a/src/backend/utils/cache/plancache.c b/src/backend/utils/cache/plancache.c
index 8fd9f2b..a1af0c1 100644
--- a/src/backend/utils/cache/plancache.c
+++ b/src/backend/utils/cache/plancache.c
@@ -124,6 +124,10 @@ InitPlanCache(void)
CacheRegisterSyscacheCallback(AMOPOPID, PlanCacheSysCallback, (Datum) 0);
/* User mapping change may invalidate plans with pushed down foreign join */
CacheRegisterSyscacheCallback(USERMAPPINGOID, PlanCacheUserMappingCallback, (Datum) 0);
+ /* Foreign table/data wrapper/server alterations invalidate any related plans */
+ CacheRegisterSyscacheCallback(FOREIGNTABLEREL, PlanCacheFuncCallback, (Datum) 0);
+ CacheRegisterSyscacheCallback(FOREIGNDATAWRAPPEROID, PlanCacheFuncCallback, (Datum) 0);
+ CacheRegisterSyscacheCallback(FOREIGNSERVEROID, PlanCacheFuncCallback, (Datum) 0);
}
/*
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
