Re: [HACKERS] PG10 transition tables, wCTEs and multiple operations on the same table

On Fri, Jun 2, 2017 at 8:46 PM, Thomas Munro
<thomas.munro@enterprisedb.com> wrote:

> So, afterTriggers.query_stack is used to handle the reentrancy that
> results from triggers running further statements that might fire
> triggers.  It isn't used for dealing with extra ModifyTable nodes that
> can appear in a plan because of wCTEs.  Could it also be used for that
> purpose?  I think that would only work if it is the case that each
> ModifyTable node begin and then runs to completion (ie no interleaving
> of wCTE execution) and then its AS trigger fires, which I'm not sure
> about.

I don't think we want to commit to anything that depends on a CTE
creating an optimization fence, although *maybe* that would be OK in
the case of DML as a CTE.  That's a pretty special case; I'm not
sure whether the standard discusses it.

> If that is the case, perhaps AfterTriggerBeginQuery and
> AfterTriggerEndQuery could become the responsibility of
> nodeModifyTable.c rather than execMain.c.  I haven't tried this yet
> and it may well be too ambitious at this stage.

In a world where one statement can contain multiple DML statements
within CTEs, that may make sense regardless of transition table
issues; but I agree we're past the time to be considering making
such a change for v10.

> Other ideas: (1) ban wCTEs that target relations with transition
> tables in PG10, because we're out of time;

Given that we haven't even discussed what to do about an UPDATE
statement with a CTE that updates the same table when there are
BEFORE EACH ROW UPDATE triggers on that table (which perhaps return
NULL for some but not all rows?), I'm not sure we've yet plumbed the
depths of this morass.

For example, for this:

drop table if exists t1 cascade;

create table t1 (c1 int not null, c2 text not null default '');
insert into t1 (c1) values (1), (2), (3), (4), (5);

drop function if exists t1_upd_func() cascade;
create function t1_upd_func() returns trigger language plpgsql
as $$
begin if old.c1 between 2 and 4 then   new.c2 = old.c2 || ';' || old.c1::text || ',' || new.c1::text; end if; if old.c1
>3 then   return null; end if; return new;
 
end;
$$;
create trigger t1_upd_update before update on t1 for each row execute procedure t1_upd_func();
with x as (update t1 set c1 = c1 - 1 returning c1) update t1 set c1 = t1.c1 + 1 from x where x.c1 = t1.c1;
select * from t1;

... what is the correct result?

I get this:
c1 |  c2
----+------ 4 | 5 | 0 | 1 | ;2,1 2 | ;3,2
(5 rows)

It is definitely not what I expected, and seems wrong in multiple
ways from a logical perspective, looking  at those as set-based
operations.  (Tautologies about the procedural mechanism that
creates the result are not defenses of the result itself.)

Can we fix things exclusive of transition tables in this release?
If not, the only reasonable thing to do is to try to avoid further
complicating things with CTE/transition table usage until we sort
out the basics of triggers firing from CTE DML in the first place.

--
Kevin Grittner
VMware vCenter Server
https://www.vmware.com/