Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> wrote:
> On 29.06.2011 00:33, Kevin Grittner wrote:
>> Heikki Linnakangas wrote:
>>> On 28.06.2011 20:47, Kevin Grittner wrote:
>>
>>> Hmm, the calls in question are the ones in heapgettup() and
>>> heapgettup_pagemode(), which are subroutines of heap_getnext().
>>> heap_getnext() is only used in sequential scans, so it seems
>>> safe to remove those calls.
>>
>> I haven't found anything to the contrary, if I understand
>> correctly, Dan found the same, and all the tests pass without
>> them. Here's a patch to remove them. This makes the
>> recently-added rs_relpredicatelocked boolean field unnecessary,
>> so that's removed in this patch, too.
>
> Thanks, committed. I also moved the PredicateLockRelation() call
> to heap_beginscan(), per earlier discussion.
Thanks!
Before we leave the subject of modularity, do you think the entire
"else" clause dealing with the lossy bitmaps should be a heapam.c
function called from nodeBitmapHeapscan.c? With the move of the
PredicateLockRelation() call you mention above, that leaves this as
the only place in the executor which references SSI, and it also is
the only place in the executor to call PageGetMaxOffsetNumber() and
OffsetNumberNext(), which seem like AM things. The logic seems
somewhat similar to heap_hot_search_buffer() and such a function
would take roughly the same parameters.
On the other hand, it's obviously not a bug, so maybe that's
something to put on a list to look at later.
-Kevin
