Re: [PATCH] Make ON CONFLICT DO NOTHING and ON CONFLICT DO UPDATE consistent

From: Aleksander Alekseev <aleksander(at)timescale(dot)com>
To: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Andres Freund <andres(at)anarazel(dot)de>, Peter Geoghegan <pg(at)bowt(dot)ie>
Subject: Re: [PATCH] Make ON CONFLICT DO NOTHING and ON CONFLICT DO UPDATE consistent
Date: 2023-01-26 10:07:08
Message-ID: CAJ7c6TMLsSAbE8pGkub7CvYxL3pPaqXVQ44ff1V2-a0XiPVeaw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

Attachment Content-Type Size
v2-0001-Make-ON-CONFLICT-DO-NOTHING-and-ON-CONFLICT-DO-UP.patch application/octet-stream 19.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Dean Rasheed 2023-01-26 10:27:58 Re: to_hex() for negative inputs
Previous Message Alvaro Herrera 2023-01-26 09:42:41 Re: Helper functions for wait_for_catchup() in Cluster.pm