On Mon, Aug 22, 2022 at 9:52 PM Jonathan S. Katz <jk...@postgresql.org> wrote: > > Andres, Robert: Do these changes address your concerns about the use of > > substransactions and reduce the risk of xid wraparound? > > Andres, Andrew, Amit, Robert -- as you have either worked on this or > expressed opinions -- any thoughts on this current patch set?
I do not think that using subtransactions as part of the expression evaluation process is a sound idea pretty much under any circumstances. Maybe if the subtransations aren't commonly created and don't usually get XIDs there wouldn't be a big problem in practice, but it's an awfully heavyweight operation to be done inside expression evaluation even in corner cases. I think that if we need to make certain operations that would throw errors not throw errors, we need to refactor interfaces until it's possible to return an error indicator up to the appropriate level, not just let the error be thrown and catch it. The patches in question are thousands of lines of new code that I simply do not have time or interest to review in detail. I didn't commit this feature, or write this feature, or review this feature. I'm not familiar with any of the code. To really know what's going on here, I would need to review not only the new patches but also all the code in the original commits, and probably some of the preexisting code from before those commits that I have never examined in the past. That would take me quite a few months even if I had absolutely nothing else to do. And because I haven't been involved in this patch set in any way, I don't think it's really my responsibility. At the end of the day, the RMT is going to have to take a call here. It seems to me that Andres's concerns about code quality and lack of comments are probably somewhat legitimate, and in particular I do not think the use of subtransactions is a good idea. I also don't think that trying to fix those problems or generally improve the code by committing thousands of lines of new code in August when we're targeting a release in September or October is necessarily a good idea. But I'm also not in a position to say that the project is going to be irreparably damaged if we just ship what we've got, perhaps after fixing the most acute problems that we currently know about. This is after all relatively isolated from the rest of the system. Fixing the stuff that touches the core executor is probably pretty important, but beyond that, the worst thing that happens is the feature sucks and people who try to use it have bad experiences. That would be bad, and might be a sufficient reason to revert, but it's not nearly as bad as, say, the whole system being slow, or data loss for every user, or something like that. And we do have other bad code in the system. Is this a lot worse? I'm not in a position to say one way or the other. -- Robert Haas EDB: http://www.enterprisedb.com
