Dimitri Fontaine escribió: > Robert Haas <robertmh...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
