Tom Lane wrote: > Alvaro Herrera <alvhe...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
