Re: Using results from INSERT ... RETURNING

Marko Tiikkaja <marko(dot)tiikkaja(at)cs(dot)helsinki(dot)fi> writes:
> Tom Lane wrote:
>> Could you pull out a patch that includes those changes, please?

> Sorry for the delay, my master was a bit behind :-( . I moved the
> trigger code to nodeDml.c with minor changes and removed unused
> resultRelation stuff from DML nodes completely. This also has the
> README stuff in it.

Applied with a moderate amount of editorialization. Notably, I didn't
like what you'd done with the EvalPlanQual stuff, and after a bit of
reflection decided that the right thing was to teach EvalPlanQual to
execute just the desired subplan. Also, I put back the marking of
the ModifyTuple node with its target relations, which you'd removed
in the v4 patch --- I'm convinced that that will be necessary in some
form or other later, so taking it out now seemed like moving backward.

I did not do anything about changing EXPLAIN's output of trigger
information. The stumbling block there is that EXPLAIN executes
queued AFTER trigger events only after finishing the main plan tree
execution. The part of that that is driven by ModifyTable is just
the *queuing* of the triggers, not their *execution*. So my previous
claim that all the trigger execution would now be part of ModifyTable
was wrong. There are several things we could do here:

1. Nothing. I don't care for this, though, because it will lead to
the inconsistent behavior that BEFORE triggers count as part of the
displayed runtime for ModifyTuple and AFTER triggers don't.

2. Move actual execution of (non-deferred) AFTER triggers inside
ModifyTuple. This might be a good idea in order to have the most
consistent results for a series of WITH queries, but I'm not sure.

3. Have EXPLAIN show BEFORE triggers as associated with ModifyTuple
while still showing AFTER triggers as "free standing". Seems a bit
inconsistent.

Comments?

Also, working on this patch made me really want to pull SELECT FOR
UPDATE/SHARE locking out as a separate node too. We've talked about
that before but never got round to it. It's really necessary to do
that in order to have something like
INSERT INTO foo SELECT * FROM bar FOR UPDATE;
behave sanely. I don't recall at the moment whether that worked
sanely before, but it is definitely broken as of CVS tip. Perhaps
I'll work on that this weekend.

regards, tom lane