I wrote:
> I'm inclined to think that it would be best to move the responsibility
> for calling AfterTriggerBeginQuery/AfterTriggerEndQuery into the
> executor.  That would get us down to

>         CreateQueryDesc(...);
>         ExecutorStart(...);   // now includes AfterTriggerBeginQuery
>         ExecutorRun(...);     // zero or more times
>         ExecutorFinish(...);  // ModifyTable cleanup, AfterTriggerEndQuery
>         ExecutorEnd(...);     // just does what it's always done
>         FreeQueryDesc(...);

> where EXPLAIN without ANALYZE would skip ExecutorRun and ExecutorFinish.

> IMO the major disadvantage of a refactoring like this is the possibility
> of sins of omission in third-party code, in particular somebody not
> noticing the added requirement to call ExecutorFinish.  We could help
> them out by adding an Assert in ExecutorEnd to verify that
> ExecutorFinish had been called (unless explain-only mode).  A variant of
> that problem is an auto_explain-like add-on not noticing that they
> probably want to hook into ExecutorFinish if they'd previously been
> hooking ExecutorRun.  I don't see any simple check for that though.
> The other possible failure mode is forgetting to remove calls to the two
> trigger functions, but we could encourage getting that right by renaming
> those two functions.

This is committed.  I desisted from the last change (renaming the
trigger functions) because it seemed unnecessary.  If someone does
forget to remove redundant AfterTriggerBeginQuery/AfterTriggerEndQuery
calls, it won't hurt them much, just waste a few cycles stacking and
unstacking useless trigger contexts.

                        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

Reply via email to