From: Andrei Lepikhov <a.lepikhov@postgrespro.ru>
To: Tender Wang <tndrwang@gmail.com>
To: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
To: Richard Guo <guofenglinux@gmail.com>
Subject: Re: "type with xxxx does not exist" when doing ExecMemoize()
Date: Mon, 26 Feb 2024 14:54:21 +0700
Message-ID: <7a6a807c-193f-4333-937b-8211b1330579@postgrespro.ru> (raw)
In-Reply-To: <CAHewXNm9KEn8CMOK4q0zutkX4BK8o754LgQbzYKgcj7_g1GgjQ@mail.gmail.com>
References: <CAHewXNnT6N6UJkya0z-jLFzVxcwGfeRQSfhiwA+NyLg-x8iGew@mail.gmail.com>
<d3da9856-0394-48e6-9f41-543c589e9585@postgrespro.ru>
<CAHewXNm9KEn8CMOK4q0zutkX4BK8o754LgQbzYKgcj7_g1GgjQ@mail.gmail.com>
On 26/2/2024 12:44, Tender Wang wrote:
>
>
> Andrei Lepikhov <a.lepikhov@postgrespro.ru
> <mailto:a.lepikhov@postgrespro.ru>> 于2024年2月26日周一 10:57写道：
>
> On 25/2/2024 20:32, Tender Wang wrote:
> > I think in prepare_probe_slot(), should called datumCopy as the
> attached
> > patch does.
> >
> > Any thoughts? Thanks.
> Thanks for the report.
> I think it is better to invent a Runtime Memory Context; likewise,
> it is
> already designed in IndexScan and derivatives. Here, you just allocate
> the value in some upper memory context.
> Also, I'm curious why such a trivial error hasn't been found for a
> long time
>
>
> 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?
--
regards,
Andrei Lepikhov
Postgres Professional
Attachments:v3-0001-Store-Memoize-probeslot-values-in-the-hash-table-mem.patch (text/plain, 4.6K) download
From e32900e50730bccfde26355609d6b0b3e970f0a8 Mon Sep 17 00:00:00 2001
From: "tender.wang" <tender.wang@openpie.com>
Date: Mon, 26 Feb 2024 13:38:30 +0800
Subject: [PATCH] Store Memoize probeslot values in the hash table memory
context.
Values of probeslot evaluates in expression memory context which can be reset
on hash comparison when we still need it for the equality comparison.
So we copy the result to probeslot from ExecEvalExpr.
---
src/backend/executor/nodeMemoize.c | 23 ++++++++++++++++-------
src/test/regress/expected/memoize.out | 26 ++++++++++++++++++++++++++
src/test/regress/sql/memoize.sql | 14 ++++++++++++++
3 files changed, 56 insertions(+), 7 deletions(-)
diff --git a/src/backend/executor/nodeMemoize.c b/src/backend/executor/nodeMemoize.cindex 18870f10e1..6402943772 100644--- a/src/backend/executor/nodeMemoize.c+++ b/src/backend/executor/nodeMemoize.c@@ -312,17 +312,26 @@ prepare_probe_slot(MemoizeState *mstate, MemoizeKey *key)
if (key == NULL)
{
ExprContext *econtext = mstate->ss.ps.ps_ExprContext;
+ Datum value;+ bool isnull;+ TupleDesc tup = pslot->tts_tupleDescriptor;+ Form_pg_attribute att;
MemoryContext oldcontext;
- oldcontext = MemoryContextSwitchTo(econtext->ecxt_per_tuple_memory);-
/* Set the probeslot's values based on the current parameter values */
for (int i = 0; i < numKeys; i++)
- pslot->tts_values[i] = ExecEvalExpr(mstate->param_exprs[i],- econtext,- &pslot->tts_isnull[i]);-- MemoryContextSwitchTo(oldcontext);+ {+ att = TupleDescAttr(tup, i);+ value = ExecEvalExprSwitchContext(mstate->param_exprs[i],+ econtext,+ &isnull);+ /* Copy the value to avoid freed after resetting ExprContext */+ oldcontext = MemoryContextSwitchTo(mstate->tableContext);+ pslot->tts_values[i] = datumCopy(value, att->attbyval, att->attlen);+ MemoryContextSwitchTo(oldcontext);++ pslot->tts_isnull[i] = isnull;+ }
}
else
{
diff --git a/src/test/regress/expected/memoize.out b/src/test/regress/expected/memoize.outindex cf6886a288..9842b238c6 100644--- a/src/test/regress/expected/memoize.out+++ b/src/test/regress/expected/memoize.out@@ -196,6 +196,32 @@ SELECT * FROM flt f1 INNER JOIN flt f2 ON f1.f >= f2.f;', false);
(10 rows)
DROP TABLE flt;
+CREATE TABLE expr_key (x numeric, t text);+INSERT INTO expr_key (x,t) SELECT d1::numeric, d1::text FROM (+ SELECT round((d/pi())::numeric,7) AS d1 FROM generate_series(1,20) AS d);+-- Having duplicates we must see hits in the Memoize node+INSERT INTO expr_key SELECT * FROM expr_key;+CREATE INDEX expr_key_idx_x_t ON expr_key (x,t);+VACUUM ANALYZE expr_key;+SELECT explain_memoize('+SELECT * FROM expr_key t1 INNER JOIN expr_key t2+ON t1.x=t2.t::numeric AND t1.t::numeric=t2.x;', true);+ explain_memoize +-------------------------------------------------------------------------------------------+ Nested Loop (actual rows=80 loops=N)+ -> Index Only Scan using expr_key_idx_x_t on expr_key t1 (actual rows=40 loops=N)+ Heap Fetches: N+ -> Memoize (actual rows=2 loops=N)+ Cache Key: t1.x, (t1.t)::numeric+ Cache Mode: logical+ Hits: N Misses: N Evictions: Zero Overflows: 0 Memory Usage: NkB+ -> Index Only Scan using expr_key_idx_x_t on expr_key t2 (actual rows=2 loops=N)+ Index Cond: (x = (t1.t)::numeric)+ Filter: (t1.x = (t)::numeric)+ Heap Fetches: N+(11 rows)++DROP TABLE expr_key;
-- Exercise Memoize in binary mode with a large fixed width type and a
-- varlena type.
CREATE TABLE strtest (n name, t text);
diff --git a/src/test/regress/sql/memoize.sql b/src/test/regress/sql/memoize.sqlindex 1f4ab0ba3b..f2f7643571 100644--- a/src/test/regress/sql/memoize.sql+++ b/src/test/regress/sql/memoize.sql@@ -103,6 +103,20 @@ SELECT * FROM flt f1 INNER JOIN flt f2 ON f1.f >= f2.f;', false);
DROP TABLE flt;
+CREATE TABLE expr_key (x numeric, t text);+INSERT INTO expr_key (x,t) SELECT d1::numeric, d1::text FROM (+ SELECT round((d/pi())::numeric,7) AS d1 FROM generate_series(1,20) AS d);+-- Having duplicates we must see hits in the Memoize node+INSERT INTO expr_key SELECT * FROM expr_key;+CREATE INDEX expr_key_idx_x_t ON expr_key (x,t);+VACUUM ANALYZE expr_key;++SELECT explain_memoize('+SELECT * FROM expr_key t1 INNER JOIN expr_key t2+ON t1.x=t2.t::numeric AND t1.t::numeric=t2.x;', true);++DROP TABLE expr_key;+
-- Exercise Memoize in binary mode with a large fixed width type and a
-- varlena type.
CREATE TABLE strtest (n name, t text);
--
2.43.2
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: a.lepikhov@postgrespro.ru, tndrwang@gmail.com, pgsql-hackers@lists.postgresql.org, guofenglinux@gmail.com
Subject: Re: "type with xxxx does not exist" when doing ExecMemoize()
In-Reply-To: <7a6a807c-193f-4333-937b-8211b1330579@postgrespro.ru>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox