On 01/16/2015 03:01 PM, Andres Freund wrote:
Hi,/* - * Grab an exclusive lock on the pk table, so that someone doesn't delete - * rows out from under us. (Although a lesser lock would do for that - * purpose, we'll need exclusive lock anyway to add triggers to the pk - * table; trying to start with a lesser lock will just create a risk of - * deadlock.) + * Grab ShareRowExclusiveLock on the pk table, so that someone doesn't + * delete rows out from under us. Note that this does not create risks + * of deadlocks as triggers add added to the pk table using the same + * lock. */"add added" doesn't look intended. The rest of the sentence doesn't look entirely right either.
It was intended to be "are added", but the sentence is pretty awful anyway. I am not sure the sentence is really necessary anyway.
/* * Triggers must be on tables or views, and there are additional @@ -526,8 +526,7 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString, * can skip this for internally generated triggers, since the name * modification above should be sufficient. * - * NOTE that this is cool only because we have AccessExclusiveLock on the - * relation, so the trigger set won't be changing underneath us. + * NOTE that this is cool only because of the unique contraint.I fail to see what the unique constraint has to do with this? The previous comment refers to the fact that the AccessExclusiveLock is what prevents a race where another transaction adds a trigger with the same name already exists. Yes, the unique index would, as noted earlier in the comment, catch the error. But that's not the point of the check. Unless I miss something the comment is just as true if you replace the access exclusive with share row exlusive as it's also self conflicting.
Yeah, this must have been a remainder from the version where I only grabbed a ShareLock. The comment should be restored.
Hm. Pushing the snapshot is supposed to make this fully mvcc? Idon't think that's actually sufficient because of the deparsing of the WHEN clause and of the function name.
Indeed. As Noah and I discussed previously in this thread we would need to do quite a bit of refactoring of ruleutils.c to make it fully MVCC. For this reason I opted to only lower the lock levels of ADD and ALTER TRIGGER, and not DROP TRIGGER. Neither of those require MVCC of then WHEN clause.
Andreas -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
