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

From: Tender Wang <tndrwang(at)gmail(dot)com>
To: Andrei Lepikhov <a(dot)lepikhov(at)postgrespro(dot)ru>
Cc: Richard Guo <guofenglinux(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-28 06:53:54
Message-ID: CAHewXNknsjG6JoDM48FWyyiFesG=K18vJMOY_LzPBDNV7DvmuQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs pgsql-hackers

The attached patch is a new version based on v3(not including Andrei's the
test case). There is no need to call datumCopy when
isnull is true.

I have not added a new runtime memoryContext so far. Continue to use
mstate->tableContext, I'm not sure the memory used of probeslot will affect
mstate->mem_limit.
Maybe adding a new memoryContext is better. I think I should spend a little
time to learn nodeMemoize.c more deeply.

Andrei Lepikhov <a(dot)lepikhov(at)postgrespro(dot)ru> 于2024年2月26日周一 20:29写道:

> On 26/2/2024 18:34, Richard Guo wrote:
> >
> > On Mon, Feb 26, 2024 at 3:54 PM Andrei Lepikhov
> > <a(dot)lepikhov(at)postgrespro(dot)ru <mailto: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.
> 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
>
>

--
Tender Wang
OpenPie: https://en.openpie.com/

Attachment Content-Type Size
v4-0001-Fix-RangeType-oid-not-found-when-doing-Memoize.patch application/octet-stream 1.9 KB

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Andrei Lepikhov 2024-02-28 07:25:04 Re: "type with xxxx does not exist" when doing ExecMemoize()
Previous Message Devrim Gündüz 2024-02-27 21:40:38 Re: systemd[1]: postgresql-16.service: Killing process 25992 (postgres) with signal SIGKILL.

Browse pgsql-hackers by date

  From Date Subject
Next Message shveta malik 2024-02-28 06:59:01 Re: Synchronizing slots from primary to standby
Previous Message Zhijie Hou (Fujitsu) 2024-02-28 06:48:37 RE: Synchronizing slots from primary to standby