Re: BEFORE UPDATE trigger on postgres_fdw table not work

On 2019/05/28 12:54, Amit Langote wrote:
> On 2019/05/27 22:02, Tom Lane wrote:
>> Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)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