Skip to content

Commit

Permalink
Fix planner's row-mark code for inheritance from a foreign table.
Browse files Browse the repository at this point in the history
Commit 428b260 broke planning of cases where row marks are needed
(SELECT FOR UPDATE, etc) and one of the query's tables is a foreign
table that has regular table(s) as inheritance children.  We got the
reverse case right, but apparently were thinking that foreign tables
couldn't be inheritance parents.  Not so; so we need to be able to
add a CTID junk column while adding a new child, not only a wholerow
junk column.

Back-patch to v12 where the faulty code came in.

Amit Langote

Discussion: https://postgr.es/m/CA+HiwqEmo3FV1LAQ4TVyS2h1WM=kMkZUmbNuZSCnfHvMcUcPeA@mail.gmail.com
  • Loading branch information
tglsfdc committed Jun 2, 2021
1 parent 79c50ca commit 8895923
Show file tree
Hide file tree
Showing 3 changed files with 126 additions and 2 deletions.
86 changes: 86 additions & 0 deletions contrib/postgres_fdw/expected/postgres_fdw.out
Expand Up @@ -7255,6 +7255,92 @@ select * from bar where f1 in (select f1 from foo) for share;
4 | 44
(4 rows)

-- Now check SELECT FOR UPDATE/SHARE with an inherited source table,
-- where the parent is itself a foreign table
create table loct4 (f1 int, f2 int, f3 int);
create foreign table foo2child (f3 int) inherits (foo2)
server loopback options (table_name 'loct4');
NOTICE: moving and merging column "f3" with inherited definition
DETAIL: User-specified column moved to the position of the inherited column.
explain (verbose, costs off)
select * from bar where f1 in (select f1 from foo2) for share;
QUERY PLAN
--------------------------------------------------------------------------------------
LockRows
Output: bar.f1, bar.f2, bar.ctid, foo2.*, bar.*, bar.tableoid, foo2.tableoid
-> Hash Join
Output: bar.f1, bar.f2, bar.ctid, foo2.*, bar.*, bar.tableoid, foo2.tableoid
Inner Unique: true
Hash Cond: (bar.f1 = foo2.f1)
-> Append
-> Seq Scan on public.bar bar_1
Output: bar_1.f1, bar_1.f2, bar_1.ctid, bar_1.*, bar_1.tableoid
-> Foreign Scan on public.bar2 bar_2
Output: bar_2.f1, bar_2.f2, bar_2.ctid, bar_2.*, bar_2.tableoid
Remote SQL: SELECT f1, f2, f3, ctid FROM public.loct2 FOR SHARE
-> Hash
Output: foo2.*, foo2.f1, foo2.tableoid
-> HashAggregate
Output: foo2.*, foo2.f1, foo2.tableoid
Group Key: foo2.f1
-> Append
-> Foreign Scan on public.foo2 foo2_1
Output: foo2_1.*, foo2_1.f1, foo2_1.tableoid
Remote SQL: SELECT f1, f2, f3 FROM public.loct1
-> Foreign Scan on public.foo2child foo2_2
Output: foo2_2.*, foo2_2.f1, foo2_2.tableoid
Remote SQL: SELECT f1, f2, f3 FROM public.loct4
(24 rows)

select * from bar where f1 in (select f1 from foo2) for share;
f1 | f2
----+----
2 | 22
4 | 44
(2 rows)

drop foreign table foo2child;
-- And with a local child relation of the foreign table parent
create table foo2child (f3 int) inherits (foo2);
NOTICE: moving and merging column "f3" with inherited definition
DETAIL: User-specified column moved to the position of the inherited column.
explain (verbose, costs off)
select * from bar where f1 in (select f1 from foo2) for share;
QUERY PLAN
-------------------------------------------------------------------------------------------------
LockRows
Output: bar.f1, bar.f2, bar.ctid, foo2.*, bar.*, bar.tableoid, foo2.ctid, foo2.tableoid
-> Hash Join
Output: bar.f1, bar.f2, bar.ctid, foo2.*, bar.*, bar.tableoid, foo2.ctid, foo2.tableoid
Inner Unique: true
Hash Cond: (bar.f1 = foo2.f1)
-> Append
-> Seq Scan on public.bar bar_1
Output: bar_1.f1, bar_1.f2, bar_1.ctid, bar_1.*, bar_1.tableoid
-> Foreign Scan on public.bar2 bar_2
Output: bar_2.f1, bar_2.f2, bar_2.ctid, bar_2.*, bar_2.tableoid
Remote SQL: SELECT f1, f2, f3, ctid FROM public.loct2 FOR SHARE
-> Hash
Output: foo2.*, foo2.f1, foo2.ctid, foo2.tableoid
-> HashAggregate
Output: foo2.*, foo2.f1, foo2.ctid, foo2.tableoid
Group Key: foo2.f1
-> Append
-> Foreign Scan on public.foo2 foo2_1
Output: foo2_1.*, foo2_1.f1, foo2_1.ctid, foo2_1.tableoid
Remote SQL: SELECT f1, f2, f3, ctid FROM public.loct1
-> Seq Scan on public.foo2child foo2_2
Output: foo2_2.*, foo2_2.f1, foo2_2.ctid, foo2_2.tableoid
(23 rows)

select * from bar where f1 in (select f1 from foo2) for share;
f1 | f2
----+----
2 | 22
4 | 44
(2 rows)

drop table foo2child;
-- 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);
Expand Down
21 changes: 21 additions & 0 deletions contrib/postgres_fdw/sql/postgres_fdw.sql
Expand Up @@ -1891,6 +1891,27 @@ explain (verbose, costs off)
select * from bar where f1 in (select f1 from foo) for share;
select * from bar where f1 in (select f1 from foo) for share;

-- Now check SELECT FOR UPDATE/SHARE with an inherited source table,
-- where the parent is itself a foreign table
create table loct4 (f1 int, f2 int, f3 int);
create foreign table foo2child (f3 int) inherits (foo2)
server loopback options (table_name 'loct4');

explain (verbose, costs off)
select * from bar where f1 in (select f1 from foo2) for share;
select * from bar where f1 in (select f1 from foo2) for share;

drop foreign table foo2child;

-- And with a local child relation of the foreign table parent
create table foo2child (f3 int) inherits (foo2);

explain (verbose, costs off)
select * from bar where f1 in (select f1 from foo2) for share;
select * from bar where f1 in (select f1 from foo2) for share;

drop table foo2child;

-- 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);
Expand Down
21 changes: 19 additions & 2 deletions src/backend/optimizer/util/inherit.c
Expand Up @@ -232,8 +232,25 @@ expand_inherited_rtentry(PlannerInfo *root, RelOptInfo *rel,
char resname[32];
List *newvars = NIL;

/* The old PlanRowMark should already have necessitated adding TID */
Assert(old_allMarkTypes & ~(1 << ROW_MARK_COPY));
/* Add TID junk Var if needed, unless we had it already */
if (new_allMarkTypes & ~(1 << ROW_MARK_COPY) &&
!(old_allMarkTypes & ~(1 << ROW_MARK_COPY)))
{
/* Need to fetch TID */
var = makeVar(oldrc->rti,
SelfItemPointerAttributeNumber,
TIDOID,
-1,
InvalidOid,
0);
snprintf(resname, sizeof(resname), "ctid%u", oldrc->rowmarkId);
tle = makeTargetEntry((Expr *) var,
list_length(root->processed_tlist) + 1,
pstrdup(resname),
true);
root->processed_tlist = lappend(root->processed_tlist, tle);
newvars = lappend(newvars, var);
}

/* Add whole-row junk Var if needed, unless we had it already */
if ((new_allMarkTypes & (1 << ROW_MARK_COPY)) &&
Expand Down

0 comments on commit 8895923

Please sign in to comment.