Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit cb5b286

Browse files
committed
Fix bug in Tid scan.
Commit 147e372 changed Tid scan so that it calls table_beginscan() and uses the scan option for seq scan. This change caused two issues. (1) The change caused Tid scan to take a predicate lock on the entire relation in serializable transaction even when relation-level lock is not necessary. This could lead to an unexpected serialization error. (2) The change caused Tid scan to increment the number of seq_scan in pg_stat_*_tables views even though it's not seq scan. This could confuse the users. This commit adds the scan option for Tid scan and makes Tid scan use it, to avoid those issues. Back-patch to v12, where the bug was introduced. Author: Tatsuhito Kasahara Reviewed-by: Kyotaro Horiguchi, Masahiko Sawada, Fujii Masao Discussion: https://postgr.es/m/CAP0=ZVKy+gTbFmB6X_UW0pP3WaeJ-fkUWHoD-pExS=at3CY76g@mail.gmail.com
1 parent b059d2f commit cb5b286

File tree

5 files changed

+46
-10
lines changed

5 files changed

+46
-10
lines changed

src/backend/executor/nodeTidscan.c

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -143,9 +143,8 @@ TidListEval(TidScanState *tidstate)
143143
*/
144144
if (tidstate->ss.ss_currentScanDesc == NULL)
145145
tidstate->ss.ss_currentScanDesc =
146-
table_beginscan(tidstate->ss.ss_currentRelation,
147-
tidstate->ss.ps.state->es_snapshot,
148-
0, NULL);
146+
table_beginscan_tid(tidstate->ss.ss_currentRelation,
147+
tidstate->ss.ps.state->es_snapshot);
149148
scan = tidstate->ss.ss_currentScanDesc;
150149

151150
/*

src/backend/utils/adt/tid.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -381,7 +381,7 @@ currtid_byreloid(PG_FUNCTION_ARGS)
381381
ItemPointerCopy(tid, result);
382382

383383
snapshot = RegisterSnapshot(GetLatestSnapshot());
384-
scan = table_beginscan(rel, snapshot, 0, NULL);
384+
scan = table_beginscan_tid(rel, snapshot);
385385
table_tuple_get_latest_tid(scan, result);
386386
table_endscan(scan);
387387
UnregisterSnapshot(snapshot);
@@ -419,7 +419,7 @@ currtid_byrelname(PG_FUNCTION_ARGS)
419419
ItemPointerCopy(tid, result);
420420

421421
snapshot = RegisterSnapshot(GetLatestSnapshot());
422-
scan = table_beginscan(rel, snapshot, 0, NULL);
422+
scan = table_beginscan_tid(rel, snapshot);
423423
table_tuple_get_latest_tid(scan, result);
424424
table_endscan(scan);
425425
UnregisterSnapshot(snapshot);

src/include/access/tableam.h

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -47,18 +47,19 @@ typedef enum ScanOptions
4747
SO_TYPE_SEQSCAN = 1 << 0,
4848
SO_TYPE_BITMAPSCAN = 1 << 1,
4949
SO_TYPE_SAMPLESCAN = 1 << 2,
50-
SO_TYPE_ANALYZE = 1 << 3,
50+
SO_TYPE_TIDSCAN = 1 << 3,
51+
SO_TYPE_ANALYZE = 1 << 4,
5152

5253
/* several of SO_ALLOW_* may be specified */
5354
/* allow or disallow use of access strategy */
54-
SO_ALLOW_STRAT = 1 << 4,
55+
SO_ALLOW_STRAT = 1 << 5,
5556
/* report location to syncscan logic? */
56-
SO_ALLOW_SYNC = 1 << 5,
57+
SO_ALLOW_SYNC = 1 << 6,
5758
/* verify visibility page-at-a-time? */
58-
SO_ALLOW_PAGEMODE = 1 << 6,
59+
SO_ALLOW_PAGEMODE = 1 << 7,
5960

6061
/* unregister snapshot at scan end? */
61-
SO_TEMP_SNAPSHOT = 1 << 7
62+
SO_TEMP_SNAPSHOT = 1 << 8
6263
} ScanOptions;
6364

6465
/*
@@ -829,6 +830,19 @@ table_beginscan_sampling(Relation rel, Snapshot snapshot,
829830
return rel->rd_tableam->scan_begin(rel, snapshot, nkeys, key, NULL, flags);
830831
}
831832

833+
/*
834+
* table_beginscan_tid is an alternative entry point for setting up a
835+
* TableScanDesc for a Tid scan. As with bitmap scans, it's worth using
836+
* the same data structure although the behavior is rather different.
837+
*/
838+
static inline TableScanDesc
839+
table_beginscan_tid(Relation rel, Snapshot snapshot)
840+
{
841+
uint32 flags = SO_TYPE_TIDSCAN;
842+
843+
return rel->rd_tableam->scan_begin(rel, snapshot, 0, NULL, NULL, flags);
844+
}
845+
832846
/*
833847
* table_beginscan_analyze is an alternative entry point for setting up a
834848
* TableScanDesc for an ANALYZE scan. As with bitmap scans, it's worth using

src/test/regress/expected/tidscan.out

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -277,4 +277,20 @@ SELECT count(*) FROM tenk1 t1 JOIN tenk1 t2 ON t1.ctid = t2.ctid;
277277
(1 row)
278278

279279
RESET enable_hashjoin;
280+
-- check predicate lock on CTID
281+
BEGIN ISOLATION LEVEL SERIALIZABLE;
282+
SELECT * FROM tidscan WHERE ctid = '(0,1)';
283+
id
284+
----
285+
1
286+
(1 row)
287+
288+
-- locktype should be 'tuple'
289+
SELECT locktype, mode FROM pg_locks WHERE pid = pg_backend_pid() AND mode = 'SIReadLock';
290+
locktype | mode
291+
----------+------------
292+
tuple | SIReadLock
293+
(1 row)
294+
295+
ROLLBACK;
280296
DROP TABLE tidscan;

src/test/regress/sql/tidscan.sql

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,4 +94,11 @@ SELECT count(*) FROM tenk1 t1 JOIN tenk1 t2 ON t1.ctid = t2.ctid;
9494
SELECT count(*) FROM tenk1 t1 JOIN tenk1 t2 ON t1.ctid = t2.ctid;
9595
RESET enable_hashjoin;
9696

97+
-- check predicate lock on CTID
98+
BEGIN ISOLATION LEVEL SERIALIZABLE;
99+
SELECT * FROM tidscan WHERE ctid = '(0,1)';
100+
-- locktype should be 'tuple'
101+
SELECT locktype, mode FROM pg_locks WHERE pid = pg_backend_pid() AND mode = 'SIReadLock';
102+
ROLLBACK;
103+
97104
DROP TABLE tidscan;

0 commit comments

Comments
 (0)