Mochizuki-san, On 2019/05/28 13:10, Shohei Mochizuki wrote: > On 2019/05/28 12:54, Amit Langote wrote: >> On 2019/05/27 22:02, Tom Lane wrote: >>> 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)
Ah, I had missed the AFTER triggers case, which seems to be working fine as you've shown here. Thanks, Amit