Re: TABLESAMPLE patch is really in pretty sad shape

Petr Jelinek <petr(at)2ndquadrant(dot)com> writes:
> On 2015-07-19 22:56, Tom Lane wrote:
>> * I specified that omitting NextSampleBlock is allowed and causes the
>> core code to do a standard seqscan, including syncscan support, which
>> is a behavior that's impossible with the current API. If we fix
>> the bernoulli code to have history-independent sampling behavior,
>> I see no reason that syncscan shouldn't be enabled for it.

> Umm, we were actually doing syncscan as well before.

Doesn't look like it to me: heap_beginscan_sampling always passes
allow_sync = false to heap_beginscan_internal.

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.

regards, tom lane