Hi, On 2023-01-25 22:00:50 +0300, Aleksander Alekseev wrote: > Perhaps that's not a bug especially considering the fact that the > documentation describes this behavior, but in any case the fact that: > > ``` > INSERT INTO t VALUES (1,1) ON CONFLICT (a) DO UPDATE SET b = 0; > INSERT INTO t VALUES (1,2) ON CONFLICT (a) DO UPDATE SET b = 0; > ``` > > and: > > ``` > INSERT INTO t VALUES (1,1), (1,2) ON CONFLICT (a) DO NOTHING; > `` > > .. both work, and: > > ``` > INSERT INTO t VALUES (1,1), (1,2) ON CONFLICT (a) DO UPDATE SET b = 0; > ``` > > ... doesn't is rather confusing. There is no reason why the latest > query shouldn't work except for a slight complication of the code. > Which seems to be a reasonable tradeoff, for me at least.
I don't agree that this is just about a "slight complication" of the code. I think semantically the proposed new behaviour is pretty bogus. It *certainly* can't be right to just continue with the update in heap_update, as you've done. You'd have to skip the update, not execute it. What am I missing here? 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. > > But what's the justification for erroring out in the DO NOTHING case? > > > > [...] > > > > It seems somewhat likely that a behavioural change will cause trouble for > > some > > of the uses of DO NOTHING out there. > > Just to make sure we are on the same page. The patch doesn't break the > current DO NOTHING behavior but rather makes DO UPDATE work the same > way DO NOTHING does. I see that now - I somehow thought you were recommending to error out in both cases, rather than the other way round. Greetings, Andres Freund
