On Sat, May 4, 2024 at 10:46 PM Andrei Lepikhov <a.lepik...@postgrespro.ru> wrote: > Having no objective negative feedback, we have no reason to change > anything in the design or any part of the code. It looks regrettable and > unusual.
To me, this sounds like you think it's someone else's job to tell you what is wrong with the patch, or how to fix it, and if they don't, then you should get to have the patch as part of PostgreSQL. But that is not how we do things, nor should we. I agree that it sucks when you need feedback and don't get it, and I've written about that elsewhere and recently. But if you don't get feedback and as a result you can't get the patch to an acceptable level, or if you do get feedback but the patch fails to reach an acceptable level anyway, then the only correct decision is for us to not ship that code. That obviously sucks from the point of view of the patch author, and also of the committer, but consider the alternative. Once patches get through an initial release and become part of the product, the responsibility for fixing problems is understood to slowly move from the original committer to the community as a whole. In practice, that means that a lot of the work of fixing things that are broken, after some initial period, ends up falling on committers other than the person who did the initial commit. Even one or two problematic commits can generate an enormous amount of work for people who weren't involved in the original development and may not even have agreed with the development direction, and it is more than fair for those people to express a view about whether they are willing to carry that burden or not. When they aren't, I do think that's regrettable, but I don't think it's unusual. Just in this release, we've removed at least two previously-released features because they're in bad shape and nobody's willing to maintain them (snapshot too old, AIX support). > After designing the feature, fixing its bugs, and reviewing joint > patches on the commitfest, the question more likely lies in the planner > design. For example, I wonder if anyone here knows why exactly the > optimiser makes a copy of the whole query subtree in some places. > Another example is PlannerInfo. Can we really control all the > consequences of introducing, let's say, a new JoinDomain entity? Bluntly, if you can't control those consequences, then you aren't allowed to make that change. I know first-hand how difficult some of these problems are. Sometime in the last year or three, I spent weeks getting rid of ONE global variable (ThisTimeLineID). It took an absolutely inordinate amount of time, and it became clear to me that I was never going to get rid of enough global variables in that part of the code to be able to write a patch for the feature I wanted without risk of unforeseen consequences. So I gave up on the entire feature. Maybe I'll try again at some point, or maybe somebody else will feel like cleaning up that code and then I can try again with a cleaner base, but what I don't get to do is write a buggy patch for the feature I want and commit it anyway. I either figure out a way to do it that I believe is low-risk and that the community judges to be acceptable, or I don't do it. I want to go on record right now as disagreeing with the plan proposed in the commit message for the revert commit, namely, committing this again early in the v18 cycle. I don't think Tom would have proposed reverting this feature unless he believed that it had more serious problems than could be easily fixed in a short period of time. I think that concern is well-founded, given the number of fixes that were committed. It seems likely that the patch needs significant rework and stabilization before it gets committed again, and I think it shouldn't be committed again without explicit agreement from Tom or one of the other committers who have significant experience with the query planner. That is not to say that I don't approve generally of the idea of committing things earlier in the release cycle: I certainly do. It gives us more time to shake out problems with patches before we ship. But it only makes sense if we collectively believe that the patch is mostly correct, and only needs fine-tuning, and I think there are good reasons to believe that we shouldn't have that level of confidence in this case. -- Robert Haas EDB: http://www.enterprisedb.com
