Re: "type with xxxx does not exist" when doing ExecMemoize()

From: Richard Guo <guofenglinux(at)gmail(dot)com>
To: Andrei Lepikhov <a(dot)lepikhov(at)postgrespro(dot)ru>
Cc: Tender Wang <tndrwang(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, David Rowley <dgrowleyml(at)gmail(dot)com>
Subject: Re: "type with xxxx does not exist" when doing ExecMemoize()
Date: 2024-02-26 11:34:33
Message-ID: CAMbWs481ZsCBi+63mRatE0VByijSFwhu=OBARFptM4rsza-2Yw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs pgsql-hackers

On Mon, Feb 26, 2024 at 3:54 PM Andrei Lepikhov <a(dot)lepikhov(at)postgrespro(dot)ru>
wrote:

> On 26/2/2024 12:44, Tender Wang wrote:
> > Make sense. I found MemoizeState already has a MemoryContext, so I used
> it.
> > I update the patch.
> This approach is better for me. In the next version of this patch, I
> included a test case. I am still unsure about the context chosen and the
> stability of the test case. Richard, you recently fixed some Memoize
> issues, could you look at this problem and patch?

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(at)postgresql(dot)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.ru

Thanks
Richard

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message PG Bug reporting form 2024-02-26 12:03:11 BUG #18364: psql execution error: Segmentation fault
Previous Message Andrei Lepikhov 2024-02-26 09:30:50 Re: "type with xxxx does not exist" when doing ExecMemoize()

Browse pgsql-hackers by date

  From Date Subject
Next Message Xing Guo 2024-02-26 11:42:18 Re: Control your disk usage in PG: Introduction to Disk Quota Extension
Previous Message Tomas Vondra 2024-02-26 09:43:31 Re: Improve eviction algorithm in ReorderBuffer