Re: Event Triggers: adding information

Dimitri Fontaine escribió:
> Robert Haas <robertmhaas@gmail.com> writes:
> > OK, I committed this.
>
> Thanks. Please find attached a rebased patch, v6. I think it's looking
> more like what you would expect now:

I gave this patch a look.  I'm not done with it; I mostly gave the
utility.c changes some love so that the end result is not as cluttered.
I did this by creating a couple of macros that call the Start() thing,
then set the OID, then call the End() thing.  This made pretty obvious
the places that were missing to set the object OID; I changed the CREATE
TABLE AS code to return it, for instance.  The patch I attach applies on
top of Dimitri's v6 patch.  With these changes, it's much easier to
review the big standard_ProcessUtility() switch and verify cases that
are being handled correctly.  (A few places cannot use the macros I
defined because they use more complex arrangements.  This is fine, I
think, because they are few enough that can be verified manually.)

I also noticed that ALTER DEFAULT PRIVILEGES was trying to fire event
triggers, even though we don't support GRANT and REVOKE; it seems to me
that the three things out to be supported together.  I'm told David
Fetter would call this "DCL support", and we're not supporting DCL at
this stage.  So DEFAULT PRIVILEGES ought to be not supported.

I also observed that the SECURITY LABEL commands does not fire event
triggers; I think it should.  But then maybe this can be rightly called
DCL as well, so perhaps it's all right to leave it out.

DROP OWNED is not firing event triggers.  There is a comment therein
that says "we don't fire them for shared objects", but this is wrong:
this command is dropping local objects, not shared, so it seems
necessary to me that triggers are fired.

I also noticed that some cases such as DROP <whatever> do not report the
OIDs of all the objects that are being dropped, only one of them.  This
seems bogus to me; if you do "DROP TABLE foo,bar" then both tables
should be reported.

> Given the OID, we use the ObjectProperty[] structure to know which cache
> or catalog to scan in order to get the name and namespace of the object.

The changes to make ObjectPropertyType visible to code outside
objectaddress.c look bogus to me.  I think we should make this new code
use the interfaces we already have.

--
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services