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

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.

Best regards,

--
Tatsuhito Kasahara
kasahara.tatsuhito _at_ gmail.com