Re: Tid scan increments value of pg_stat_all_tables.seq_scan. (butnot seq_tup_read) - Mailing list pgsql-hackers

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.

Attach the v2 patch which change the status to following. (same as
-v11 but have new SO_TYPE_TIDSCAN flag)

                                                 increments
scan type    table_beginscan?, per scan, per tuple     , SO_TYPE flags
=============================================================================
TID scan   : yes             , <none>, <none>        , SO_TYPE_TIDSCAN

Is it acceptable change for HEAD and v12?

Best regards,

-- 
Tatsuhito Kasahara
kasahara.tatsuhito _at_ gmail.com