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

Commit41ee68a

Browse files
Fix memory leak in indexUnchanged hint mechanism.
Commit9dc718b added a "logically unchanged by UPDATE" hintingmechanism, which is currently used within nbtree indexes only (seecommitd168b66). This mechanism determined whether or not the incomingitem is a logically unchanged duplicate (a duplicate needed only forMVCC versioning purposes) once per row updated per non-HOT update. Thisapproach led to memory leaks which were noticeable with an UPDATEstatement that updated sufficiently many rows, at least on tables thathappen to have an expression index.On HEAD, fix the issue by adding a cache to the executor's per-indexIndexInfo struct.Take a different approach on Postgres 14 to avoid an ABI break: simplypass down the hint to all indexes unconditionally with non-HOT UPDATEs.This is deemed acceptable because the hint is currently interpretedwithin btinsert() as "perform a bottom-up index deletion pass if andwhen the only alternative is splitting the leaf page -- prefer to deleteany LP_DEAD-set items first". nbtree must always treat the hint as anoisy signal about what might work, as a strategy of last resort, withcosts imposed on non-HOT updaters. (The same thing might not be truewithin another index AM that applies the hint, which is why the originalbehavior is preserved on HEAD.)Author: Peter Geoghegan <pg@bowt.ie>Reported-By: Klaudie Willis <Klaudie.Willis@protonmail.com>Diagnosed-By: Tom Lane <tgl@sss.pgh.pa.us>Discussion:https://postgr.es/m/261065.1639497535@sss.pgh.pa.usBackpatch: 14-, where the hinting mechanism was added.
1 parentaf8d530 commit41ee68a

File tree

1 file changed

+5
-128
lines changed

1 file changed

+5
-128
lines changed

‎src/backend/executor/execIndexing.c

Lines changed: 5 additions & 128 deletions
Original file line numberDiff line numberDiff line change
@@ -136,11 +136,6 @@ static bool check_exclusion_or_unique_constraint(Relation heap, Relation index,
136136
staticboolindex_recheck_constraint(Relationindex,Oid*constr_procs,
137137
Datum*existing_values,bool*existing_isnull,
138138
Datum*new_values);
139-
staticboolindex_unchanged_by_update(ResultRelInfo*resultRelInfo,
140-
EState*estate,IndexInfo*indexInfo,
141-
RelationindexRelation);
142-
staticboolindex_expression_changed_walker(Node*node,
143-
Bitmapset*allUpdatedCols);
144139

145140
/* ----------------------------------------------------------------
146141
*ExecOpenIndices
@@ -406,11 +401,12 @@ ExecInsertIndexTuples(ResultRelInfo *resultRelInfo,
406401
* There's definitely going to be an index_insert() call for this
407402
* index. If we're being called as part of an UPDATE statement,
408403
* consider if the 'indexUnchanged' = true hint should be passed.
404+
*
405+
* XXX We always assume that the hint should be passed for an UPDATE.
406+
* This is a workaround for a bug in PostgreSQL 14. In practice this
407+
* won't make much difference for current users of the hint.
409408
*/
410-
indexUnchanged=update&&index_unchanged_by_update(resultRelInfo,
411-
estate,
412-
indexInfo,
413-
indexRelation);
409+
indexUnchanged=update;
414410

415411
satisfiesConstraint=
416412
index_insert(indexRelation,/* index relation */
@@ -923,122 +919,3 @@ index_recheck_constraint(Relation index, Oid *constr_procs,
923919

924920
return true;
925921
}
926-
927-
/*
928-
* Check if ExecInsertIndexTuples() should pass indexUnchanged hint.
929-
*
930-
* When the executor performs an UPDATE that requires a new round of index
931-
* tuples, determine if we should pass 'indexUnchanged' = true hint for one
932-
* single index.
933-
*/
934-
staticbool
935-
index_unchanged_by_update(ResultRelInfo*resultRelInfo,EState*estate,
936-
IndexInfo*indexInfo,RelationindexRelation)
937-
{
938-
Bitmapset*updatedCols=ExecGetUpdatedCols(resultRelInfo,estate);
939-
Bitmapset*extraUpdatedCols=ExecGetExtraUpdatedCols(resultRelInfo,estate);
940-
Bitmapset*allUpdatedCols;
941-
boolhasexpression= false;
942-
List*idxExprs;
943-
944-
/*
945-
* Check for indexed attribute overlap with updated columns.
946-
*
947-
* Only do this for key columns. A change to a non-key column within an
948-
* INCLUDE index should not be counted here. Non-key column values are
949-
* opaque payload state to the index AM, a little like an extra table TID.
950-
*/
951-
for (intattr=0;attr<indexInfo->ii_NumIndexKeyAttrs;attr++)
952-
{
953-
intkeycol=indexInfo->ii_IndexAttrNumbers[attr];
954-
955-
if (keycol <=0)
956-
{
957-
/*
958-
* Skip expressions for now, but remember to deal with them later
959-
* on
960-
*/
961-
hasexpression= true;
962-
continue;
963-
}
964-
965-
if (bms_is_member(keycol-FirstLowInvalidHeapAttributeNumber,
966-
updatedCols)||
967-
bms_is_member(keycol-FirstLowInvalidHeapAttributeNumber,
968-
extraUpdatedCols))
969-
{
970-
/* Changed key column -- don't hint for this index */
971-
return false;
972-
}
973-
}
974-
975-
/*
976-
* When we get this far and index has no expressions, return true so that
977-
* index_insert() call will go on to pass 'indexUnchanged' = true hint.
978-
*
979-
* The _absence_ of an indexed key attribute that overlaps with updated
980-
* attributes (in addition to the total absence of indexed expressions)
981-
* shows that the index as a whole is logically unchanged by UPDATE.
982-
*/
983-
if (!hasexpression)
984-
return true;
985-
986-
/*
987-
* Need to pass only one bms to expression_tree_walker helper function.
988-
* Avoid allocating memory in common case where there are no extra cols.
989-
*/
990-
if (!extraUpdatedCols)
991-
allUpdatedCols=updatedCols;
992-
else
993-
allUpdatedCols=bms_union(updatedCols,extraUpdatedCols);
994-
995-
/*
996-
* We have to work slightly harder in the event of indexed expressions,
997-
* but the principle is the same as before: try to find columns (Vars,
998-
* actually) that overlap with known-updated columns.
999-
*
1000-
* If we find any matching Vars, don't pass hint for index. Otherwise
1001-
* pass hint.
1002-
*/
1003-
idxExprs=RelationGetIndexExpressions(indexRelation);
1004-
hasexpression=index_expression_changed_walker((Node*)idxExprs,
1005-
allUpdatedCols);
1006-
list_free(idxExprs);
1007-
if (extraUpdatedCols)
1008-
bms_free(allUpdatedCols);
1009-
1010-
if (hasexpression)
1011-
return false;
1012-
1013-
return true;
1014-
}
1015-
1016-
/*
1017-
* Indexed expression helper for index_unchanged_by_update().
1018-
*
1019-
* Returns true when Var that appears within allUpdatedCols located.
1020-
*/
1021-
staticbool
1022-
index_expression_changed_walker(Node*node,Bitmapset*allUpdatedCols)
1023-
{
1024-
if (node==NULL)
1025-
return false;
1026-
1027-
if (IsA(node,Var))
1028-
{
1029-
Var*var= (Var*)node;
1030-
1031-
if (bms_is_member(var->varattno-FirstLowInvalidHeapAttributeNumber,
1032-
allUpdatedCols))
1033-
{
1034-
/* Var was updated -- indicates that we should not hint */
1035-
return true;
1036-
}
1037-
1038-
/* Still haven't found a reason to not pass the hint */
1039-
return false;
1040-
}
1041-
1042-
returnexpression_tree_walker(node,index_expression_changed_walker,
1043-
(void*)allUpdatedCols);
1044-
}

0 commit comments

Comments
 (0)

[8]ページ先頭

©2009-2025 Movatter.jp