I looked at this issue a bit. It seems to me what happens is that at
first the memory areas referenced by probeslot->tts_values[] are
allocated in the per tuple context (see prepare_probe_slot). And then
in MemoizeHash_hash, after we've calculated the hashkey, we will reset
the per tuple context. However, later in MemoizeHash_equal, we still
need to reference the values in probeslot->tts_values[], which have been
cleared.
Actually the context would always be reset in MemoizeHash_equal, for
both binary and logical mode. So I kind of wonder if it's necessary to
reset the context in MemoizeHash_hash.
The ResetExprContext call in MemoizeHash_hash was introduced in
0b053e78b to fix a memory leak issue.
commit 0b053e78b5990cd01e7169ee5bd2bb8e4045deea
Author: David Rowley <
drowley@postgresql.org>
Date: Thu Oct 5 20:30:47 2023 +1300
Fix memory leak in Memoize code
It seems to me that switching to the per-tuple memory context is
sufficient to fix the memory leak. Calling ResetExprContext in
MemoizeHash_hash each time seems too aggressive.
I tried to remove the ResetExprContext call in MemoizeHash_hash and did
not see the memory leak with the repro query in [1].
diff --git a/src/backend/executor/nodeMemoize.c b/src/backend/executor/nodeMemoize.c
index 18870f10e1..f2f025520d 100644
--- a/src/backend/executor/nodeMemoize.c
+++ b/src/backend/executor/nodeMemoize.c
@@ -207,7 +207,6 @@ MemoizeHash_hash(struct memoize_hash *tb, const MemoizeKey *key)
}
}
- ResetExprContext(econtext);
MemoryContextSwitchTo(oldcontext);
return murmurhash32(hashkey);
}
Looping in David to have a look.
[1]
https://www.postgresql.org/message-id/flat/83281eed63c74e4f940317186372abfd%40cft.ruThanks
Richard
