Re: TABLESAMPLE patch is really in pretty sad shape

On 2015-07-20 17:23, Tom Lane wrote:
>
> Doesn't look like it to me: heap_beginscan_sampling always passes
> allow_sync = false to heap_beginscan_internal.
>

Oh, right.

> More to the point, the existing coding of all these methods is such
> that they would fail to be reproducible if syncscan were enabled,
> because the scan start point wouldn't be the same. That's fixable,
> but it'll take more work than just dealing with the above oversight.
> In any case, given that Simon has stated he wants support for sample
> methods that don't try to be reproducible, we need the method
> to be able to say whether syncscan is allowed.
>

I am not completely clear on how we do this tbh, do you mean we use the
info about repeatability for this?

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. 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().

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