Re: TABLESAMPLE patch is really in pretty sad shape

On 13 July 2015 at 14:39, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> Michael Paquier <michael(dot)paquier(at)gmail(dot)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/
<http://www.2ndquadrant.com/>
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services