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
