Amit Langote писал 2021-06-01 15:47:
Perhaps, we can get away with adding the wholerow Var with the target
relation's reltype when the target foreign table is not a "child"
relation, but the root target relation itself. Maybe like the
attached?
Hi.
I think the patch fixes this issue, but it still preserves chances to
get RECORD in fetch_more_data()
(at least with combination with asymmetric partition-wise join).
What about the following patch?
--
Best regards,
Alexander Pyhalov,
Postgres Professional
From 806a9b4fbf376e8fa75669229c8401ea76dd1cb2 Mon Sep 17 00:00:00 2001
From: Alexander Pyhalov <a.pyha...@postgrespro.ru>
Date: Tue, 1 Jun 2021 12:00:08 +0300
Subject: [PATCH] [PGPRO-4769] Fix foreign update
Tags: shardman
---
.../postgres_fdw/expected/postgres_fdw.out | 66 +++++++++----------
src/backend/optimizer/util/appendinfo.c | 53 ++++++++++++---
src/include/optimizer/appendinfo.h | 2 +
3 files changed, 78 insertions(+), 43 deletions(-)
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index 7df30010f25..d424a1a5365 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -7258,19 +7258,19 @@ select * from bar where f1 in (select f1 from foo) for share;
-- Check UPDATE with inherited target and an inherited source table
explain (verbose, costs off)
update bar set f2 = f2 + 100 where f1 in (select f1 from foo);
- QUERY PLAN
--------------------------------------------------------------------------------------------------------
+ QUERY PLAN
+-----------------------------------------------------------------------------------------------------
Update on public.bar
Update on public.bar bar_1
Foreign Update on public.bar2 bar_2
Remote SQL: UPDATE public.loct2 SET f2 = $2 WHERE ctid = $1
-> Hash Join
- Output: (bar.f2 + 100), foo.ctid, bar.tableoid, bar.ctid, (NULL::record), foo.*, foo.tableoid
+ Output: (bar.f2 + 100), foo.ctid, bar.tableoid, bar.ctid, (NULL::bar2), foo.*, foo.tableoid
Inner Unique: true
Hash Cond: (bar.f1 = foo.f1)
-> Append
-> Seq Scan on public.bar bar_1
- Output: bar_1.f2, bar_1.f1, bar_1.tableoid, bar_1.ctid, NULL::record
+ Output: bar_1.f2, bar_1.f1, bar_1.tableoid, bar_1.ctid, NULL::bar2
-> Foreign Scan on public.bar2 bar_2
Output: bar_2.f2, bar_2.f1, bar_2.tableoid, bar_2.ctid, bar_2.*
Remote SQL: SELECT f1, f2, f3, ctid FROM public.loct2 FOR UPDATE
@@ -7305,21 +7305,21 @@ update bar set f2 = f2 + 100
from
( select f1 from foo union all select f1+3 from foo ) ss
where bar.f1 = ss.f1;
- QUERY PLAN
-------------------------------------------------------------------------------------------------
+ QUERY PLAN
+----------------------------------------------------------------------------------------------
Update on public.bar
Update on public.bar bar_1
Foreign Update on public.bar2 bar_2
Remote SQL: UPDATE public.loct2 SET f2 = $2 WHERE ctid = $1
-> Merge Join
- Output: (bar.f2 + 100), (ROW(foo.f1)), bar.tableoid, bar.ctid, (NULL::record)
+ Output: (bar.f2 + 100), (ROW(foo.f1)), bar.tableoid, bar.ctid, (NULL::bar2)
Merge Cond: (bar.f1 = foo.f1)
-> Sort
- Output: bar.f2, bar.f1, bar.tableoid, bar.ctid, (NULL::record)
+ Output: bar.f2, bar.f1, bar.tableoid, bar.ctid, (NULL::bar2)
Sort Key: bar.f1
-> Append
-> Seq Scan on public.bar bar_1
- Output: bar_1.f2, bar_1.f1, bar_1.tableoid, bar_1.ctid, NULL::record
+ Output: bar_1.f2, bar_1.f1, bar_1.tableoid, bar_1.ctid, NULL::bar2
-> Foreign Scan on public.bar2 bar_2
Output: bar_2.f2, bar_2.f1, bar_2.tableoid, bar_2.ctid, bar_2.*
Remote SQL: SELECT f1, f2, f3, ctid FROM public.loct2 FOR UPDATE
@@ -7496,10 +7496,10 @@ update bar set f2 = f2 + 100 returning *;
Update on public.bar bar_1
Foreign Update on public.bar2 bar_2
-> Result
- Output: (bar.f2 + 100), bar.tableoid, bar.ctid, (NULL::record)
+ Output: (bar.f2 + 100), bar.tableoid, bar.ctid, (NULL::bar2)
-> Append
-> Seq Scan on public.bar bar_1
- Output: bar_1.f2, bar_1.tableoid, bar_1.ctid, NULL::record
+ Output: bar_1.f2, bar_1.tableoid, bar_1.ctid, NULL::bar2
-> Foreign Update on public.bar2 bar_2
Remote SQL: UPDATE public.loct2 SET f2 = (f2 + 100) RETURNING f1, f2
(11 rows)
@@ -7531,10 +7531,10 @@ update bar set f2 = f2 + 100;
Foreign Update on public.bar2 bar_2
Remote SQL: UPDATE public.loct2 SET f1 = $2, f2 = $3, f3 = $4 WHERE ctid = $1 RETURNING f1, f2, f3
-> Result
- Output: (bar.f2 + 100), bar.tableoid, bar.ctid, (NULL::record)
+ Output: (bar.f2 + 100), bar.tableoid, bar.ctid, (NULL::bar2)
-> Append
-> Seq Scan on public.bar bar_1
- Output: bar_1.f2, bar_1.tableoid, bar_1.ctid, NULL::record
+ Output: bar_1.f2, bar_1.tableoid, bar_1.ctid, NULL::bar2
-> Foreign Scan on public.bar2 bar_2
Output: bar_2.f2, bar_2.tableoid, bar_2.ctid, bar_2.*
Remote SQL: SELECT f1, f2, f3, ctid FROM public.loct2 FOR UPDATE
@@ -7563,7 +7563,7 @@ delete from bar where f2 < 400;
Remote SQL: DELETE FROM public.loct2 WHERE ctid = $1 RETURNING f1, f2, f3
-> Append
-> Seq Scan on public.bar bar_1
- Output: bar_1.tableoid, bar_1.ctid, NULL::record
+ Output: bar_1.tableoid, bar_1.ctid, NULL::bar2
Filter: (bar_1.f2 < 400)
-> Foreign Scan on public.bar2 bar_2
Output: bar_2.tableoid, bar_2.ctid, bar_2.*
@@ -7599,19 +7599,19 @@ analyze remt1;
analyze remt2;
explain (verbose, costs off)
update parent set b = parent.b || remt2.b from remt2 where parent.a = remt2.a returning *;
- QUERY PLAN
-----------------------------------------------------------------------------------------------------------------
+ QUERY PLAN
+---------------------------------------------------------------------------------------------------------------
Update on public.parent
Output: parent_1.a, parent_1.b, remt2.a, remt2.b
Update on public.parent parent_1
Foreign Update on public.remt1 parent_2
Remote SQL: UPDATE public.loct1 SET b = $2 WHERE ctid = $1 RETURNING a, b
-> Nested Loop
- Output: (parent.b || remt2.b), remt2.*, remt2.a, remt2.b, parent.tableoid, parent.ctid, (NULL::record)
+ Output: (parent.b || remt2.b), remt2.*, remt2.a, remt2.b, parent.tableoid, parent.ctid, (NULL::remt1)
Join Filter: (parent.a = remt2.a)
-> Append
-> Seq Scan on public.parent parent_1
- Output: parent_1.b, parent_1.a, parent_1.tableoid, parent_1.ctid, NULL::record
+ Output: parent_1.b, parent_1.a, parent_1.tableoid, parent_1.ctid, NULL::remt1
-> Foreign Scan on public.remt1 parent_2
Output: parent_2.b, parent_2.a, parent_2.tableoid, parent_2.ctid, parent_2.*
Remote SQL: SELECT a, b, ctid FROM public.loct1 FOR UPDATE
@@ -7871,7 +7871,7 @@ update utrtest set a = 1 where a = 1 or a = 2 returning *;
-> Foreign Update on public.remp utrtest_1
Remote SQL: UPDATE public.loct SET a = 1 WHERE (((a = 1) OR (a = 2))) RETURNING a, b
-> Seq Scan on public.locp utrtest_2
- Output: 1, utrtest_2.tableoid, utrtest_2.ctid, NULL::record
+ Output: 1, utrtest_2.tableoid, utrtest_2.ctid, NULL::utrtest
Filter: ((utrtest_2.a = 1) OR (utrtest_2.a = 2))
(10 rows)
@@ -7910,8 +7910,8 @@ insert into utrtest values (2, 'qux');
-- with a direct modification plan
explain (verbose, costs off)
update utrtest set a = 1 returning *;
- QUERY PLAN
----------------------------------------------------------------------------
+ QUERY PLAN
+----------------------------------------------------------------------------
Update on public.utrtest
Output: utrtest_1.a, utrtest_1.b
Foreign Update on public.remp utrtest_1
@@ -7920,7 +7920,7 @@ update utrtest set a = 1 returning *;
-> Foreign Update on public.remp utrtest_1
Remote SQL: UPDATE public.loct SET a = 1 RETURNING a, b
-> Seq Scan on public.locp utrtest_2
- Output: 1, utrtest_2.tableoid, utrtest_2.ctid, NULL::record
+ Output: 1, utrtest_2.tableoid, utrtest_2.ctid, NULL::utrtest
(9 rows)
update utrtest set a = 1 returning *;
@@ -7946,7 +7946,7 @@ update utrtest set a = 1 from (values (1), (2)) s(x) where a = s.x returning *;
Output: utrtest_1.a, utrtest_1.tableoid, utrtest_1.ctid, utrtest_1.*
Remote SQL: SELECT a, b, ctid FROM public.loct FOR UPDATE
-> Seq Scan on public.locp utrtest_2
- Output: utrtest_2.a, utrtest_2.tableoid, utrtest_2.ctid, NULL::record
+ Output: utrtest_2.a, utrtest_2.tableoid, utrtest_2.ctid, NULL::utrtest
-> Hash
Output: "*VALUES*".*, "*VALUES*".column1
-> Values Scan on "*VALUES*"
@@ -7970,15 +7970,15 @@ insert into utrtest values (3, 'xyzzy');
-- with a direct modification plan
explain (verbose, costs off)
update utrtest set a = 3 returning *;
- QUERY PLAN
----------------------------------------------------------------------------
+ QUERY PLAN
+----------------------------------------------------------------------------
Update on public.utrtest
Output: utrtest_1.a, utrtest_1.b
Update on public.locp utrtest_1
Foreign Update on public.remp utrtest_2
-> Append
-> Seq Scan on public.locp utrtest_1
- Output: 3, utrtest_1.tableoid, utrtest_1.ctid, NULL::record
+ Output: 3, utrtest_1.tableoid, utrtest_1.ctid, NULL::utrtest
-> Foreign Update on public.remp utrtest_2
Remote SQL: UPDATE public.loct SET a = 3 RETURNING a, b
(9 rows)
@@ -7988,19 +7988,19 @@ ERROR: cannot route tuples into foreign table to be updated "remp"
-- with a non-direct modification plan
explain (verbose, costs off)
update utrtest set a = 3 from (values (2), (3)) s(x) where a = s.x returning *;
- QUERY PLAN
------------------------------------------------------------------------------------------------------
+ QUERY PLAN
+------------------------------------------------------------------------------------------------------
Update on public.utrtest
Output: utrtest_1.a, utrtest_1.b, "*VALUES*".column1
Update on public.locp utrtest_1
Foreign Update on public.remp utrtest_2
Remote SQL: UPDATE public.loct SET a = $2 WHERE ctid = $1 RETURNING a, b
-> Hash Join
- Output: 3, "*VALUES*".*, "*VALUES*".column1, utrtest.tableoid, utrtest.ctid, (NULL::record)
+ Output: 3, "*VALUES*".*, "*VALUES*".column1, utrtest.tableoid, utrtest.ctid, (NULL::utrtest)
Hash Cond: (utrtest.a = "*VALUES*".column1)
-> Append
-> Seq Scan on public.locp utrtest_1
- Output: utrtest_1.a, utrtest_1.tableoid, utrtest_1.ctid, NULL::record
+ Output: utrtest_1.a, utrtest_1.tableoid, utrtest_1.ctid, NULL::utrtest
-> Foreign Scan on public.remp utrtest_2
Output: utrtest_2.a, utrtest_2.tableoid, utrtest_2.ctid, utrtest_2.*
Remote SQL: SELECT a, b, ctid FROM public.loct FOR UPDATE
@@ -10157,8 +10157,8 @@ RESET enable_hashjoin;
-- Test that UPDATE/DELETE with inherited target works with async_capable enabled
EXPLAIN (VERBOSE, COSTS OFF)
UPDATE async_pt SET c = c || c WHERE b = 0 RETURNING *;
- QUERY PLAN
-----------------------------------------------------------------------------------------------------------
+ QUERY PLAN
+------------------------------------------------------------------------------------------------------------
Update on public.async_pt
Output: async_pt_1.a, async_pt_1.b, async_pt_1.c
Foreign Update on public.async_p1 async_pt_1
@@ -10170,7 +10170,7 @@ UPDATE async_pt SET c = c || c WHERE b = 0 RETURNING *;
-> Foreign Update on public.async_p2 async_pt_2
Remote SQL: UPDATE public.base_tbl2 SET c = (c || c) WHERE ((b = 0)) RETURNING a, b, c
-> Seq Scan on public.async_p3 async_pt_3
- Output: (async_pt_3.c || async_pt_3.c), async_pt_3.tableoid, async_pt_3.ctid, NULL::record
+ Output: (async_pt_3.c || async_pt_3.c), async_pt_3.tableoid, async_pt_3.ctid, NULL::async_pt
Filter: (async_pt_3.b = 0)
(13 rows)
diff --git a/src/backend/optimizer/util/appendinfo.c b/src/backend/optimizer/util/appendinfo.c
index af46f581ac1..6d440af41d2 100644
--- a/src/backend/optimizer/util/appendinfo.c
+++ b/src/backend/optimizer/util/appendinfo.c
@@ -16,6 +16,7 @@
#include "access/htup_details.h"
#include "access/table.h"
+#include "catalog/partition.h"
#include "foreign/fdwapi.h"
#include "nodes/makefuncs.h"
#include "nodes/nodeFuncs.h"
@@ -752,7 +753,7 @@ find_appinfos_by_relids(PlannerInfo *root, Relids relids, int *nappinfos)
*
* The Var must be equal(), aside from varno, to any other row-identity
* column with the same rowid_name. Thus, for example, "wholerow"
- * row identities had better use vartype == RECORDOID.
+ * row identities had better use vartype of the base relation.
*
* rtindex is currently redundant with rowid_var->varno, but we specify
* it as a separate parameter in case this is ever generalized to support
@@ -846,6 +847,46 @@ add_row_identity_var(PlannerInfo *root, Var *orig_var,
root->processed_tlist = lappend(root->processed_tlist, tle);
}
+/*
+ * add_row_identity_var_wholerow
+ * Wrapper over add_row_identity_var() to register a "wholerow"
+ * row-identity column
+ *
+ * This has been made into a function unlike for other row-identity Vars,
+ * because the vartype that must be assigned to the Var differs based on
+ * whether it's being added for the root or for a child target relation.
+ * FDWs should call this for example instead of making the Var by themselves.
+ */
+void
+add_row_identity_var_wholerow(PlannerInfo *root, Index rtindex,
+ Relation target_relation)
+{
+ Oid reloid;
+ Relation r;
+ Var *var;
+
+ r = target_relation;
+ reloid = RelationGetRelid(target_relation);
+
+ while (RelationGetForm(r)->relispartition)
+ {
+ reloid = get_partition_parent(reloid, false);
+ if (r != target_relation)
+ table_close(r, AccessShareLock);
+ r = table_open(reloid, AccessShareLock);
+ }
+
+ var = makeVar(rtindex,
+ InvalidAttrNumber,
+ RelationGetForm(r)->reltype,
+ -1,
+ InvalidOid,
+ 0);
+ if (r != target_relation)
+ table_close(r, AccessShareLock);
+ add_row_identity_var(root, var, rtindex, "wholerow");
+}
+
/*
* add_row_identity_columns
*
@@ -909,15 +950,7 @@ add_row_identity_columns(PlannerInfo *root, Index rtindex,
(target_relation->trigdesc &&
(target_relation->trigdesc->trig_delete_after_row ||
target_relation->trigdesc->trig_delete_before_row)))
- {
- var = makeVar(rtindex,
- InvalidAttrNumber,
- RECORDOID,
- -1,
- InvalidOid,
- 0);
- add_row_identity_var(root, var, rtindex, "wholerow");
- }
+ add_row_identity_var_wholerow(root, rtindex, target_relation);
}
}
diff --git a/src/include/optimizer/appendinfo.h b/src/include/optimizer/appendinfo.h
index 39d04d9cc02..97442a4ee17 100644
--- a/src/include/optimizer/appendinfo.h
+++ b/src/include/optimizer/appendinfo.h
@@ -42,6 +42,8 @@ extern AppendRelInfo **find_appinfos_by_relids(PlannerInfo *root,
Relids relids, int *nappinfos);
extern void add_row_identity_var(PlannerInfo *root, Var *rowid_var,
Index rtindex, const char *rowid_name);
+extern void add_row_identity_var_wholerow(PlannerInfo *root, Index rtindex,
+ Relation target_relation);
extern void add_row_identity_columns(PlannerInfo *root, Index rtindex,
RangeTblEntry *target_rte,
Relation target_relation);
--
2.25.1
