Re: Command Triggers patch v18

Thom Brown <thom@linux.com> writes:
> The new command triggers work correctly.

Thanks for your continued testing :)

> Having looked at your regression tests, you don't seem to have enough
> "before" triggers in the tests.  There's no test for before CREATE
> TABLE, CREATE TABLE AS or SELECT INTO.  In my tests I have 170 unique
> command triggers, but there are only 44 in the regression test.  Is
> there a reason why there aren't many tests?

Now that we share the same code for ANY triggers and specific ones, I
guess we could drop a lot of specific command triggers from the
regression tests.

> A problem still outstanding is that when I build the docs, the CREATE

I would like to get back on code level review now if at all possible,
and I would integrate your suggestions here into the next patch revision
if another one is needed.

The only point yet to address from last round from Andres is about the
API around CommandFiresTrigger() and the Memory Context we use here.
We're missing an explicit Reset call, and to be able to have we need to
have a more complex API, because of the way RemoveObjects() and
RemoveRelations() work.

We would need to add no-reset APIs and an entry point to manually reset
the memory context, which currently gets disposed at the same time as
its parent context, the current one that's been setup before entering
standard_ProcessUtility().

Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr     PostgreSQL : Expertise, Formation et Support