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

Hello.

At Wed, 29 Jan 2020 23:24:09 +0900, Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com> wrote in
> On 2020/01/29 20:06, Kasahara Tatsuhito wrote:
> > Hi.
> > Attached patch solve this problem.
> > This patch adds table_beginscan_tid() and call it in TidListEval()
> > instead of table_beginscan().
> > table_beginscan_tid() is the same as table_beginscan() but do not set
> > SO_TYPE_SEQSCAN to flags.
> > Although I'm not sure this behavior is really problem or not,
> > it seems to me that previous behavior is more prefer.
> > Is it worth to apply to HEAD and v12 branch ?
>
> I've not read the patch yet, but I agree that updating only seq_scan
> but not seq_tup_read in Tid Scan sounds strange. IMO at least
> both should be update together or neither should be updated.

Basically agreed, but sample scan doesn't increment seq_scan but
increments seq_tup_read.

Aside from that fact, before 147e3722f7 TidScan didn't need a scan
descriptor so didn't call table_beginscan. table_beginscan didn't
increment the counter for bitmapscan and samplescan. The commit
changes TidScan to call beginscan but didn't change table_beginscan
not to increment the counter for tidscans.

From the view of the view pg_stat_*_tables, the counters moves as follows.

increments
scan type table_beginscan?, per scan, per tuple , SO_TYPE flags
=============================================================================
seq scan : yes , seq_scan, seq_tup_read , SO_TYPE_SEQSCAN
index scan : no , idx_scan, idx_tup_fetch , <none>
bitmap scan: yes , idx_scan, idx_tup_fetch , SO_TYPE_BITMAPSCAN
sample scan: yes , <none> , seq_tup_read , SO_TYPE_SAMPLESCAN
TID scan : yes , seq_scan, <none> , <none>

bitmap scan and sample scan are historically excluded by corresponding
flags is_bitmapscan and is_samplescan and the commit c3b23ae457 moved
the work to SO_TYPE_* flags. After 147e3722f7, TID scan has the same
characteristics, that is, it calls table_beginscan but doesn't
increment seq_scan. But it doesn't have corresponding flag value.

I'd rather think that whatever calls table_beginscan should have
corresponding SO_TYPE_* flags. (Note: index scan doesn't call it.)

It would be another issue what we should do for the sample scan case.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center