Re: TABLESAMPLE patch is really in pretty sad shape

Petr Jelinek <petr(at)2ndquadrant(dot)com> writes:
> Another thing that's not clear to me after playing with this is how do
> we want to detect if to do pagemode scan or not. I understand that it's
> neat optimization to say pagemode = true in bernoulli when percentage is
> high and false when it's low but then this would have to come from the
> BeginSampleScan() in the proposed API, but then BeginSampleScan would
> not have access to HeapScanDesc as it would not be inited yet when it's
> called.

Right.

> The info that BeginSampleScan() needs can be obtained directly
> from ss_currentRelation I guess, but it's somewhat strange to pass
> semi-initialized SampleScanState to the BeginSampleScan().

Doesn't seem like a big problem from here. The HeapScanDesc pointer
will be null at that point, so it's not like attempts to access it
would escape notice.

We could alternatively provide two scan-initialization callbacks,
one that analyzes the parameters before we do heap_beginscan,
and another that can do additional setup afterwards. Actually,
that second call would really not be meaningfully different from
the ReScan call, so another solution would be to just automatically
invoke ReScan after we've created the HeapScanDesc. TSMs could work
around not having this by complicating their NextBlock function a bit
(so that it would do some initialization on first call) but perhaps
it would be easier to have the additional init call.

regards, tom lane