Skip to content

Navigation Menu

Sign in

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
Appearance settings

Commit e3fcca0

Browse filesBrowse files
tvondratvondra
committed
Revert changes in HOT handling of BRIN indexes
This reverts commits 5753d4e and fe60b67 that modified HOT to ignore BRIN indexes. The commit message for 5753d4e claims that: When determining whether an index update may be skipped by using HOT, we can ignore attributes indexed only by BRIN indexes. There are no index pointers to individual tuples in BRIN, and the page range summary will be updated anyway as it relies on visibility info. This is partially incorrect - it's true BRIN indexes don't point to individual tuples, so HOT chains are not an issue, but the visibitlity info is not sufficient to keep the index up to date. This can easily result in corrupted indexes, as demonstrated in the hackers thread. This does not mean relaxing the HOT restrictions for BRIN is a lost cause, but it needs to handle the two aspects (allowing HOT chains and updating the page range summaries) as separate. But that requires a major changes, and it's too late for that in the current dev cycle. Reported-by: Tomas Vondra Discussion: https://postgr.es/m/05ebcb44-f383-86e3-4f31-0a97a55634cf@enterprisedb.com
1 parent 664da2a commit e3fcca0
Copy full SHA for e3fcca0

File tree

17 files changed

+27
-502
lines changed
Filter options

17 files changed

+27
-502
lines changed

‎doc/src/sgml/indexam.sgml

Copy file name to clipboardExpand all lines: doc/src/sgml/indexam.sgml
-11
Original file line numberDiff line numberDiff line change
@@ -126,8 +126,6 @@ typedef struct IndexAmRoutine
126126
bool amcaninclude;
127127
/* does AM use maintenance_work_mem? */
128128
bool amusemaintenanceworkmem;
129-
/* does AM block HOT update? */
130-
bool amhotblocking;
131129
/* OR of parallel vacuum flags */
132130
uint8 amparallelvacuumoptions;
133131
/* type of data stored in index, or InvalidOid if variable */
@@ -248,15 +246,6 @@ typedef struct IndexAmRoutine
248246
null, independently of <structfield>amoptionalkey</structfield>.
249247
</para>
250248

251-
<para>
252-
The <structfield>amhotblocking</structfield> flag indicates whether the
253-
access method blocks <acronym>HOT</acronym> when an indexed attribute is
254-
updated. Access methods without pointers to individual tuples (like
255-
<acronym>BRIN</acronym>) may allow <acronym>HOT</acronym> even in this
256-
case. This does not apply to attributes referenced in index predicates;
257-
an update of such attribute always disables <acronym>HOT</acronym>.
258-
</para>
259-
260249
</sect1>
261250

262251
<sect1 id="index-functions">

‎src/backend/access/brin/brin.c

Copy file name to clipboardExpand all lines: src/backend/access/brin/brin.c
-1
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,6 @@ brinhandler(PG_FUNCTION_ARGS)
108108
amroutine->amcanparallel = false;
109109
amroutine->amcaninclude = false;
110110
amroutine->amusemaintenanceworkmem = false;
111-
amroutine->amhotblocking = false;
112111
amroutine->amparallelvacuumoptions =
113112
VACUUM_OPTION_PARALLEL_CLEANUP;
114113
amroutine->amkeytype = InvalidOid;

‎src/backend/access/gin/ginutil.c

Copy file name to clipboardExpand all lines: src/backend/access/gin/ginutil.c
-1
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,6 @@ ginhandler(PG_FUNCTION_ARGS)
5656
amroutine->amcanparallel = false;
5757
amroutine->amcaninclude = false;
5858
amroutine->amusemaintenanceworkmem = true;
59-
amroutine->amhotblocking = true;
6059
amroutine->amparallelvacuumoptions =
6160
VACUUM_OPTION_PARALLEL_BULKDEL | VACUUM_OPTION_PARALLEL_CLEANUP;
6261
amroutine->amkeytype = InvalidOid;

‎src/backend/access/gist/gist.c

Copy file name to clipboardExpand all lines: src/backend/access/gist/gist.c
-1
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,6 @@ gisthandler(PG_FUNCTION_ARGS)
7878
amroutine->amcanparallel = false;
7979
amroutine->amcaninclude = true;
8080
amroutine->amusemaintenanceworkmem = false;
81-
amroutine->amhotblocking = true;
8281
amroutine->amparallelvacuumoptions =
8382
VACUUM_OPTION_PARALLEL_BULKDEL | VACUUM_OPTION_PARALLEL_COND_CLEANUP;
8483
amroutine->amkeytype = InvalidOid;

‎src/backend/access/hash/hash.c

Copy file name to clipboardExpand all lines: src/backend/access/hash/hash.c
-1
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,6 @@ hashhandler(PG_FUNCTION_ARGS)
7575
amroutine->amcanparallel = false;
7676
amroutine->amcaninclude = false;
7777
amroutine->amusemaintenanceworkmem = false;
78-
amroutine->amhotblocking = true;
7978
amroutine->amparallelvacuumoptions =
8079
VACUUM_OPTION_PARALLEL_BULKDEL;
8180
amroutine->amkeytype = INT4OID;

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

Copy file name to clipboardExpand all lines: src/backend/access/heap/heapam.c
+1-1
Original file line numberDiff line numberDiff line change
@@ -3190,7 +3190,7 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup,
31903190
* Note that we get copies of each bitmap, so we need not worry about
31913191
* relcache flush happening midway through.
31923192
*/
3193-
hot_attrs = RelationGetIndexAttrBitmap(relation, INDEX_ATTR_BITMAP_HOT_BLOCKING);
3193+
hot_attrs = RelationGetIndexAttrBitmap(relation, INDEX_ATTR_BITMAP_ALL);
31943194
key_attrs = RelationGetIndexAttrBitmap(relation, INDEX_ATTR_BITMAP_KEY);
31953195
id_attrs = RelationGetIndexAttrBitmap(relation,
31963196
INDEX_ATTR_BITMAP_IDENTITY_KEY);

‎src/backend/access/nbtree/nbtree.c

Copy file name to clipboardExpand all lines: src/backend/access/nbtree/nbtree.c
-1
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,6 @@ bthandler(PG_FUNCTION_ARGS)
114114
amroutine->amcanparallel = true;
115115
amroutine->amcaninclude = true;
116116
amroutine->amusemaintenanceworkmem = false;
117-
amroutine->amhotblocking = true;
118117
amroutine->amparallelvacuumoptions =
119118
VACUUM_OPTION_PARALLEL_BULKDEL | VACUUM_OPTION_PARALLEL_COND_CLEANUP;
120119
amroutine->amkeytype = InvalidOid;

‎src/backend/access/spgist/spgutils.c

Copy file name to clipboardExpand all lines: src/backend/access/spgist/spgutils.c
-1
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,6 @@ spghandler(PG_FUNCTION_ARGS)
6262
amroutine->amcanparallel = false;
6363
amroutine->amcaninclude = true;
6464
amroutine->amusemaintenanceworkmem = false;
65-
amroutine->amhotblocking = true;
6665
amroutine->amparallelvacuumoptions =
6766
VACUUM_OPTION_PARALLEL_BULKDEL | VACUUM_OPTION_PARALLEL_COND_CLEANUP;
6867
amroutine->amkeytype = InvalidOid;

‎src/backend/utils/cache/relcache.c

Copy file name to clipboardExpand all lines: src/backend/utils/cache/relcache.c
+23-30
Original file line numberDiff line numberDiff line change
@@ -2439,10 +2439,10 @@ RelationDestroyRelation(Relation relation, bool remember_tupdesc)
24392439
list_free_deep(relation->rd_fkeylist);
24402440
list_free(relation->rd_indexlist);
24412441
list_free(relation->rd_statlist);
2442+
bms_free(relation->rd_indexattr);
24422443
bms_free(relation->rd_keyattr);
24432444
bms_free(relation->rd_pkattr);
24442445
bms_free(relation->rd_idattr);
2445-
bms_free(relation->rd_hotblockingattr);
24462446
if (relation->rd_pubdesc)
24472447
pfree(relation->rd_pubdesc);
24482448
if (relation->rd_options)
@@ -5104,10 +5104,10 @@ RelationGetIndexPredicate(Relation relation)
51045104
Bitmapset *
51055105
RelationGetIndexAttrBitmap(Relation relation, IndexAttrBitmapKind attrKind)
51065106
{
5107+
Bitmapset *indexattrs; /* indexed columns */
51075108
Bitmapset *uindexattrs; /* columns in unique indexes */
51085109
Bitmapset *pkindexattrs; /* columns in the primary index */
51095110
Bitmapset *idindexattrs; /* columns in the replica identity */
5110-
Bitmapset *hotblockingattrs; /* columns with HOT blocking indexes */
51115111
List *indexoidlist;
51125112
List *newindexoidlist;
51135113
Oid relpkindex;
@@ -5116,18 +5116,18 @@ RelationGetIndexAttrBitmap(Relation relation, IndexAttrBitmapKind attrKind)
51165116
MemoryContext oldcxt;
51175117

51185118
/* Quick exit if we already computed the result. */
5119-
if (relation->rd_attrsvalid)
5119+
if (relation->rd_indexattr != NULL)
51205120
{
51215121
switch (attrKind)
51225122
{
5123+
case INDEX_ATTR_BITMAP_ALL:
5124+
return bms_copy(relation->rd_indexattr);
51235125
case INDEX_ATTR_BITMAP_KEY:
51245126
return bms_copy(relation->rd_keyattr);
51255127
case INDEX_ATTR_BITMAP_PRIMARY_KEY:
51265128
return bms_copy(relation->rd_pkattr);
51275129
case INDEX_ATTR_BITMAP_IDENTITY_KEY:
51285130
return bms_copy(relation->rd_idattr);
5129-
case INDEX_ATTR_BITMAP_HOT_BLOCKING:
5130-
return bms_copy(relation->rd_hotblockingattr);
51315131
default:
51325132
elog(ERROR, "unknown attrKind %u", attrKind);
51335133
}
@@ -5158,7 +5158,7 @@ RelationGetIndexAttrBitmap(Relation relation, IndexAttrBitmapKind attrKind)
51585158
relreplindex = relation->rd_replidindex;
51595159

51605160
/*
5161-
* For each index, add referenced attributes to appropriate bitmaps.
5161+
* For each index, add referenced attributes to indexattrs.
51625162
*
51635163
* Note: we consider all indexes returned by RelationGetIndexList, even if
51645164
* they are not indisready or indisvalid. This is important because an
@@ -5167,10 +5167,10 @@ RelationGetIndexAttrBitmap(Relation relation, IndexAttrBitmapKind attrKind)
51675167
* CONCURRENTLY is far enough along that we should ignore the index, it
51685168
* won't be returned at all by RelationGetIndexList.
51695169
*/
5170+
indexattrs = NULL;
51705171
uindexattrs = NULL;
51715172
pkindexattrs = NULL;
51725173
idindexattrs = NULL;
5173-
hotblockingattrs = NULL;
51745174
foreach(l, indexoidlist)
51755175
{
51765176
Oid indexOid = lfirst_oid(l);
@@ -5235,9 +5235,8 @@ RelationGetIndexAttrBitmap(Relation relation, IndexAttrBitmapKind attrKind)
52355235
*/
52365236
if (attrnum != 0)
52375237
{
5238-
if (indexDesc->rd_indam->amhotblocking)
5239-
hotblockingattrs = bms_add_member(hotblockingattrs,
5240-
attrnum - FirstLowInvalidHeapAttributeNumber);
5238+
indexattrs = bms_add_member(indexattrs,
5239+
attrnum - FirstLowInvalidHeapAttributeNumber);
52415240

52425241
if (isKey && i < indexDesc->rd_index->indnkeyatts)
52435242
uindexattrs = bms_add_member(uindexattrs,
@@ -5254,15 +5253,10 @@ RelationGetIndexAttrBitmap(Relation relation, IndexAttrBitmapKind attrKind)
52545253
}
52555254

52565255
/* Collect all attributes used in expressions, too */
5257-
if (indexDesc->rd_indam->amhotblocking)
5258-
pull_varattnos(indexExpressions, 1, &hotblockingattrs);
5256+
pull_varattnos(indexExpressions, 1, &indexattrs);
52595257

5260-
/*
5261-
* Collect all attributes in the index predicate, too. We have to
5262-
* ignore amhotblocking flag, because the row might become indexable,
5263-
* in which case we have to add it to the index.
5264-
*/
5265-
pull_varattnos(indexPredicate, 1, &hotblockingattrs);
5258+
/* Collect all attributes in the index predicate, too */
5259+
pull_varattnos(indexPredicate, 1, &indexattrs);
52665260

52675261
index_close(indexDesc, AccessShareLock);
52685262
}
@@ -5290,46 +5284,46 @@ RelationGetIndexAttrBitmap(Relation relation, IndexAttrBitmapKind attrKind)
52905284
bms_free(uindexattrs);
52915285
bms_free(pkindexattrs);
52925286
bms_free(idindexattrs);
5293-
bms_free(hotblockingattrs);
5287+
bms_free(indexattrs);
52945288

52955289
goto restart;
52965290
}
52975291

52985292
/* Don't leak the old values of these bitmaps, if any */
5293+
bms_free(relation->rd_indexattr);
5294+
relation->rd_indexattr = NULL;
52995295
bms_free(relation->rd_keyattr);
53005296
relation->rd_keyattr = NULL;
53015297
bms_free(relation->rd_pkattr);
53025298
relation->rd_pkattr = NULL;
53035299
bms_free(relation->rd_idattr);
53045300
relation->rd_idattr = NULL;
5305-
bms_free(relation->rd_hotblockingattr);
5306-
relation->rd_hotblockingattr = NULL;
53075301

53085302
/*
53095303
* Now save copies of the bitmaps in the relcache entry. We intentionally
5310-
* set rd_attrsvalid last, because that's what signals validity of the
5311-
* values; if we run out of memory before making that copy, we won't leave
5312-
* the relcache entry looking like the other ones are valid but empty.
5304+
* set rd_indexattr last, because that's the one that signals validity of
5305+
* the values; if we run out of memory before making that copy, we won't
5306+
* leave the relcache entry looking like the other ones are valid but
5307+
* empty.
53135308
*/
53145309
oldcxt = MemoryContextSwitchTo(CacheMemoryContext);
53155310
relation->rd_keyattr = bms_copy(uindexattrs);
53165311
relation->rd_pkattr = bms_copy(pkindexattrs);
53175312
relation->rd_idattr = bms_copy(idindexattrs);
5318-
relation->rd_hotblockingattr = bms_copy(hotblockingattrs);
5319-
relation->rd_attrsvalid = true;
5313+
relation->rd_indexattr = bms_copy(indexattrs);
53205314
MemoryContextSwitchTo(oldcxt);
53215315

53225316
/* We return our original working copy for caller to play with */
53235317
switch (attrKind)
53245318
{
5319+
case INDEX_ATTR_BITMAP_ALL:
5320+
return indexattrs;
53255321
case INDEX_ATTR_BITMAP_KEY:
53265322
return uindexattrs;
53275323
case INDEX_ATTR_BITMAP_PRIMARY_KEY:
53285324
return pkindexattrs;
53295325
case INDEX_ATTR_BITMAP_IDENTITY_KEY:
53305326
return idindexattrs;
5331-
case INDEX_ATTR_BITMAP_HOT_BLOCKING:
5332-
return hotblockingattrs;
53335327
default:
53345328
elog(ERROR, "unknown attrKind %u", attrKind);
53355329
return NULL;
@@ -6250,11 +6244,10 @@ load_relcache_init_file(bool shared)
62506244
rel->rd_indexlist = NIL;
62516245
rel->rd_pkindex = InvalidOid;
62526246
rel->rd_replidindex = InvalidOid;
6253-
rel->rd_attrsvalid = false;
6247+
rel->rd_indexattr = NULL;
62546248
rel->rd_keyattr = NULL;
62556249
rel->rd_pkattr = NULL;
62566250
rel->rd_idattr = NULL;
6257-
rel->rd_hotblockingattr = NULL;
62586251
rel->rd_pubdesc = NULL;
62596252
rel->rd_statvalid = false;
62606253
rel->rd_statlist = NIL;

‎src/include/access/amapi.h

Copy file name to clipboardExpand all lines: src/include/access/amapi.h
-2
Original file line numberDiff line numberDiff line change
@@ -244,8 +244,6 @@ typedef struct IndexAmRoutine
244244
bool amcaninclude;
245245
/* does AM use maintenance_work_mem? */
246246
bool amusemaintenanceworkmem;
247-
/* does AM block HOT update? */
248-
bool amhotblocking;
249247
/* OR of parallel vacuum flags. See vacuum.h for flags. */
250248
uint8 amparallelvacuumoptions;
251249
/* type of data stored in index, or InvalidOid if variable */

‎src/include/utils/rel.h

Copy file name to clipboardExpand all lines: src/include/utils/rel.h
+1-2
Original file line numberDiff line numberDiff line change
@@ -155,11 +155,10 @@ typedef struct RelationData
155155
List *rd_statlist; /* list of OIDs of extended stats */
156156

157157
/* data managed by RelationGetIndexAttrBitmap: */
158-
bool rd_attrsvalid; /* are bitmaps of attrs valid? */
158+
Bitmapset *rd_indexattr; /* identifies columns used in indexes */
159159
Bitmapset *rd_keyattr; /* cols that can be ref'd by foreign keys */
160160
Bitmapset *rd_pkattr; /* cols included in primary key */
161161
Bitmapset *rd_idattr; /* included in replica identity index */
162-
Bitmapset *rd_hotblockingattr; /* cols blocking HOT update */
163162

164163
PublicationDesc *rd_pubdesc; /* publication descriptor, or NULL */
165164

‎src/include/utils/relcache.h

Copy file name to clipboardExpand all lines: src/include/utils/relcache.h
+2-2
Original file line numberDiff line numberDiff line change
@@ -55,10 +55,10 @@ extern bytea **RelationGetIndexAttOptions(Relation relation, bool copy);
5555

5656
typedef enum IndexAttrBitmapKind
5757
{
58+
INDEX_ATTR_BITMAP_ALL,
5859
INDEX_ATTR_BITMAP_KEY,
5960
INDEX_ATTR_BITMAP_PRIMARY_KEY,
60-
INDEX_ATTR_BITMAP_IDENTITY_KEY,
61-
INDEX_ATTR_BITMAP_HOT_BLOCKING
61+
INDEX_ATTR_BITMAP_IDENTITY_KEY
6262
} IndexAttrBitmapKind;
6363

6464
extern Bitmapset *RelationGetIndexAttrBitmap(Relation relation,

‎src/test/modules/dummy_index_am/dummy_index_am.c

Copy file name to clipboardExpand all lines: src/test/modules/dummy_index_am/dummy_index_am.c
-1
Original file line numberDiff line numberDiff line change
@@ -298,7 +298,6 @@ dihandler(PG_FUNCTION_ARGS)
298298
amroutine->amcanparallel = false;
299299
amroutine->amcaninclude = false;
300300
amroutine->amusemaintenanceworkmem = false;
301-
amroutine->amhotblocking = true;
302301
amroutine->amparallelvacuumoptions = VACUUM_OPTION_NO_PARALLEL;
303302
amroutine->amkeytype = InvalidOid;
304303

‎src/test/regress/expected/brin.out

Copy file name to clipboardExpand all lines: src/test/regress/expected/brin.out
-58
Original file line numberDiff line numberDiff line change
@@ -567,61 +567,3 @@ SELECT * FROM brintest_3 WHERE b < '0';
567567

568568
DROP TABLE brintest_3;
569569
RESET enable_seqscan;
570-
-- Test handling of index predicates - updating attributes in predicates
571-
-- should block HOT even for BRIN. We update a row that was not indexed
572-
-- due to the index predicate, and becomes indexable.
573-
CREATE TABLE brin_hot_2 (a int, b int);
574-
INSERT INTO brin_hot_2 VALUES (1, 100);
575-
CREATE INDEX ON brin_hot_2 USING brin (b) WHERE a = 2;
576-
UPDATE brin_hot_2 SET a = 2;
577-
EXPLAIN (COSTS OFF) SELECT * FROM brin_hot_2 WHERE a = 2 AND b = 100;
578-
QUERY PLAN
579-
-----------------------------------
580-
Seq Scan on brin_hot_2
581-
Filter: ((a = 2) AND (b = 100))
582-
(2 rows)
583-
584-
SELECT COUNT(*) FROM brin_hot_2 WHERE a = 2 AND b = 100;
585-
count
586-
-------
587-
1
588-
(1 row)
589-
590-
SET enable_seqscan = off;
591-
EXPLAIN (COSTS OFF) SELECT * FROM brin_hot_2 WHERE a = 2 AND b = 100;
592-
QUERY PLAN
593-
---------------------------------------------
594-
Bitmap Heap Scan on brin_hot_2
595-
Recheck Cond: ((b = 100) AND (a = 2))
596-
-> Bitmap Index Scan on brin_hot_2_b_idx
597-
Index Cond: (b = 100)
598-
(4 rows)
599-
600-
SELECT COUNT(*) FROM brin_hot_2 WHERE a = 2 AND b = 100;
601-
count
602-
-------
603-
1
604-
(1 row)
605-
606-
-- test BRIN index doesn't block HOT update
607-
CREATE TABLE brin_hot (
608-
id integer PRIMARY KEY,
609-
val integer NOT NULL
610-
) WITH (autovacuum_enabled = off, fillfactor = 70);
611-
INSERT INTO brin_hot SELECT *, 0 FROM generate_series(1, 235);
612-
CREATE INDEX val_brin ON brin_hot using brin(val);
613-
UPDATE brin_hot SET val = -3 WHERE id = 42;
614-
-- ensure pending stats are flushed
615-
SELECT pg_stat_force_next_flush();
616-
pg_stat_force_next_flush
617-
--------------------------
618-
619-
(1 row)
620-
621-
SELECT pg_stat_get_tuples_hot_updated('brin_hot'::regclass::oid);
622-
pg_stat_get_tuples_hot_updated
623-
--------------------------------
624-
1
625-
(1 row)
626-
627-
DROP TABLE brin_hot;

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.