I noticed that 428b260f87e8 (v12) broke the cases where a parent
foreign table has row marks assigned. Specifically, the following
Assert added to expand_inherited_rtentry() by that commit looks bogus
in this regard:
/* The old PlanRowMark should already have necessitated adding TID */
Assert(old_allMarkTypes & ~(1 << ROW_MARK_COPY));
The Assert appears to have been written based on the assumption that
the root parent would always be a local heap relation, but given that
we allow foreign tables also to be inheritance parents, that
assumption is false.
Problem cases:
create extension postgres_fdw ;
create server loopback foreign data wrapper postgres_fdw;
create user mapping for current_user server loopback ;
create table loct1 (a int);
create foreign table ft_parent (a int) server loopback options
(table_name 'loct1');
create table loct2 (a int);
create foreign table ft_child () inherits (ft_parent) server loopback
options (table_name 'loct2');
explain (verbose) select * from ft_parent FOR UPDATE;
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
The connection to the server was lost. Attempting reset: Failed.
!?> \q
Just commenting out that Assert will let the above work, but that's
not enough, because any child tables that are local can't access ctidN
junk columns that should have been added before reaching
expand_inherited_rtentry(), but wouldn't because the parent is a
foreign table.
create table loct3 () inherits (ft_parent);
explain (verbose) select * from ft_parent FOR UPDATE;
ERROR: could not find junk ctid1 column
The right thing would have been to have the same code as in
preprocess_targetlist() to add a TID row marking junk column if
needed. Attached a patch for that, which also adds the test cases.
Actually, I had to make a separate version of the patch to apply to
the v12 branch, because EXPLAIN outputs relation aliases a bit
differently starting in v13, which is attached too.
--
Amit Langote
EDB: http://www.enterprisedb.com
