Re: Join push-down support for foreign tables

> Here is v4 patch of Join push-down support for foreign tables. This
> patch requires Custom/Foreign join patch v7 posted by Kaigai-san.
>
Thanks for your efforts,

> In this version I added check about query type which gives up pushing
> down joins when the join is a part of an underlying query of
> UPDATE/DELETE.
>
> As of now postgres_fdw builds a proper remote query but it can't bring
> ctid value up to postgresExecForeignUpdate()...
>
The "ctid" reference shall exist as an usual column reference in
the target-list of join-rel. It is not origin of the problem.

See my investigation below. I guess special treatment on whole-row-
reference is problematic, rather than ctid.

> How to reproduce the error, please execute query below after running
> attached init_fdw.sql for building test environment. Note that the
> script drops "user1", and creates database "fdw" and "pgbench".
>
> fdw=# explain (verbose) update pgbench_branches b set filler = 'foo'
> from pgbench_tellers t where t.bid = b.bid and t.tid < 10;
>
> QUERY
> PLAN
>
> ----------------------------------------------------------------------------
> ----------------------------------------------------------------------------
> --------------------
> ----------------------------------------------------------------------------
> -----------------------------------
> Update on public.pgbench_branches b (cost=100.00..100.67 rows=67 width=390)
> Remote SQL: UPDATE public.pgbench_branches SET filler = $2 WHERE ctid = $1
> -> Foreign Scan (cost=100.00..100.67 rows=67 width=390)
> Output: b.bid, b.bbalance, 'foo
>
> '::character(88), b.ctid, *
> Remote SQL: SELECT r.a_0, r.a_1, r.a_2, l FROM (SELECT tid,
> bid, tbalance, filler FROM public.pgbench_tellers WHERE ((tid < 10)))
> l (a_0, a_1) INNER JOIN (SELECT b
> id, bbalance, NULL, ctid FROM public.pgbench_branches FOR UPDATE) r
> (a_0, a_1, a_2, a_3) ON ((r.a_0 = l.a_1))
> (5 rows)
> fdw=# explain (analyze, verbose) update pgbench_branches b set filler
> = 'foo' from pgbench_tellers t where t.bid = b.bid and t.tid < 10;
> ERROR: ctid is NULL
>
It seems to me the left relation has smaller number of alias definitions
than required. The left SELECT statement has 4 target-entries, however,
only a_0 and a_1 are defined.
The logic to assign aliases of relation/column might be problematic.
Because deparseColumnAliases() add aliases for each target-entries of
underlying SELECT statement, but skips whole-row0-reference.
On the other hands, postgres_fdw takes special treatment on whole-
row-reference. Once a whole-row-reference is used, postgres_fdw
put all the non-system columns on the target-list of remote SELECT
statement.
Thus, it makes mismatch between baserel->targetlist and generated
aliases.

I think we have two options:
(1) Stop special treatment for whole-row-reference, even if it is
simple foreign-scan (which does not involve join).
(2) Add a new routine to reconstruct whole-row-reference of
a particular relation from the target-entries of joined relations.

My preference is (2). It can keep the code simple, and I doubt
whether network traffic optimization actually has advantage towards
disadvantage of local tuple reconstruction (which consumes additional
CPU cycles).

Does it make sense for you?
--
NEC OSS Promotion Center / PG-Strom Project
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>