Re: TABLESAMPLE patch is really in pretty sad shape

On 2015-07-24 01:26, Petr Jelinek wrote:
> On 2015-07-24 00:39, Tom Lane wrote:
>> I wrote:
>>> OK, so "InitSampleScan" for a function called at ExecInitSampleScan time
>>> (which we might as well make optional), and then we'll use
>>> BeginSampleScan
>>> for the function that gets the parameters.  The restart/ReScan function
>>> goes away since BeginSampleScan will take its place.
>>
>> Here's a WIP patch implementing things this way.  I've also taken the
>> time
>> to do a complete code review, and fixed quite a large number of things,
>> some cosmetic and some not so much.  I have not yet touched the tsm
>> contrib modules (so they won't even compile...), but I'm reasonably happy
>> with the state of the core code now.
>>
>
> Eh, I was just writing email with my rewrite :)
>
> I'll do review of this instead then.
>
> The only major difference that I see so far and I'd like you to
> incorporate that into your patch is that I renamed the SampleScanCost to
> SampleScanGetRelSize because that reflects much better the use of it, it
> isn't really used for costing, but for getting the pages and tuples of
> the baserel.
>

Ok, attached are couple of cosmetic changes - what I wrote above, plus
comment about why we do float8 + hashing for REPEATABLE (it's not
obvious IMHO) and one additional test query.

Do you want to do the contrib changes yourself as well or should I take it?

--
  Petr Jelinek                  http://www.2ndQuadrant.com/
  PostgreSQL Development, 24x7 Support, Training & Services