On 2016/04/05 0:23, Tom Lane wrote: > Amit Langote <amitlangot...@gmail.com> writes: >> On Mon, Apr 4, 2016 at 11:24 PM, Tom Lane <t...@sss.pgh.pa.us> 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 (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers