On Thu, Jan 30, 2020 at 1:49 PM Kyotaro Horiguchi <horikyota....@gmail.com> wrote: > At Thu, 30 Jan 2020 13:30:56 +0900, Kasahara Tatsuhito > <kasahara.tatsuh...@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
fix_tidscan_increments_seqscan_num_v2.patch
Description: Binary data