Hi, On 2023-02-08 16:08:39 +0300, Aleksander Alekseev wrote: > > 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.
Yes, and the fact is that cmin == cmax is something that we don't normally produce, yet you emit it now, without, as far as I can tell it, a convincing reason. > > > 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. 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. This is particularly true for something like heapam, where a lot of the tricky behaviour requires complicated interactions between multiple connections. > 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. I've said my piece, as-is I vote to reject the patch. Greetings, Andres Freund
