Hi,
> To me it's a pretty fundamental violation of how heap visibility works.
I don't think this has much to do with heap visibility. It's true that
generally a command doesn't see its own tuples. This is done in order
to avoid the Halloween problem which however can't happen in this
particular case.
Other than that the heap doesn't care about the visibility, it merely
stores the tuples. The visibility is determined by xmin/xmax, the
isolation level, etc.
It's true that the patch changes visibility rules in one very
particular edge case. This alone is arguably not a good enough reason
to reject a patch.
> Given that we skip the update in "UPDATE", your argument doesn't hold much
> water.
Peter made this argument above too and I will give the same anwer.
There is no reason why two completely different SQL statements should
behave the same.
> > That's a reasonable concern, however I was unable to break unique
> > constraints or triggers so far:
>
> I think you'd have to do a careful analysis of a lot of code for that to hold
> any water.
Alternatively we could work smarter, not harder, and let the hardware
find the bugs for us. Writing tests is much simpler and bullet-proof
than analyzing the code.
Again, to clarify, I'm merely playing the role of Devil's advocate
here. I'm not saying that the patch should necessarily be accepted,
nor am I 100% sure that it has any undiscovered bugs. However the
arguments against received so far don't strike me personally as being
particularly convincing.
As an example, one could argue that there are applications that
*expect* to get an ERROR in the case of self-conflicting inserts. And
by changing this behavior we will break these applications. If the
majority believes that we seriously care about this it would be a good
enough reason to withdraw the patch.
--
Best regards,
Aleksander Alekseev