On 2016/11/22 15:24, Etsuro Fujita wrote:
On 2016/11/22 4:49, Tom Lane wrote:
Etsuro Fujita <fujita.ets...@lab.ntt.co.jp> writes:
On 2016/11/10 5:17, Tom Lane wrote:
I think there's a very good argument that we should just treat any
inval
in pg_foreign_data_wrapper as a reason to blow away the whole plan
cache.
People aren't going to alter FDW-level options often enough to make it
worth tracking things more finely. Personally I'd put
pg_foreign_server
changes in the same boat, though maybe those are changed slightly more
often. If it's worth doing anything more than that, it would be to
note
which plans mention foreign tables at all, and not invalidate those
that
don't. We could do that with, say, a hasForeignTables bool in
PlannedStmt.
I agree on that point.
OK, please update the patch to handle those catalogs that way.
Will do.
Done. I modified the patch so that any inval in pg_foreign_server also
blows the whole plan cache.
That leaves the question of whether it's worth detecting table-level
option changes this way, or whether we should just handle those by
forcing
a relcache inval in ATExecGenericOptions, as in Amit's original
patch in
<5702298d.4090...@lab.ntt.co.jp>. I kind of like that approach; that
patch was short and sweet, and it put the cost on the unusual path
(ALTER
TABLE) not the common path (every time we create a query plan).
I think it'd be better to detect table-level option changes because that
could allow us to do more;
Why not? But the bigger picture here is that relcache inval is the
established practice for forcing replanning due to table-level changes,
and I have very little sympathy for inventing a new mechanism for that
just for foreign tables. If it were worth bypassing replanning for
changes in tables in dead subqueries, for instance, it would surely be
worth making that happen for all table types not just foreign tables.
My point here is that FDW-option changes wouldn't affect replanning by
core; even if forcing a replan, we would have the same foreign tables
excluded by constraint exclusion, for example. So, considering updates
on pg_foreign_table to be rather frequent, it might be better to avoid
replanning for the pg_foreign_table changes to foreign tables excluded
by constraint exclusion, for example. My concern about the
relcache-inval approach is: that approach wouldn't allow us to do that,
IIUC.
I thought updates on pg_foreign_table would be rather frequent, but we
don't prove that that is enough that more-detailed tracking is worth the
effort. Yes, as you mentioned, it wouldn't be a good idea to invent the
new mechanism just for foreign tables. So, +1 for relcache inval.
Attached is an updated version of the patch, in which I also modified
regression tests; re-created tests to check to see if execution as well
as explain work properly and added some tests for altering server-level
options.
Sorry for the delay.
Best regards,
Etsuro Fujita
*** a/contrib/postgres_fdw/expected/postgres_fdw.out
--- b/contrib/postgres_fdw/expected/postgres_fdw.out
***************
*** 3575,3586 **** EXECUTE st5('foo', 1);
--- 3575,3754 ----
1 | 1 | 00001 | Fri Jan 02 00:00:00 1970 PST | Fri Jan 02 00:00:00 1970 | 1 | 1 | foo
(1 row)
+ -- altering FDW options requires replanning
+ PREPARE st6 AS SELECT * FROM ft1 t1 WHERE t1.c1 = t1.c2;
+ EXPLAIN (VERBOSE, COSTS OFF) EXECUTE st6;
+ QUERY PLAN
+ ----------------------------------------------------------------------------------------------
+ Foreign Scan on public.ft1 t1
+ Output: c1, c2, c3, c4, c5, c6, c7, c8
+ Remote SQL: SELECT "C 1", c2, c3, c4, c5, c6, c7, c8 FROM "S 1"."T 1" WHERE (("C 1" = c2))
+ (3 rows)
+
+ PREPARE st7(int,int,text) AS INSERT INTO ft1 (c1,c2,c3) VALUES ($1,$2,$3);
+ EXPLAIN (VERBOSE, COSTS OFF) EXECUTE st7(1001,101,'foo');
+ QUERY PLAN
+ -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ Insert on public.ft1
+ Remote SQL: INSERT INTO "S 1"."T 1"("C 1", c2, c3, c4, c5, c6, c7, c8) VALUES ($1, $2, $3, $4, $5, $6, $7, $8)
+ -> Result
+ Output: NULL::integer, 1001, 101, 'foo'::text, NULL::timestamp with time zone, NULL::timestamp without time zone, NULL::character varying, 'ft1 '::character(10), NULL::user_enum
+ (4 rows)
+
+ PREPARE st8(int) AS UPDATE ft1 SET c3 = 'bar' WHERE c1 = $1 RETURNING *;
+ EXPLAIN (VERBOSE, COSTS OFF) EXECUTE st8(1001);
+ QUERY PLAN
+ --------------------------------------------------------------------------------------------------------------------------------
+ Update on public.ft1
+ Output: c1, c2, c3, c4, c5, c6, c7, c8
+ -> Foreign Update on public.ft1
+ Remote SQL: UPDATE "S 1"."T 1" SET c3 = 'bar'::text WHERE (("C 1" = 1001)) RETURNING "C 1", c2, c3, c4, c5, c6, c7, c8
+ (4 rows)
+
+ PREPARE st9(int) AS DELETE FROM ft1 WHERE c1 = $1 RETURNING *;
+ EXPLAIN (VERBOSE, COSTS OFF) EXECUTE st9(1001);
+ QUERY PLAN
+ ----------------------------------------------------------------------------------------------------------------
+ Delete on public.ft1
+ Output: c1, c2, c3, c4, c5, c6, c7, c8
+ -> Foreign Delete on public.ft1
+ Remote SQL: DELETE FROM "S 1"."T 1" WHERE (("C 1" = 1001)) RETURNING "C 1", c2, c3, c4, c5, c6, c7, c8
+ (4 rows)
+
+ ALTER TABLE "S 1"."T 1" RENAME TO "T 0";
+ ALTER FOREIGN TABLE ft1 OPTIONS (SET table_name 'T 0');
+ EXPLAIN (VERBOSE, COSTS OFF) EXECUTE st6;
+ QUERY PLAN
+ ----------------------------------------------------------------------------------------------
+ Foreign Scan on public.ft1 t1
+ Output: c1, c2, c3, c4, c5, c6, c7, c8
+ Remote SQL: SELECT "C 1", c2, c3, c4, c5, c6, c7, c8 FROM "S 1"."T 0" WHERE (("C 1" = c2))
+ (3 rows)
+
+ EXECUTE st6;
+ 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
+ 2 | 2 | 00002 | Sat Jan 03 00:00:00 1970 PST | Sat Jan 03 00:00:00 1970 | 2 | 2 | foo
+ 3 | 3 | 00003 | Sun Jan 04 00:00:00 1970 PST | Sun Jan 04 00:00:00 1970 | 3 | 3 | foo
+ 4 | 4 | 00004 | Mon Jan 05 00:00:00 1970 PST | Mon Jan 05 00:00:00 1970 | 4 | 4 | foo
+ 5 | 5 | 00005 | Tue Jan 06 00:00:00 1970 PST | Tue Jan 06 00:00:00 1970 | 5 | 5 | foo
+ 6 | 6 | 00006 | Wed Jan 07 00:00:00 1970 PST | Wed Jan 07 00:00:00 1970 | 6 | 6 | foo
+ 7 | 7 | 00007 | Thu Jan 08 00:00:00 1970 PST | Thu Jan 08 00:00:00 1970 | 7 | 7 | foo
+ 8 | 8 | 00008 | Fri Jan 09 00:00:00 1970 PST | Fri Jan 09 00:00:00 1970 | 8 | 8 | foo
+ 9 | 9 | 00009 | Sat Jan 10 00:00:00 1970 PST | Sat Jan 10 00:00:00 1970 | 9 | 9 | foo
+ (9 rows)
+
+ EXPLAIN (VERBOSE, COSTS OFF) EXECUTE st7(1001,101,'foo');
+ QUERY PLAN
+ -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ Insert on public.ft1
+ Remote SQL: INSERT INTO "S 1"."T 0"("C 1", c2, c3, c4, c5, c6, c7, c8) VALUES ($1, $2, $3, $4, $5, $6, $7, $8)
+ -> Result
+ Output: NULL::integer, 1001, 101, 'foo'::text, NULL::timestamp with time zone, NULL::timestamp without time zone, NULL::character varying, 'ft1 '::character(10), NULL::user_enum
+ (4 rows)
+
+ EXECUTE st7(1001,101,'foo');
+ EXPLAIN (VERBOSE, COSTS OFF) EXECUTE st8(1001);
+ QUERY PLAN
+ --------------------------------------------------------------------------------------------------------------------------------
+ Update on public.ft1
+ Output: c1, c2, c3, c4, c5, c6, c7, c8
+ -> Foreign Update on public.ft1
+ Remote SQL: UPDATE "S 1"."T 0" SET c3 = 'bar'::text WHERE (("C 1" = 1001)) RETURNING "C 1", c2, c3, c4, c5, c6, c7, c8
+ (4 rows)
+
+ EXECUTE st8(1001);
+ c1 | c2 | c3 | c4 | c5 | c6 | c7 | c8
+ ------+-----+-----+----+----+----+------------+----
+ 1001 | 101 | bar | | | | ft1 |
+ (1 row)
+
+ EXPLAIN (VERBOSE, COSTS OFF) EXECUTE st9(1001);
+ QUERY PLAN
+ ----------------------------------------------------------------------------------------------------------------
+ Delete on public.ft1
+ Output: c1, c2, c3, c4, c5, c6, c7, c8
+ -> Foreign Delete on public.ft1
+ Remote SQL: DELETE FROM "S 1"."T 0" WHERE (("C 1" = 1001)) RETURNING "C 1", c2, c3, c4, c5, c6, c7, c8
+ (4 rows)
+
+ EXECUTE st9(1001);
+ c1 | c2 | c3 | c4 | c5 | c6 | c7 | c8
+ ------+-----+-----+----+----+----+------------+----
+ 1001 | 101 | bar | | | | ft1 |
+ (1 row)
+
+ ALTER TABLE "S 1"."T 0" RENAME TO "T 1";
+ ALTER FOREIGN TABLE ft1 OPTIONS (SET table_name 'T 1');
+ PREPARE st10 AS SELECT count(c3) FROM ft1 t1 WHERE t1.c1 = postgres_fdw_abs(t1.c2);
+ EXPLAIN (VERBOSE, COSTS OFF) EXECUTE st10;
+ QUERY PLAN
+ -----------------------------------------------------------------------------------------------
+ Foreign Scan
+ Output: (count(c3))
+ Relations: Aggregate on (public.ft1 t1)
+ Remote SQL: SELECT count(c3) FROM "S 1"."T 1" WHERE (("C 1" = public.postgres_fdw_abs(c2)))
+ (4 rows)
+
+ PREPARE st11 AS SELECT count(c3) FROM ft1 t1 WHERE t1.c1 === t1.c2;
+ EXPLAIN (VERBOSE, COSTS OFF) EXECUTE st11;
+ QUERY PLAN
+ -----------------------------------------------------------------------------------------
+ Foreign Scan
+ Output: (count(c3))
+ Relations: Aggregate on (public.ft1 t1)
+ Remote SQL: SELECT count(c3) FROM "S 1"."T 1" WHERE (("C 1" OPERATOR(public.===) c2))
+ (4 rows)
+
+ ALTER SERVER loopback OPTIONS (DROP extensions);
+ EXPLAIN (VERBOSE, COSTS OFF) EXECUTE st10;
+ QUERY PLAN
+ -----------------------------------------------------------
+ Aggregate
+ Output: count(c3)
+ -> Foreign Scan on public.ft1 t1
+ Output: c3
+ Filter: (t1.c1 = postgres_fdw_abs(t1.c2))
+ Remote SQL: SELECT "C 1", c2, c3 FROM "S 1"."T 1"
+ (6 rows)
+
+ EXECUTE st10;
+ count
+ -------
+ 9
+ (1 row)
+
+ EXPLAIN (VERBOSE, COSTS OFF) EXECUTE st11;
+ QUERY PLAN
+ -----------------------------------------------------------
+ Aggregate
+ Output: count(c3)
+ -> Foreign Scan on public.ft1 t1
+ Output: c3
+ Filter: (t1.c1 === t1.c2)
+ Remote SQL: SELECT "C 1", c2, c3 FROM "S 1"."T 1"
+ (6 rows)
+
+ EXECUTE st11;
+ count
+ -------
+ 9
+ (1 row)
+
+ ALTER SERVER loopback OPTIONS (ADD extensions 'postgres_fdw');
-- cleanup
DEALLOCATE st1;
DEALLOCATE st2;
DEALLOCATE st3;
DEALLOCATE st4;
DEALLOCATE st5;
+ DEALLOCATE st6;
+ DEALLOCATE st7;
+ DEALLOCATE st8;
+ DEALLOCATE st9;
+ DEALLOCATE st10;
+ DEALLOCATE st11;
-- 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;
*** a/contrib/postgres_fdw/sql/postgres_fdw.sql
--- b/contrib/postgres_fdw/sql/postgres_fdw.sql
***************
*** 881,892 **** EXPLAIN (VERBOSE, COSTS OFF) EXECUTE st5('foo', 1);
--- 881,930 ----
EXPLAIN (VERBOSE, COSTS OFF) EXECUTE st5('foo', 1);
EXECUTE st5('foo', 1);
+ -- altering FDW options requires replanning
+ PREPARE st6 AS SELECT * FROM ft1 t1 WHERE t1.c1 = t1.c2;
+ EXPLAIN (VERBOSE, COSTS OFF) EXECUTE st6;
+ PREPARE st7(int,int,text) AS INSERT INTO ft1 (c1,c2,c3) VALUES ($1,$2,$3);
+ EXPLAIN (VERBOSE, COSTS OFF) EXECUTE st7(1001,101,'foo');
+ PREPARE st8(int) AS UPDATE ft1 SET c3 = 'bar' WHERE c1 = $1 RETURNING *;
+ EXPLAIN (VERBOSE, COSTS OFF) EXECUTE st8(1001);
+ PREPARE st9(int) AS DELETE FROM ft1 WHERE c1 = $1 RETURNING *;
+ EXPLAIN (VERBOSE, COSTS OFF) EXECUTE st9(1001);
+ ALTER TABLE "S 1"."T 1" RENAME TO "T 0";
+ ALTER FOREIGN TABLE ft1 OPTIONS (SET table_name 'T 0');
+ EXPLAIN (VERBOSE, COSTS OFF) EXECUTE st6;
+ EXECUTE st6;
+ EXPLAIN (VERBOSE, COSTS OFF) EXECUTE st7(1001,101,'foo');
+ EXECUTE st7(1001,101,'foo');
+ EXPLAIN (VERBOSE, COSTS OFF) EXECUTE st8(1001);
+ EXECUTE st8(1001);
+ EXPLAIN (VERBOSE, COSTS OFF) EXECUTE st9(1001);
+ EXECUTE st9(1001);
+ ALTER TABLE "S 1"."T 0" RENAME TO "T 1";
+ ALTER FOREIGN TABLE ft1 OPTIONS (SET table_name 'T 1');
+ PREPARE st10 AS SELECT count(c3) FROM ft1 t1 WHERE t1.c1 = postgres_fdw_abs(t1.c2);
+ EXPLAIN (VERBOSE, COSTS OFF) EXECUTE st10;
+ PREPARE st11 AS SELECT count(c3) FROM ft1 t1 WHERE t1.c1 === t1.c2;
+ EXPLAIN (VERBOSE, COSTS OFF) EXECUTE st11;
+ ALTER SERVER loopback OPTIONS (DROP extensions);
+ EXPLAIN (VERBOSE, COSTS OFF) EXECUTE st10;
+ EXECUTE st10;
+ EXPLAIN (VERBOSE, COSTS OFF) EXECUTE st11;
+ EXECUTE st11;
+ ALTER SERVER loopback OPTIONS (ADD extensions 'postgres_fdw');
+
-- cleanup
DEALLOCATE st1;
DEALLOCATE st2;
DEALLOCATE st3;
DEALLOCATE st4;
DEALLOCATE st5;
+ DEALLOCATE st6;
+ DEALLOCATE st7;
+ DEALLOCATE st8;
+ DEALLOCATE st9;
+ DEALLOCATE st10;
+ DEALLOCATE st11;
-- System columns, except ctid and oid, should not be sent to remote
EXPLAIN (VERBOSE, COSTS OFF)
*** a/src/backend/commands/tablecmds.c
--- b/src/backend/commands/tablecmds.c
***************
*** 11266,11271 **** ATExecGenericOptions(Relation rel, List *options)
--- 11266,11277 ----
simple_heap_update(ftrel, &tuple->t_self, tuple);
CatalogUpdateIndexes(ftrel, tuple);
+ /*
+ * Invalidate relcache so that after this commit all sessions will
+ * refresh any cached plans that might reference the old options
+ */
+ CacheInvalidateRelcache(rel);
+
InvokeObjectPostAlterHook(ForeignTableRelationId,
RelationGetRelid(rel), 0);
*** a/src/backend/utils/cache/plancache.c
--- b/src/backend/utils/cache/plancache.c
***************
*** 118,123 **** InitPlanCache(void)
--- 118,125 ----
CacheRegisterSyscacheCallback(NAMESPACEOID, PlanCacheSysCallback, (Datum) 0);
CacheRegisterSyscacheCallback(OPEROID, PlanCacheSysCallback, (Datum) 0);
CacheRegisterSyscacheCallback(AMOPOPID, PlanCacheSysCallback, (Datum) 0);
+ CacheRegisterSyscacheCallback(FOREIGNSERVEROID, PlanCacheSysCallback, (Datum) 0);
+ CacheRegisterSyscacheCallback(FOREIGNDATAWRAPPEROID, PlanCacheSysCallback, (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
