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

On 2020/02/03 16:39, Kasahara Tatsuhito wrote:
> On Mon, Feb 3, 2020 at 4:22 PM Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)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