postgres.git / summary / log / commit / refs

commit    f24523672de9c37423800a74cffa7aec27146047
Author:   Tomas Vondra <tomas.vondra@postgresql.org>
Date:     Wed Jun 07 14:48:50 2023 +0000
First released in: 16.0, 12.16, 13.12, 14.9, 15.4

    Use per-tuple context in ExecGetAllUpdatedCols
    
    Commit fc22b6623b (generated columns) replaced ExecGetUpdatedCols() with
    ExecGetAllUpdatedCols() in a couple places handling UPDATE (triggers and
    lock mode). However, ExecGetUpdatedCols() did exec_rt_fetch() while
    ExecGetAllUpdatedCols() also allocates memory through bms_union()
    without paying attention to the memory context and happened to use the
    long-lived ExecutorState, leaking the memory until the end of the query.
    
    The amount of leaked memory is proportional to the number of (updated)
    attributes, types of UPDATE triggers, and the number of processed rows
    (which for UPDATE ... FROM ... may be much higher than updated rows).
    
    Fixed by switching to the per-tuple context in GetAllUpdatedColumns().
    This is fine for all in-core callers, but external callers may need to
    copy the result. But we're not aware of any such callers.
    
    Note the issue was introduced by fc22b6623b, but the macros were later
    renamed by f50e888990.
    
    Backpatch to 12, where the issue was introduced.
    
    Reported-by: Tomas Vondra
    Reviewed-by: Andres Freund, Tom Lane, Jakub Wartak
    Backpatch-through: 12
    Discussion: https://postgr.es/m/222a3442-7f7d-246c-ed9b-a76209d19239@enterprisedb.com


 src/backend/executor/execUtils.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c
index 25aba9a243a..4758ab41321 100644
--- a/src/backend/executor/execUtils.c
+++ b/src/backend/executor/execUtils.c
@@ -1345,12 +1345,26 @@ ExecGetExtraUpdatedCols(ResultRelInfo *relinfo, EState *estate)
 	return relinfo->ri_extraUpdatedCols;
 }
 
-/* Return columns being updated, including generated columns */
+/*
+ * Return columns being updated, including generated columns
+ *
+ * The bitmap is allocated in per-tuple memory context. It's up to the caller to
+ * copy it into a different context with the appropriate lifespan, if needed.
+ */
 Bitmapset *
 ExecGetAllUpdatedCols(ResultRelInfo *relinfo, EState *estate)
 {
-	return bms_union(ExecGetUpdatedCols(relinfo, estate),
-					 ExecGetExtraUpdatedCols(relinfo, estate));
+	Bitmapset	   *ret;
+	MemoryContext	oldcxt;
+
+	oldcxt = MemoryContextSwitchTo(GetPerTupleMemoryContext(estate));
+
+	ret = bms_union(ExecGetUpdatedCols(relinfo, estate),
+					ExecGetExtraUpdatedCols(relinfo, estate));
+
+	MemoryContextSwitchTo(oldcxt);
+
+	return ret;
 }
 
 /*


[parent: fb5a7d84d23f]