On 2015-01-16 15:16:20 +0100, Andreas Karlsson wrote: > 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.
Right. > 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. I'm unconvinced that this is true. Using a snapshot for part of getting a definition certainly opens the door for getting strange results. Acquiring a lock that prevents schema changes on the table and then getting the definition using the syscaches guarantees that that definition is at least self consistent because no further schema changes are possible and the catalog caches will be up2date. What you're doing though is doing part of the scan using the transaction's snapshot (as used by pg_dump that will usually be a repeatable read snapshot and possibly quite old) and the other using a fresh catalog snapshot. This can result in rather odd things. Just consider: S1: CREATE TABLE flubber(id serial primary key, data text); S1: CREATE FUNCTION blarg() RETURNS TRIGGER LANGUAGE plpgsql AS $$BEGIN RETURN NEW; END;$$; S1: CREATE TRIGGER flubber_blarg BEFORE INSERT ON flubber FOR EACH ROW WHEN (NEW.data IS NOT NULL) EXECUTE PROCEDURE blarg(); S2: BEGIN TRANSACTION ISOLATION LEVEL SERIALIZABLE; S2: SELECT 'dosomethingelse'; S1: ALTER TABLE flubber RENAME TO wasflubber; S1: ALTER TABLE wasflubber RENAME COLUMN data TO wasdata; S1: ALTER TRIGGER flubber_blarg ON wasflubber RENAME TO wasflubber_blarg; S1: ALTER FUNCTION blarg() RENAME TO wasblarg; S2: SELECT pg_get_triggerdef(oid) FROM pg_trigger; This will give you: The old trigger name. The new table name. The new column name. The new function name. I don't think using a snapshot for tiny parts of these functions actually buys anything. Now, this isn't a pattern you introduced. But I think we should think about this for a second before expanding it further. Before you argue that this isn't relevant for pg_dump: It is. Precisely the above can happen - just replace the 'dosomethingelse' with several LOCK TABLEs as pg_dump does. The first blocks after a snapshot has been acquired. While waiting, all the ALTERs happen. Arguably the benefit here is that the window for this issue is becoming smaller as pg_dump (and hopefully other possible callers) acquire exclusive locks on the table. I.e. that the lowering of the lock level doesn't introduce new races. But on the other side of the coin, this makes the result of pg_get_triggerdef() even *more* inaccurate in many cases. Greetings, Andres Freund -- Andres Freund 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
