On Tue, Apr 5, 2016 at 10:54 AM, Amit Langote
<langote_amit...@lab.ntt.co.jp> wrote:
> 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.
Although this is a better solution than the previous one, it
invalidates the query tree along with the generic plan. Invalidating
the query tree and the generic plan required parsing the query again
and generating the plan. Instead I think, we should invalidate only
the generic plan and not the query tree. The attached patch adds the
dependencies from create_foreignscan_plan() which is called for any
foreign access. I have also added testcases to test the functionality.
Let me know your comments on the patch.
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index d97e694..7ca1102 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -2480,26 +2480,114 @@ EXPLAIN (VERBOSE, COSTS OFF) EXECUTE st5('foo', 1);
Filter: (t1.c8 = $1)
Remote SQL: SELECT "C 1", c2, c3, c4, c5, c6, c7, c8 FROM "S 1"."T 1" WHERE (("C 1" = $1::integer))
(4 rows)
EXECUTE st5('foo', 1);
c1 | c2 | c3 | c4 | c5 | c6 | c7 | c8
----+----+-------+------------------------------+--------------------------+----+------------+-----
1 | 1 | 00001 | Fri Jan 02 00:00:00 1970 PST | Fri Jan 02 00:00:00 1970 | 1 | 1 | foo
(1 row)
+-- changing foreign table option should change the plan
+PREPARE st6 AS SELECT * FROM ft4;
+PREPARE st7 AS UPDATE ft4 SET c1 = c1;
+PREPARE st8 AS UPDATE ft4 SET c1 = random();
+EXPLAIN (VERBOSE, COSTS OFF) EXECUTE st6;
+ QUERY PLAN
+--------------------------------------------------
+ Foreign Scan on public.ft4
+ Output: c1, c2, c3
+ Remote SQL: SELECT c1, c2, c3 FROM "S 1"."T 3"
+(3 rows)
+
+EXPLAIN (VERBOSE, COSTS OFF) EXECUTE st7;
+ QUERY PLAN
+----------------------------------------------------
+ Update on public.ft4
+ -> Foreign Update on public.ft4
+ Remote SQL: UPDATE "S 1"."T 3" SET c1 = c1
+(3 rows)
+
+EXPLAIN (VERBOSE, COSTS OFF) EXECUTE st8;
+ QUERY PLAN
+---------------------------------------------------------------------
+ Update on public.ft4
+ Remote SQL: UPDATE "S 1"."T 3" SET c1 = $2 WHERE ctid = $1
+ -> Foreign Scan on public.ft4
+ Output: random(), c2, c3, ctid
+ Remote SQL: SELECT c2, c3, ctid FROM "S 1"."T 3" FOR UPDATE
+(5 rows)
+
+ALTER FOREIGN TABLE ft4 OPTIONS (SET table_name 'T 4');
+EXPLAIN (VERBOSE, COSTS OFF) EXECUTE st6;
+ QUERY PLAN
+--------------------------------------------------
+ Foreign Scan on public.ft4
+ Output: c1, c2, c3
+ Remote SQL: SELECT c1, c2, c3 FROM "S 1"."T 4"
+(3 rows)
+
+EXPLAIN (VERBOSE, COSTS OFF) EXECUTE st7;
+ QUERY PLAN
+----------------------------------------------------
+ Update on public.ft4
+ -> Foreign Update on public.ft4
+ Remote SQL: UPDATE "S 1"."T 4" SET c1 = c1
+(3 rows)
+
+EXPLAIN (VERBOSE, COSTS OFF) EXECUTE st8;
+ QUERY PLAN
+---------------------------------------------------------------------
+ Update on public.ft4
+ Remote SQL: UPDATE "S 1"."T 4" SET c1 = $2 WHERE ctid = $1
+ -> Foreign Scan on public.ft4
+ Output: random(), c2, c3, ctid
+ Remote SQL: SELECT c2, c3, ctid FROM "S 1"."T 4" FOR UPDATE
+(5 rows)
+
+-- restore the original options and check again
+ALTER FOREIGN TABLE ft4 OPTIONS (SET table_name 'T 3');
+EXPLAIN (VERBOSE, COSTS OFF) EXECUTE st6;
+ QUERY PLAN
+--------------------------------------------------
+ Foreign Scan on public.ft4
+ Output: c1, c2, c3
+ Remote SQL: SELECT c1, c2, c3 FROM "S 1"."T 3"
+(3 rows)
+
+EXPLAIN (VERBOSE, COSTS OFF) EXECUTE st7;
+ QUERY PLAN
+----------------------------------------------------
+ Update on public.ft4
+ -> Foreign Update on public.ft4
+ Remote SQL: UPDATE "S 1"."T 3" SET c1 = c1
+(3 rows)
+
+EXPLAIN (VERBOSE, COSTS OFF) EXECUTE st8;
+ QUERY PLAN
+---------------------------------------------------------------------
+ Update on public.ft4
+ Remote SQL: UPDATE "S 1"."T 3" SET c1 = $2 WHERE ctid = $1
+ -> Foreign Scan on public.ft4
+ Output: random(), c2, c3, ctid
+ Remote SQL: SELECT c2, c3, ctid FROM "S 1"."T 3" FOR UPDATE
+(5 rows)
+
-- cleanup
DEALLOCATE st1;
DEALLOCATE st2;
DEALLOCATE st3;
DEALLOCATE st4;
DEALLOCATE st5;
+DEALLOCATE st6;
+DEALLOCATE st7;
+DEALLOCATE st8;
-- System columns, except ctid and oid, should not be sent to remote
EXPLAIN (VERBOSE, COSTS OFF)
SELECT * FROM ft1 t1 WHERE t1.tableoid = 'pg_class'::regclass LIMIT 1;
QUERY PLAN
-------------------------------------------------------------------------------
Limit
Output: c1, c2, c3, c4, c5, c6, c7, c8
-> Foreign Scan on public.ft1 t1
Output: c1, c2, c3, c4, c5, c6, c7, c8
Filter: (t1.tableoid = '1259'::oid)
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
index 4f68e89..9788606 100644
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -570,27 +570,46 @@ EXPLAIN (VERBOSE, COSTS OFF) EXECUTE st4(1);
EXPLAIN (VERBOSE, COSTS OFF) EXECUTE st4(1);
-- value of $1 should not be sent to remote
PREPARE st5(user_enum,int) AS SELECT * FROM ft1 t1 WHERE c8 = $1 and c1 = $2;
EXPLAIN (VERBOSE, COSTS OFF) EXECUTE st5('foo', 1);
EXPLAIN (VERBOSE, COSTS OFF) EXECUTE st5('foo', 1);
EXPLAIN (VERBOSE, COSTS OFF) EXECUTE st5('foo', 1);
EXPLAIN (VERBOSE, COSTS OFF) EXECUTE st5('foo', 1);
EXPLAIN (VERBOSE, COSTS OFF) EXECUTE st5('foo', 1);
EXPLAIN (VERBOSE, COSTS OFF) EXECUTE st5('foo', 1);
EXECUTE st5('foo', 1);
+-- changing foreign table option should change the plan
+PREPARE st6 AS SELECT * FROM ft4;
+PREPARE st7 AS UPDATE ft4 SET c1 = c1;
+PREPARE st8 AS UPDATE ft4 SET c1 = random();
+EXPLAIN (VERBOSE, COSTS OFF) EXECUTE st6;
+EXPLAIN (VERBOSE, COSTS OFF) EXECUTE st7;
+EXPLAIN (VERBOSE, COSTS OFF) EXECUTE st8;
+ALTER FOREIGN TABLE ft4 OPTIONS (SET table_name 'T 4');
+EXPLAIN (VERBOSE, COSTS OFF) EXECUTE st6;
+EXPLAIN (VERBOSE, COSTS OFF) EXECUTE st7;
+EXPLAIN (VERBOSE, COSTS OFF) EXECUTE st8;
+-- restore the original options and check again
+ALTER FOREIGN TABLE ft4 OPTIONS (SET table_name 'T 3');
+EXPLAIN (VERBOSE, COSTS OFF) EXECUTE st6;
+EXPLAIN (VERBOSE, COSTS OFF) EXECUTE st7;
+EXPLAIN (VERBOSE, COSTS OFF) EXECUTE st8;
-- cleanup
DEALLOCATE st1;
DEALLOCATE st2;
DEALLOCATE st3;
DEALLOCATE st4;
DEALLOCATE st5;
+DEALLOCATE st6;
+DEALLOCATE st7;
+DEALLOCATE st8;
-- System columns, except ctid and oid, should not be sent to remote
EXPLAIN (VERBOSE, COSTS OFF)
SELECT * FROM ft1 t1 WHERE t1.tableoid = 'pg_class'::regclass LIMIT 1;
SELECT * FROM ft1 t1 WHERE t1.tableoid = 'ft1'::regclass LIMIT 1;
EXPLAIN (VERBOSE, COSTS OFF)
SELECT tableoid::regclass, * FROM ft1 t1 LIMIT 1;
SELECT tableoid::regclass, * FROM ft1 t1 LIMIT 1;
EXPLAIN (VERBOSE, COSTS OFF)
SELECT * FROM ft1 t1 WHERE t1.ctid = '(0,2)';
diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index 47158f6..2d53435 100644
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -16,39 +16,41 @@
*/
#include "postgres.h"
#include <limits.h>
#include <math.h>
#include "access/stratnum.h"
#include "access/sysattr.h"
#include "catalog/pg_class.h"
#include "foreign/fdwapi.h"
+#include "foreign/foreign.h"
#include "miscadmin.h"
#include "nodes/extensible.h"
#include "nodes/makefuncs.h"
#include "nodes/nodeFuncs.h"
#include "optimizer/clauses.h"
#include "optimizer/cost.h"
#include "optimizer/paths.h"
#include "optimizer/placeholder.h"
#include "optimizer/plancat.h"
#include "optimizer/planmain.h"
#include "optimizer/planner.h"
#include "optimizer/predtest.h"
#include "optimizer/restrictinfo.h"
#include "optimizer/subselect.h"
#include "optimizer/tlist.h"
#include "optimizer/var.h"
#include "parser/parse_clause.h"
#include "parser/parsetree.h"
#include "utils/lsyscache.h"
+#include "utils/syscache.h"
/*
* Flag bits that can appear in the flags argument of create_plan_recurse().
* These can be OR-ed together.
*
* CP_EXACT_TLIST specifies that the generated plan node must return exactly
* the tlist specified by the path's pathtarget (this overrides both
* CP_SMALL_TLIST and CP_LABEL_TLIST, if those are set). Otherwise, the
* plan node is allowed to return just the Vars and PlaceHolderVars needed
@@ -263,20 +265,24 @@ static SetOp *make_setop(SetOpCmd cmd, SetOpStrategy strategy, Plan *lefttree,
List *distinctList, AttrNumber flagColIdx, int firstFlag,
long numGroups);
static LockRows *make_lockrows(Plan *lefttree, List *rowMarks, int epqParam);
static Result *make_result(List *tlist, Node *resconstantqual, Plan *subplan);
static ModifyTable *make_modifytable(PlannerInfo *root,
CmdType operation, bool canSetTag,
Index nominalRelation,
List *resultRelations, List *subplans,
List *withCheckOptionLists, List *returningLists,
List *rowMarks, OnConflictExpr *onconflict, int epqParam);
+static void record_plan_foreign_object_dependency(PlannerInfo *root, Oid oid,
+ int cacheId);
+static void record_foreignscan_plan_dependencies(PlannerInfo *root,
+ ForeignScan *scan);
/*
* create_plan
* Creates the access plan for a query by recursively processing the
* desired tree of pathnodes, starting at the node 'best_path'. For
* every pathnode found, we create a corresponding plan node containing
* appropriate id, target list, and qualification information.
*
* The tlists and quals in the plan tree are still in planner format,
@@ -3310,20 +3316,22 @@ create_foreignscan_plan(PlannerInfo *root, ForeignPath *best_path,
if (bms_is_member(i - FirstLowInvalidHeapAttributeNumber, attrs_used))
{
scan_plan->fsSystemCol = true;
break;
}
}
bms_free(attrs_used);
}
+ record_foreignscan_plan_dependencies(root, scan_plan);
+
return scan_plan;
}
/*
* create_custom_plan
*
* Transform a CustomPath into a Plan.
*/
static CustomScan *
create_customscan_plan(PlannerInfo *root, CustomPath *best_path,
@@ -6211,10 +6219,66 @@ is_projection_capable_plan(Plan *plan)
case T_ModifyTable:
case T_Append:
case T_MergeAppend:
case T_RecursiveUnion:
return false;
default:
break;
}
return true;
}
+
+/*
+ * record_foreignscan_plan_dependencies
+ * Mark a plan as dependent upon the properties of foreign server, FDW and
+ * foreign table/s required by given ForeignScan.
+ *
+ * Changes to the foreign server, foreign table/s or FDW might render the plan
+ * invalid. It's not necessary that every change (e.g. changing value of an
+ * option) renders the plan invalid. Only FDW knows which changes render a plan
+ * invalid and may fail to convey the same to the core. Doing it here seems a
+ * mid-way solution.
+ */
+static void
+record_foreignscan_plan_dependencies(PlannerInfo *root, ForeignScan *scan)
+{
+ ForeignServer *server = GetForeignServer(scan->fs_server);
+ int relid;
+
+ /* Record dependency on the foreign server. */
+ record_plan_foreign_object_dependency(root, scan->fs_server,
+ FOREIGNSERVEROID);
+
+ /* Record dependency on the foreign data wrapper. */
+ record_plan_foreign_object_dependency(root, server->fdwid,
+ FOREIGNDATAWRAPPEROID);
+
+ /* Record dependency on each of the foreign tables. */
+ relid = -1;
+ while ((relid = bms_next_member(scan->fs_relids, relid)) >= 0)
+ {
+ RangeTblEntry *rte = planner_rt_fetch(relid, root);
+
+ Assert(rte->rtekind == RTE_RELATION &&
+ rte->relkind == RELKIND_FOREIGN_TABLE);
+ record_plan_foreign_object_dependency(root, rte->relid,
+ FOREIGNTABLEREL);
+ }
+
+}
+
+/*
+ * record_plan_foreign_object_dependency
+ * Records the plan's dependency on given foreign object. A helper
+ * function for record_foreignscan_plan_dependencies().
+ */
+static void
+record_plan_foreign_object_dependency(PlannerInfo *root, Oid oid, int cacheId)
+{
+ PlanInvalItem *inval_item = makeNode(PlanInvalItem);
+
+ inval_item->cacheId = cacheId;
+ inval_item->hashValue = GetSysCacheHashValue1(cacheId,
+ ObjectIdGetDatum(oid));
+
+ root->glob->invalItems = lappend(root->glob->invalItems, inval_item);
+}
diff --git a/src/backend/utils/cache/plancache.c b/src/backend/utils/cache/plancache.c
index c96a865..1d3b6be 100644
--- a/src/backend/utils/cache/plancache.c
+++ b/src/backend/utils/cache/plancache.c
@@ -112,20 +112,24 @@ static void PlanCacheSysCallback(Datum arg, int cacheid, uint32 hashvalue);
* All we need to do is hook into inval.c's callback lists.
*/
void
InitPlanCache(void)
{
CacheRegisterRelcacheCallback(PlanCacheRelCallback, (Datum) 0);
CacheRegisterSyscacheCallback(PROCOID, PlanCacheFuncCallback, (Datum) 0);
CacheRegisterSyscacheCallback(NAMESPACEOID, PlanCacheSysCallback, (Datum) 0);
CacheRegisterSyscacheCallback(OPEROID, PlanCacheSysCallback, (Datum) 0);
CacheRegisterSyscacheCallback(AMOPOPID, PlanCacheSysCallback, (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);
}
/*
* CreateCachedPlan: initially create a plan cache entry.
*
* Creation of a cached plan is divided into two steps, CreateCachedPlan and
* CompleteCachedPlan. CreateCachedPlan should be called after running the
* query through raw_parser, but before doing parse analysis and rewrite;
* CompleteCachedPlan is called after that. The reason for this arrangement
* is that it can save one round of copying of the raw parse tree, since
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
