Dimitri Fontaine escribió: > Alvaro Herrera <alvhe...@2ndquadrant.com> writes:
> >> 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. > > We need to move the new fonction get_object_name_and_namespace() into > the objectaddress.c "module" then, and decide about returning yet > another structure (because we have two values to return) or to pass that > function a couple of char **. Which devil do you prefer? I made some changes to this, and I think the result (attached) is cleaner overall. Now, this review is pretty much unfinished as far as I am concerned; mainly I've been trying to figure out how it all works and improving some stuff as I go, and I haven't looked at the complete patch yet. We might very well doing some things quite differently; for example I'm not really sure of the way we handle CommandTags, or the way we do object lookups at the event trigger stage, only to have it repeated later when the actual deletion takes place. In particular, since event_trigger.c does not know the lock strength that's going to be taken at deletion, it only grabs ShareUpdateExclusiveLock; so there is lock escalation here which could lead to deadlocks. This might need to be rethought. I added a comment somewhere, noting that perhaps it's ProcessUtility's job to set the object classId (or at least the ObjectType) at the same time the TargetOid is being set, rather than have event_trigger.c figure it out from the parse node. And so on. I haven't looked at plpgsql or pg_dump yet, either. We've been discussing on IM the handling of DROP in general. The current way it works is that the object OID is reported only if the toplevel command specifies to drop a single object (so no OID if you do DROP TABLE foo,bar); and this is all done by standard_ProcessUtility. This seems bogus to me, and it's also problematic for things like DROP SCHEMA, as well as DROP OWNED BY (which, as noted upthread, is not handled at all but should of course be). I think the way to do it is have performDeletion and performMultipleDeletions (dependency.c) call into the event trigger code, by doing something like this: 1. look up all objects to drop (i.e. resolve the list of objects specified by the user, and complete with objects dependent on those) 2. iterate over this list, firing DROP at-start event triggers 3. do the actual deletion of objects (i.e. deleteOneObject, I think) 4. iterate again over the list firing DROP at-end event triggers We would have one or more event triggers being called with a context of TOPLEVEL (for objects directly mentioned in the command), and then some more calls with a context of CASCADE. Exactly how should DROP OWNED BY be handled is not clear; perhaps we should raise one TOPLEVEL event for the objects directly owned by the role? Maybe we should just do CASCADE for all objects? This is not clear yet. Thoughts? -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
event_trigger_infos.7.alvherre1.patch.gz
Description: Binary data
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
