Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitc3b23ae

Browse files
committed
Don't to predicate lock for analyze scans, refactor scan option passing.
Before this commit, when ANALYZE was run on a table and serializablewas used (either by virtue of an explicit BEGIN TRANSACTION ISOLATIONLEVEL SERIALIZABLE, or default_transaction_isolation being set toserializable) a null pointer dereference lead to a crash.The analyze scan doesn't need a snapshot (nor predicate locking), butbefore this commit a scan only contained information about being abitmap or sample scan.Refactor the option passing to the scan_begin callback to use abitmask instead. Alternatively we could have added a new booleanparameter, but that seems harder to read. Even before this issuevarious people (Heikki, Tom, Robert) suggested doing so.These changes don't change the scan APIs outside of tableam. The flagsargument could be exposed, it's not necessary to fix thisproblem. Also the wrapper table_beginscan* functions encapsulate mostof that complexity.After these changes fixing the bug is trivial, just don't acquirepredicate lock for analyze style scans. That was already done forbitmap heap scans. Add an assert that a snapshot is passed whenacquiring the predicate lock, so this kind of bug doesn't requirerunning with serializable.Also add a comment about sample scans currently requiring predicatelocking the entire relation, that previously wasn't remarked upon.Reported-By: Joe WildishAuthor: Andres FreundDiscussion:https://postgr.es/m/4EA80A20-E9BF-49F1-9F01-5B66CAB21453@elusive.cxhttps://postgr.es/m/20190411164947.nkii4gaeilt4bui7@alap3.anarazel.dehttps://postgr.es/m/20190518203102.g7peu2fianukjuxm@alap3.anarazel.de
1 parentbd1592e commitc3b23ae

File tree

8 files changed

+160
-102
lines changed

8 files changed

+160
-102
lines changed

‎src/backend/access/heap/heapam.c

Lines changed: 69 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -245,8 +245,8 @@ initscan(HeapScanDesc scan, ScanKey key, bool keep_startblock)
245245
if (!RelationUsesLocalBuffers(scan->rs_base.rs_rd)&&
246246
scan->rs_nblocks>NBuffers /4)
247247
{
248-
allow_strat=scan->rs_base.rs_allow_strat;
249-
allow_sync=scan->rs_base.rs_allow_sync;
248+
allow_strat=(scan->rs_base.rs_flags&SO_ALLOW_STRAT)!=0;
249+
allow_sync=(scan->rs_base.rs_flags&SO_ALLOW_SYNC)!=0;
250250
}
251251
else
252252
allow_strat=allow_sync= false;
@@ -267,7 +267,10 @@ initscan(HeapScanDesc scan, ScanKey key, bool keep_startblock)
267267
if (scan->rs_base.rs_parallel!=NULL)
268268
{
269269
/* For parallel scan, believe whatever ParallelTableScanDesc says. */
270-
scan->rs_base.rs_syncscan=scan->rs_base.rs_parallel->phs_syncscan;
270+
if (scan->rs_base.rs_parallel->phs_syncscan)
271+
scan->rs_base.rs_flags |=SO_ALLOW_SYNC;
272+
else
273+
scan->rs_base.rs_flags &= ~SO_ALLOW_SYNC;
271274
}
272275
elseif (keep_startblock)
273276
{
@@ -276,16 +279,19 @@ initscan(HeapScanDesc scan, ScanKey key, bool keep_startblock)
276279
* so that rewinding a cursor doesn't generate surprising results.
277280
* Reset the active syncscan setting, though.
278281
*/
279-
scan->rs_base.rs_syncscan= (allow_sync&&synchronize_seqscans);
282+
if (allow_sync&&synchronize_seqscans)
283+
scan->rs_base.rs_flags |=SO_ALLOW_SYNC;
284+
else
285+
scan->rs_base.rs_flags &= ~SO_ALLOW_SYNC;
280286
}
281287
elseif (allow_sync&&synchronize_seqscans)
282288
{
283-
scan->rs_base.rs_syncscan= true;
289+
scan->rs_base.rs_flags |=SO_ALLOW_SYNC;
284290
scan->rs_startblock=ss_get_location(scan->rs_base.rs_rd,scan->rs_nblocks);
285291
}
286292
else
287293
{
288-
scan->rs_base.rs_syncscan= false;
294+
scan->rs_base.rs_flags &= ~SO_ALLOW_SYNC;
289295
scan->rs_startblock=0;
290296
}
291297

@@ -305,11 +311,11 @@ initscan(HeapScanDesc scan, ScanKey key, bool keep_startblock)
305311
memcpy(scan->rs_base.rs_key,key,scan->rs_base.rs_nkeys*sizeof(ScanKeyData));
306312

307313
/*
308-
* Currently, wedon't have a stats counter forbitmap heap scans (but the
309-
* underlying bitmap index scans will be counted) or sample scans (we only
310-
* update stats for tuple fetches there)
314+
* Currently, weonly have a stats counter forsequential heap scans (but
315+
*e.g for bitmap scans theunderlying bitmap index scans will be counted,
316+
*and for sample scans weupdate stats for tuple fetches).
311317
*/
312-
if (!scan->rs_base.rs_bitmapscan&& !scan->rs_base.rs_samplescan)
318+
if (scan->rs_base.rs_flags&SO_TYPE_SEQSCAN)
313319
pgstat_count_heap_scan(scan->rs_base.rs_rd);
314320
}
315321

@@ -325,7 +331,8 @@ heap_setscanlimits(TableScanDesc sscan, BlockNumber startBlk, BlockNumber numBlk
325331
HeapScanDescscan= (HeapScanDesc)sscan;
326332

327333
Assert(!scan->rs_inited);/* else too late to change */
328-
Assert(!scan->rs_base.rs_syncscan);/* else rs_startblock is significant */
334+
/* else rs_startblock is significant */
335+
Assert(!(scan->rs_base.rs_flags&SO_ALLOW_SYNC));
329336

330337
/* Check startBlk is valid (but allow case of zero blocks...) */
331338
Assert(startBlk==0||startBlk<scan->rs_nblocks);
@@ -375,7 +382,7 @@ heapgetpage(TableScanDesc sscan, BlockNumber page)
375382
RBM_NORMAL,scan->rs_strategy);
376383
scan->rs_cblock=page;
377384

378-
if (!scan->rs_base.rs_pageatatime)
385+
if (!(scan->rs_base.rs_flags&SO_ALLOW_PAGEMODE))
379386
return;
380387

381388
buffer=scan->rs_cbuf;
@@ -574,7 +581,7 @@ heapgettup(HeapScanDesc scan,
574581
* time, and much more likely that we'll just bollix things for
575582
* forward scanners.
576583
*/
577-
scan->rs_base.rs_syncscan= false;
584+
scan->rs_base.rs_flags &= ~SO_ALLOW_SYNC;
578585
/* start from last page of the scan */
579586
if (scan->rs_startblock>0)
580587
page=scan->rs_startblock-1;
@@ -738,7 +745,7 @@ heapgettup(HeapScanDesc scan,
738745
* a little bit backwards on every invocation, which is confusing.
739746
* We don't guarantee any specific ordering in general, though.
740747
*/
741-
if (scan->rs_base.rs_syncscan)
748+
if (scan->rs_base.rs_flags&SO_ALLOW_SYNC)
742749
ss_report_location(scan->rs_base.rs_rd,page);
743750
}
744751

@@ -885,7 +892,7 @@ heapgettup_pagemode(HeapScanDesc scan,
885892
* time, and much more likely that we'll just bollix things for
886893
* forward scanners.
887894
*/
888-
scan->rs_base.rs_syncscan= false;
895+
scan->rs_base.rs_flags &= ~SO_ALLOW_SYNC;
889896
/* start from last page of the scan */
890897
if (scan->rs_startblock>0)
891898
page=scan->rs_startblock-1;
@@ -1037,7 +1044,7 @@ heapgettup_pagemode(HeapScanDesc scan,
10371044
* a little bit backwards on every invocation, which is confusing.
10381045
* We don't guarantee any specific ordering in general, though.
10391046
*/
1040-
if (scan->rs_base.rs_syncscan)
1047+
if (scan->rs_base.rs_flags&SO_ALLOW_SYNC)
10411048
ss_report_location(scan->rs_base.rs_rd,page);
10421049
}
10431050

@@ -1125,12 +1132,7 @@ TableScanDesc
11251132
heap_beginscan(Relationrelation,Snapshotsnapshot,
11261133
intnkeys,ScanKeykey,
11271134
ParallelTableScanDescparallel_scan,
1128-
boolallow_strat,
1129-
boolallow_sync,
1130-
boolallow_pagemode,
1131-
boolis_bitmapscan,
1132-
boolis_samplescan,
1133-
booltemp_snap)
1135+
uint32flags)
11341136
{
11351137
HeapScanDescscan;
11361138

@@ -1151,33 +1153,39 @@ heap_beginscan(Relation relation, Snapshot snapshot,
11511153
scan->rs_base.rs_rd=relation;
11521154
scan->rs_base.rs_snapshot=snapshot;
11531155
scan->rs_base.rs_nkeys=nkeys;
1154-
scan->rs_base.rs_bitmapscan=is_bitmapscan;
1155-
scan->rs_base.rs_samplescan=is_samplescan;
1156-
scan->rs_strategy=NULL;/* set in initscan */
1157-
scan->rs_base.rs_allow_strat=allow_strat;
1158-
scan->rs_base.rs_allow_sync=allow_sync;
1159-
scan->rs_base.rs_temp_snap=temp_snap;
1156+
scan->rs_base.rs_flags=flags;
11601157
scan->rs_base.rs_parallel=parallel_scan;
1158+
scan->rs_strategy=NULL;/* set in initscan */
11611159

11621160
/*
1163-
*we can usepage-at-a-time mode if it'sanMVCC-safe snapshot
1161+
*Disablepage-at-a-time mode if it'snot aMVCC-safe snapshot.
11641162
*/
1165-
scan->rs_base.rs_pageatatime=
1166-
allow_pagemode&&snapshot&&IsMVCCSnapshot(snapshot);
1163+
if (!(snapshot&&IsMVCCSnapshot(snapshot)))
1164+
scan->rs_base.rs_flags &= ~SO_ALLOW_PAGEMODE;
11671165

11681166
/*
1169-
* For a seqscan in a serializable transaction, acquire a predicate lock
1170-
* on the entire relation. This is required not only to lock all the
1171-
* matching tuples, but also to conflict with new insertions into the
1172-
* table. In an indexscan, we take page locks on the index pages covering
1173-
* the range specified in the scan qual, but in a heap scan there is
1174-
* nothing more fine-grained to lock. A bitmap scan is a different story,
1175-
* there we have already scanned the index and locked the index pages
1176-
* covering the predicate. But in that case we still have to lock any
1177-
* matching heap tuples.
1167+
* For seqscan and sample scans in a serializable transaction, acquire a
1168+
* predicate lock on the entire relation. This is required not only to
1169+
* lock all the matching tuples, but also to conflict with new insertions
1170+
* into the table. In an indexscan, we take page locks on the index pages
1171+
* covering the range specified in the scan qual, but in a heap scan there
1172+
* is nothing more fine-grained to lock. A bitmap scan is a different
1173+
* story, there we have already scanned the index and locked the index
1174+
* pages covering the predicate. But in that case we still have to lock
1175+
* any matching heap tuples. For sample scan we could optimize the locking
1176+
* to be at least page-level granularity, but we'd need to add per-tuple
1177+
* locking for that.
11781178
*/
1179-
if (!is_bitmapscan)
1179+
if (scan->rs_base.rs_flags& (SO_TYPE_SEQSCAN |SO_TYPE_SAMPLESCAN))
1180+
{
1181+
/*
1182+
* Ensure a missing snapshot is noticed reliably, even if the
1183+
* isolation mode means predicate locking isn't performed (and
1184+
* therefore the snapshot isn't used here).
1185+
*/
1186+
Assert(snapshot);
11801187
PredicateLockRelation(relation,snapshot);
1188+
}
11811189

11821190
/* we only need to set this up once */
11831191
scan->rs_ctup.t_tableOid=RelationGetRelid(relation);
@@ -1204,10 +1212,21 @@ heap_rescan(TableScanDesc sscan, ScanKey key, bool set_params,
12041212

12051213
if (set_params)
12061214
{
1207-
scan->rs_base.rs_allow_strat=allow_strat;
1208-
scan->rs_base.rs_allow_sync=allow_sync;
1209-
scan->rs_base.rs_pageatatime=
1210-
allow_pagemode&&IsMVCCSnapshot(scan->rs_base.rs_snapshot);
1215+
if (allow_strat)
1216+
scan->rs_base.rs_flags |=SO_ALLOW_STRAT;
1217+
else
1218+
scan->rs_base.rs_flags &= ~SO_ALLOW_STRAT;
1219+
1220+
if (allow_sync)
1221+
scan->rs_base.rs_flags |=SO_ALLOW_SYNC;
1222+
else
1223+
scan->rs_base.rs_flags &= ~SO_ALLOW_SYNC;
1224+
1225+
if (allow_pagemode&&scan->rs_base.rs_snapshot&&
1226+
IsMVCCSnapshot(scan->rs_base.rs_snapshot))
1227+
scan->rs_base.rs_flags |=SO_ALLOW_PAGEMODE;
1228+
else
1229+
scan->rs_base.rs_flags &= ~SO_ALLOW_PAGEMODE;
12111230
}
12121231

12131232
/*
@@ -1246,7 +1265,7 @@ heap_endscan(TableScanDesc sscan)
12461265
if (scan->rs_strategy!=NULL)
12471266
FreeAccessStrategy(scan->rs_strategy);
12481267

1249-
if (scan->rs_base.rs_temp_snap)
1268+
if (scan->rs_base.rs_flags&SO_TEMP_SNAPSHOT)
12501269
UnregisterSnapshot(scan->rs_base.rs_snapshot);
12511270

12521271
pfree(scan);
@@ -1288,7 +1307,7 @@ heap_getnext(TableScanDesc sscan, ScanDirection direction)
12881307

12891308
HEAPDEBUG_1;/* heap_getnext( info ) */
12901309

1291-
if (scan->rs_base.rs_pageatatime)
1310+
if (scan->rs_base.rs_flags&SO_ALLOW_PAGEMODE)
12921311
heapgettup_pagemode(scan,direction,
12931312
scan->rs_base.rs_nkeys,scan->rs_base.rs_key);
12941313
else
@@ -1335,11 +1354,10 @@ heap_getnextslot(TableScanDesc sscan, ScanDirection direction, TupleTableSlot *s
13351354

13361355
HEAPAMSLOTDEBUG_1;/* heap_getnextslot( info ) */
13371356

1338-
if (scan->rs_base.rs_pageatatime)
1339-
heapgettup_pagemode(scan,direction,
1340-
scan->rs_base.rs_nkeys,scan->rs_base.rs_key);
1357+
if (sscan->rs_flags&SO_ALLOW_PAGEMODE)
1358+
heapgettup_pagemode(scan,direction,sscan->rs_nkeys,sscan->rs_key);
13411359
else
1342-
heapgettup(scan,direction,scan->rs_base.rs_nkeys,scan->rs_base.rs_key);
1360+
heapgettup(scan,direction,sscan->rs_nkeys,sscan->rs_key);
13431361

13441362
if (scan->rs_ctup.t_data==NULL)
13451363
{

‎src/backend/access/heap/heapam_handler.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2323,7 +2323,7 @@ heapam_scan_sample_next_block(TableScanDesc scan, SampleScanState *scanstate)
23232323
* a little bit backwards on every invocation, which is confusing.
23242324
* We don't guarantee any specific ordering in general, though.
23252325
*/
2326-
if (scan->rs_syncscan)
2326+
if (scan->rs_flags&SO_ALLOW_SYNC)
23272327
ss_report_location(scan->rs_rd,blockno);
23282328

23292329
if (blockno==hscan->rs_startblock)
@@ -2357,7 +2357,7 @@ heapam_scan_sample_next_tuple(TableScanDesc scan, SampleScanState *scanstate,
23572357
HeapScanDeschscan= (HeapScanDesc)scan;
23582358
TsmRoutine*tsm=scanstate->tsmroutine;
23592359
BlockNumberblockno=hscan->rs_cblock;
2360-
boolpagemode=scan->rs_pageatatime;
2360+
boolpagemode=(scan->rs_flags&SO_ALLOW_PAGEMODE)!=0;
23612361

23622362
Pagepage;
23632363
boolall_visible;
@@ -2504,7 +2504,7 @@ SampleHeapTupleVisible(TableScanDesc scan, Buffer buffer,
25042504
{
25052505
HeapScanDeschscan= (HeapScanDesc)scan;
25062506

2507-
if (scan->rs_pageatatime)
2507+
if (scan->rs_flags&SO_ALLOW_PAGEMODE)
25082508
{
25092509
/*
25102510
* In pageatatime mode, heapgetpage() already did visibility checks,

‎src/backend/access/table/tableam.c

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -93,12 +93,13 @@ table_slot_create(Relation relation, List **reglist)
9393
TableScanDesc
9494
table_beginscan_catalog(Relationrelation,intnkeys,structScanKeyData*key)
9595
{
96+
uint32flags=SO_TYPE_SEQSCAN |
97+
SO_ALLOW_STRAT |SO_ALLOW_SYNC |SO_ALLOW_PAGEMODE |SO_TEMP_SNAPSHOT;
9698
Oidrelid=RelationGetRelid(relation);
9799
Snapshotsnapshot=RegisterSnapshot(GetCatalogSnapshot(relid));
98100

99101
returnrelation->rd_tableam->scan_begin(relation,snapshot,nkeys,key,
100-
NULL, true, true, true, false,
101-
false, true);
102+
NULL,flags);
102103
}
103104

104105
void
@@ -108,7 +109,7 @@ table_scan_update_snapshot(TableScanDesc scan, Snapshot snapshot)
108109

109110
RegisterSnapshot(snapshot);
110111
scan->rs_snapshot=snapshot;
111-
scan->rs_temp_snap= true;
112+
scan->rs_flags |=SO_TEMP_SNAPSHOT;
112113
}
113114

114115

@@ -156,6 +157,8 @@ TableScanDesc
156157
table_beginscan_parallel(Relationrelation,ParallelTableScanDescparallel_scan)
157158
{
158159
Snapshotsnapshot;
160+
uint32flags=SO_TYPE_SEQSCAN |
161+
SO_ALLOW_STRAT |SO_ALLOW_SYNC |SO_ALLOW_PAGEMODE;
159162

160163
Assert(RelationGetRelid(relation)==parallel_scan->phs_relid);
161164

@@ -165,6 +168,7 @@ table_beginscan_parallel(Relation relation, ParallelTableScanDesc parallel_scan)
165168
snapshot=RestoreSnapshot((char*)parallel_scan+
166169
parallel_scan->phs_snapshot_off);
167170
RegisterSnapshot(snapshot);
171+
flags |=SO_TEMP_SNAPSHOT;
168172
}
169173
else
170174
{
@@ -173,9 +177,7 @@ table_beginscan_parallel(Relation relation, ParallelTableScanDesc parallel_scan)
173177
}
174178

175179
returnrelation->rd_tableam->scan_begin(relation,snapshot,0,NULL,
176-
parallel_scan, true, true, true,
177-
false, false,
178-
!parallel_scan->phs_snapshot_any);
180+
parallel_scan,flags);
179181
}
180182

181183

‎src/include/access/heapam.h

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -110,12 +110,7 @@ typedef enum
110110
externTableScanDescheap_beginscan(Relationrelation,Snapshotsnapshot,
111111
intnkeys,ScanKeykey,
112112
ParallelTableScanDescparallel_scan,
113-
boolallow_strat,
114-
boolallow_sync,
115-
boolallow_pagemode,
116-
boolis_bitmapscan,
117-
boolis_samplescan,
118-
booltemp_snap);
113+
uint32flags);
119114
externvoidheap_setscanlimits(TableScanDescscan,BlockNumberstartBlk,
120115
BlockNumberendBlk);
121116
externvoidheapgetpage(TableScanDescscan,BlockNumberpage);

‎src/include/access/relscan.h

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -35,13 +35,12 @@ typedef struct TableScanDescData
3535
structSnapshotData*rs_snapshot;/* snapshot to see */
3636
intrs_nkeys;/* number of scan keys */
3737
structScanKeyData*rs_key;/* array of scan key descriptors */
38-
boolrs_bitmapscan;/* true if this is really a bitmap scan */
39-
boolrs_samplescan;/* true if this is really a sample scan */
40-
boolrs_pageatatime;/* verify visibility page-at-a-time? */
41-
boolrs_allow_strat;/* allow or disallow use of access strategy */
42-
boolrs_allow_sync;/* allow or disallow use of syncscan */
43-
boolrs_temp_snap;/* unregister snapshot at scan end? */
44-
boolrs_syncscan;/* report location to syncscan logic? */
38+
39+
/*
40+
* Information about type and behaviour of the scan, a bitmask of members
41+
* of the ScanOptions enum (see tableam.h).
42+
*/
43+
uint32rs_flags;
4544

4645
structParallelTableScanDescData*rs_parallel;/* parallel scan
4746
* information */

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp