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
