What is HeapScanDescData.rs_initblock good for?
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?
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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.
--
�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
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().
I'm inclined to let it call heap_setscanlimits only if not allow_sync.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
Tom Lane wrote:
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 already did that, but feel free to whack it further if you're so
inclined.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Tom Lane wrote:
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
Tom Lane wrote:
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 already did that, but feel free to whack it further if you're so
inclined.
Nah, I just failed to see your commit.
--
�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