Marko Tiikkaja <marko.tiikk...@cs.helsinki.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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers