Michael Paquier <michael.paquier@gmail.com> writes: > Regarding the fact that those two contrib modules can be part of a > -contrib package and could be installed, nuking those two extensions > from the tree and preventing the creating of custom tablesample > methods looks like a correct course of things to do for 9.5.
TBH, I think the right thing to do at this point is to revert the entire patch and send it back for ground-up rework. I think the high-level design is wrong in many ways and I have about zero confidence in most of the code details as well.
Based on the various comments here, I don't see that as the right course of action at this point.
There are no issues relating to security or data loss, just various fixable issues in a low-impact feature, which in my view is an important feature also.
If it's to stay, it *must* get a line-by-line review from some committer-level person; and I think there are other more important things for us to be doing for 9.5.
Honestly, I am very surprised by this. My feeling was the code was neat, clear and complete, much more so than many patches I review. If I had thought the patch or its implementation was in any way contentious I would not have committed it.
I take responsibility for the state of the code and will put time into addressing the concerns mentioned and others.
If we cannot resolve them in reasonable time, a revert is possible: there is nothing riding on this from me.
--
Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services