Re: Tid scan increments value of pg_stat_all_tables.seq_scan. (butnot seq_tup_read)

On 2020/02/03 16:39, Kasahara Tatsuhito wrote:
> On Mon, Feb 3, 2020 at 4:22 PM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:
>> On 2020/02/01 16:05, Kasahara Tatsuhito wrote:
>>>       if (scan->rs_base.rs_flags & (SO_TYPE_SEQSCAN | SO_TYPE_SAMPLESCAN))
>>>       {
>>>           /*
>>>            * Ensure a missing snapshot is noticed reliably, even if the
>>>            * isolation mode means predicate locking isn't performed (and
>>>            * therefore the snapshot isn't used here).
>>>            */
>>>           Assert(snapshot);
>>>           PredicateLockRelation(relation, snapshot);
>>>       }
>>>
>>> Therefore, it can not simply remove the SO_TYPE_SEQSCAN flag from a TID scan.
>>> To keep the old behavior, I think it would be better to add a new
>>> SO_TYPE_TIDSCAN flag and take a predicate lock on the entire relation.
>>
>> But in the old behavior, PredicateLockRelation() was not called in TidScan case
>> because its flag was not SO_TYPE_SEQSCAN. No?
> No. Tid scan called PredicateLockRelation() both previous and current.
> 
> In the current (v12 and HEAD), Tid scan has SO_TYPE_SEQSCAN flag so
> that PredicateLockRelation()is called in Tid scan.
> In the Previous (- v11), in heap_beginscan_internal(), checks
> is_bitmapscan flags.
> If is_bitmapscan is set to false, calls PredicateLockRelation().
> 
> (- v11)
>      if (!is_bitmapscan)
>          PredicateLockRelation(relation, snapshot);
> 
> And in the Tid scan,  is_bitmapscan is set to false, so that
> PredicateLockRelation()is called in Tid scan.

Thanks for explaining that! But heap_beginscan_internal() was really
called in TidScan case?

Regards,


-- 
Fujii Masao
NTT DATA CORPORATION
Advanced Platform Technology Group
Research and Development Headquarters