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

On 2020/02/01 16:05, Kasahara Tatsuhito wrote:
> On Thu, Jan 30, 2020 at 1:49 PM Kyotaro Horiguchi
> <horikyota.ntt@gmail.com> wrote:
>> At Thu, 30 Jan 2020 13:30:56 +0900, Kasahara Tatsuhito <kasahara.tatsuhito@gmail.com> wrote in
>>>> TID scan   : yes             , seq_scan, <none>        , <none>
>>> Here is wrong, because TID scan came to have SO_TYPE_SEQSCAN flags
>>> from commit 147e3722f7.
>>
>> It is reflectings the discussion below, which means TID scan doesn't
>> have corresponding SO_TYPE_ value. Currently it is setting
>> SO_TYPE_SEQSCAN by accedent.
> Ah, sorry I misunderstood..
> 
> Upon further investigation, the SO_TYPE_SEQSCAN flag was also used at
> heap_beginscan() to determine whether a predicate lock was taken on
> the entire relation.
> 
>      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?

Regards,


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