Tom Lane wrote:
> Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> > Tom Lane wrote:
> >> The BRIN patch added a HeapScanDescData field rs_initblock, but so far as
> >> I can see it's utterly without use, and it's quite confusing (people might
> >> mistake it for rs_startblock, for example). Any objection to taking it
> >> out again?
>
> > Ouch, you're right, my mistake. Feel free to remove it, yeah.
>
> ... While I'm looking at it, it sure looks like the BRIN patch broke
> syncscan for those index build methods that were using it, which was
> most. You've got IndexBuildHeapRangeScan unconditionally calling
> heap_setscanlimits and thereby trashing the result of ss_get_location().
Hmm, right, I failed to notice that.
> I'm inclined to let it call heap_setscanlimits only if not allow_sync.
It is possible for a partial range scan to join an existing herd of
scans that happens to be processing that part of the table, in which
case this wouldn't be sufficient. However, two considerations: 1) range
scans, at least for BRIN, aren't normally large enough for synscans to
matter all that much; and 2) it would require additional code. So I'm
inclined to do it as you suggest, which is simplest.
(I think this is what the rs_initblock thing was for BTW: set up an
initial block number other than 0 for the range scan, so that when it
reached the end in a syncscan that started further ahead, it knew what
block to "overflow" back to.)
One scenario in which the sync scan could matter is initial index
creation.
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services