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

Hi,

> Yes, and the fact is that cmin == cmax is something that we don't normally
> produce

Not sure if this is particularly relevant to this discussion but I
can't resist noticing that the heap doesn't even store cmin and
cmax... There is only HeapTupleHeaderData.t_cid and flags. cmin/cmax
are merely smoke and mirrors we use to trick a user.

And yes, the patch doesn't seem to break much mirrors:

```
=# create table t (a int unique, b int);
=# insert into t values (1,1), (1,2) on conflict (a) do update set b = 0;
=# SELECT xmin, xmax, cmin, cmax, * FROM t;
 xmin | xmax | cmin | cmax | a | b
------+------+------+------+---+---
  731 |    0 |    0 |    0 | 1 | 0
=# begin;
=# insert into t values (2,1), (2,2), (3,1) on conflict (a) do update set b = 0;
=# SELECT xmin, xmax, cmin, cmax, * FROM t;
 xmin | xmax | cmin | cmax | a | b
------+------+------+------+---+---
  731 |    0 |    0 |    0 | 1 | 0
  732 |    0 |    0 |    0 | 2 | 0
  732 |    0 |    0 |    0 | 3 | 1

=# insert into t values (2,1), (2,2), (3,1) on conflict (a) do update set b = 0;
=# SELECT xmin, xmax, cmin, cmax, * FROM t;
 xmin | xmax | cmin | cmax | a | b
------+------+------+------+---+---
  731 |    0 |    0 |    0 | 1 | 0
  732 |  732 |    1 |    1 | 2 | 0
  732 |  732 |    1 |    1 | 3 | 0

=# commit;
=# SELECT xmin, xmax, cmin, cmax, * FROM t;
 xmin | xmax | cmin | cmax | a | b
------+------+------+------+---+---
  731 |    0 |    0 |    0 | 1 | 0
  732 |  732 |    1 |    1 | 2 | 0
  732 |  732 |    1 |    1 | 3 | 0
```

> That's a spectactularly wrong argument in almost all cases. Unless you have a
> way to get to full branch coverage or use a model checker that basically does
> the same, testing isn't going to give you a whole lot of confidence that you
> haven't introduced bugs.

But neither will reviewing a lot of code...

> I've said my piece, as-is I vote to reject the patch.

Fair enough. I'm merely saying that rejecting a patch because it
doesn't include a TLA+ model is something novel :)

> I don't buy your argument about DO UPDATE needing to be brought into
> line with DO NOTHING. In any case I'm pretty sure that Tom's remarks
> in 2016 about a behavioral inconsistencies (which you cited) actually
> called for making DO NOTHING more like DO UPDATE -- not the other way
> around.

Interesting. Yep, we could use a bit of input from Tom on this one.

This of course would break backward compatibility. But we can always
invent something like:

```
INSERT INTO ..
ON CONFLICT DO [NOTHING|UPDATE .. ]
[ALLOWING|FORBIDDING] SELF CONFLICTS;
```

... if we really want to.

> problem. To me the best argument is also the simplest: who would want
> us to allow it, and for what purpose?

Good question.

This arguably has little use for application developers. As an
application developer you typically know your unique constraints and
using this knowledge you can rewrite the query as needed and add any
other accompanying logic.

However, extension developers, as an example, often don't know the
underlying unique constraints (more specifically, it's difficult to
look for them and process them manually) and often have to process any
garbage the application developer passes to an extension.

This of course is applicable not only to extensions, but to any
middleware between the DBMS and the application.

-- 
Best regards,
Aleksander Alekseev