On 2019/05/28 12:54, Amit Langote wrote:
On 2019/05/27 22:02, Tom Lane wrote:
Amit Langote <langote_amit...@lab.ntt.co.jp> writes:
On 2019/05/27 10:52, Shohei Mochizuki wrote:
I noticed returning a modified record in a row-level BEFORE UPDATE trigger
on postgres_fdw foreign tables do not work. Attached patch fixes this issue.
This is because current fdw code adds only columns to RemoteSQL that were
explicitly targets of the UPDATE as follows.

Yeah.  So, the trigger execution correctly modifies the existing tuple
fetched from the remote server, but those changes are then essentially
discarded by postgres_fdw, that is, postgresExecForeignModify().

... Also, in the worst case, we'll end
up generating new query for every row being changed, because the trigger
may change different columns for different rows based on some condition.

Perhaps, if the table has relevant BEFORE triggers, we should just abandon
our attempts to optimize away fetching/storing all columns?  It seems like
another potential hazard here is a trigger needing to read a column that
is not mentioned in the SQL query.

The fetching side is fine, because rewriteTargetListUD() adds a
whole-row-var to the target list when the UPDATE / DELETE target is a
foreign table *and* there is a row trigger on the table.  postgres_fdw
sees that and constructs the query to fetch all columns.

So, the only problem here is the optimizing away of storing all columns,
which the Mochizuki-san's patch seems enough to fix.

Amit-san, Tom,
Thanks for the comments.

I checked other scenario. If a foreign table has AFTER trigger, remote update
query must return all columns and these cases are added at deparseReturningList
and covered by following existing test cases.

EXPLAIN (verbose, costs off)
UPDATE rem1 set f2 = '';          -- can't be pushed down
                                  QUERY PLAN
-------------------------------------------------------------------------------
 Update on public.rem1
   Remote SQL: UPDATE public.loc1 SET f2 = $2 WHERE ctid = $1 RETURNING f1, f2
   ->  Foreign Scan on public.rem1
         Output: f1, ''::text, ctid, rem1.*
         Remote SQL: SELECT f1, f2, ctid FROM public.loc1 FOR UPDATE
(5 rows)


Regards,

--
Shohei Mochizuki
TOSHIBA CORPORATION


Reply via email to