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

Hello.

At Wed, 29 Jan 2020 23:24:09 +0900, Fujii Masao <masao.fujii@oss.nttdata.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