Message-ID: <aacdf19f-778d-4a92-a2ba-2fa5f56401b4@postgrespro.ru>
Date: Mon, 26 Feb 2024 19:29:12 +0700
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: "type with xxxx does not exist" when doing ExecMemoize()
Content-Language: en-US
To: Richard Guo <guofenglinux@gmail.com>
Cc: Tender Wang <tndrwang@gmail.com>,
 PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>,
 David Rowley <dgrowleyml@gmail.com>
References: 
 <CAHewXNnT6N6UJkya0z-jLFzVxcwGfeRQSfhiwA+NyLg-x8iGew@mail.gmail.com>
 <d3da9856-0394-48e6-9f41-543c589e9585@postgrespro.ru>
 <CAHewXNm9KEn8CMOK4q0zutkX4BK8o754LgQbzYKgcj7_g1GgjQ@mail.gmail.com>
 <7a6a807c-193f-4333-937b-8211b1330579@postgrespro.ru>
 <CAMbWs481ZsCBi+63mRatE0VByijSFwhu=OBARFptM4rsza-2Yw@mail.gmail.com>
From: Andrei Lepikhov <a.lepikhov@postgrespro.ru>
Organization: Postgres Professional
In-Reply-To: 
 <CAMbWs481ZsCBi+63mRatE0VByijSFwhu=OBARFptM4rsza-2Yw@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Archived-At: 
 <https://www.postgresql.org/message-id/aacdf19f-778d-4a92-a2ba-2fa5f56401b4%40postgrespro.ru>
Precedence: bulk

On 26/2/2024 18:34, Richard Guo wrote:
> 
> On Mon, Feb 26, 2024 at 3:54 PM Andrei Lepikhov 
> <a.lepikhov@postgrespro.ru <mailto:a.lepikhov@postgrespro.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.
Agree
> 
> 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.
I can only provide one thought against this solution: what if we have a 
lot of unique hash values, maybe all of them? In that case, we still 
have a kind of 'leak' David fixed by the commit 0b053e78b5.
Also, I have a segfault report of one client. As I see, it was caused by 
too long text column in the table slot. As I see, key value, stored in 
the Memoize hash table, was corrupted, and the most plain reason is this 
bug. Should we add a test on this bug, and what do you think about the 
one proposed in v3?

-- 
regards,
Andrei Lepikhov
Postgres Professional