Re: postgres_fdw: batch inserts vs. before row triggers - Mailing list pgsql-hackers

On Tue, 19 Apr 2022 at 14:00, Tomas Vondra
<tomas.vondra@enterprisedb.com> wrote:
>
> On 4/19/22 11:16, Etsuro Fujita wrote:
> > On Sun, Apr 17, 2022 at 6:20 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
> >> Here
> >> is an example using HEAD:
> >>
> >> create extension postgres_fdw;
> >> create server loopback foreign data wrapper postgres_fdw options
> >> (dbname 'postgres');
> >> create user mapping for current_user server loopback;
> >> create table t (a int);
> >> create foreign table ft (a int) server loopback options (table_name 't');
> >> create function ft_rowcount_tf() returns trigger as $$ begin raise
> >> notice '%: rows = %', tg_name, (select count(*) from ft); return new;
> >> end; $$ language plpgsql;
> >> create trigger ft_rowcount before insert on ft for each row execute
> >> function ft_rowcount_tf();
> >>
> >> insert into ft select i from generate_series(1, 10) i;
> >> NOTICE:  ft_rowcount: rows = 0
> >> NOTICE:  ft_rowcount: rows = 1
> >> NOTICE:  ft_rowcount: rows = 2
> >> NOTICE:  ft_rowcount: rows = 3
> >> NOTICE:  ft_rowcount: rows = 4
> >> NOTICE:  ft_rowcount: rows = 5
> >> NOTICE:  ft_rowcount: rows = 6
> >> NOTICE:  ft_rowcount: rows = 7
> >> NOTICE:  ft_rowcount: rows = 8
> >> NOTICE:  ft_rowcount: rows = 9
> >> INSERT 0 10
> >>
> >> This looks good, but when batch insert is enabled, the trigger
> >> produces incorrect results:
> >>
> >> alter foreign table ft options (add batch_size '10');
> >> delete from ft;
> >>
> >> insert into ft select i from generate_series(1, 10) i;
> >> NOTICE:  ft_rowcount: rows = 0
> >> NOTICE:  ft_rowcount: rows = 0
> >> NOTICE:  ft_rowcount: rows = 0
> >> NOTICE:  ft_rowcount: rows = 0
> >> NOTICE:  ft_rowcount: rows = 0
> >> NOTICE:  ft_rowcount: rows = 0
> >> NOTICE:  ft_rowcount: rows = 0
> >> NOTICE:  ft_rowcount: rows = 0
> >> NOTICE:  ft_rowcount: rows = 0
> >> NOTICE:  ft_rowcount: rows = 0
> >> INSERT 0 10
> >
> > Actually, the results are correct, as we do batch-insert here.  But I
> > just wanted to show that the trigger behaves *differently* when doing
> > batch-insert.
>
> +1, I think it's a bug to do batch insert in this case.

I don't have a current version of the SQL spec, but one preliminary
version of SQL:2012 I retrieved via the wiki details that all BEFORE
triggers on INSERT/UPDATE/DELETE statements are all executed before
_any_ of that statements' affected data is modified.

See the "SQL:2011 (preliminary)" document you can grab on the wiki,
Part 2: for INSERT, in 15.10 (4) the BEFORE triggers on the changeset
are executed, and only after that in section 15.10 (5)(c) the
changeset is inserted into the target table. During the BEFORE-trigger
this table does not contain the rows of the changeset, thus a count(*)
on that table would result in a single value for all the BEFORE
triggers triggered on that statement, regardless of the FOR EACH ROW
specifier. The sections for DELETE are 15.7 (6) and 15.7 (7); and for
UPDATE 15.13(7) and 15.13(9) respectively.

I don't know about the semantics of triggers in the latest SQL
standard versions, but based on that sample it seems like we're
non-compliant on BEFORE trigger behaviour, and it doesn't seem like
it's documented in the trigger documentation.

I seem to recall a mail on this topic (changes in trigger execution
order with respect to the DML it is triggered by in the newest SQL
spec) but I can't seem to find that thread.

Kind regards,

Matthias van de Meent