Hi Andres, > It *certainly* can't be right to just continue with the update in heap_update,
I see no reason why. What makes this case so different from updating a tuple created by the previous command? > as you've done. You'd have to skip the update, not execute it. What am I > missing here? Simply skipping updates in a statement that literally says DO UPDATE doesn't seem to be the behavior a user would expect. > I think this'd completely break triggers, for example, because they won't be > able to get the prior row version, since it won't actually be a row ever > visible (due to cmin=cmax). > > I suspect it might break unique constraints as well, because we'd end up with > an invisible row in part of the ctid chain. That's a reasonable concern, however I was unable to break unique constraints or triggers so far: ``` CREATE TABLE t (a INT UNIQUE, b INT); CREATE OR REPLACE FUNCTION t_insert() RETURNS TRIGGER AS $$ BEGIN RAISE NOTICE 't_insert triggered: new = %, old = %', NEW, OLD; RETURN NULL; END $$ LANGUAGE 'plpgsql'; CREATE OR REPLACE FUNCTION t_update() RETURNS TRIGGER AS $$ BEGIN RAISE NOTICE 't_update triggered: new = %, old = %', NEW, OLD; RETURN NULL; END $$ LANGUAGE 'plpgsql'; CREATE TRIGGER t_insert_trigger AFTER INSERT ON t FOR EACH ROW EXECUTE PROCEDURE t_insert(); CREATE TRIGGER t_insert_update AFTER UPDATE ON t FOR EACH ROW EXECUTE PROCEDURE t_update(); INSERT INTO t VALUES (1,1), (1,2) ON CONFLICT (a) DO UPDATE SET b = 0; NOTICE: t_insert triggered: new = (1,1), old = <NULL> NOTICE: t_update triggered: new = (1,0), old = (1,1) INSERT INTO t VALUES (2,1), (2,2), (3,1) ON CONFLICT (a) DO UPDATE SET b = 0; NOTICE: t_insert triggered: new = (2,1), old = <NULL> NOTICE: t_update triggered: new = (2,0), old = (2,1) NOTICE: t_insert triggered: new = (3,1), old = <NULL> =# SELECT * FROM t; a | b ---+--- 1 | 0 2 | 0 3 | 1 ``` PFA patch v2 that also includes the test shown above. Are there any other scenarios we should check? -- Best regards, Aleksander Alekseev
v2-0001-Make-ON-CONFLICT-DO-NOTHING-and-ON-CONFLICT-DO-UP.patch
Description: Binary data