public inbox for [email protected]
help / color / mirror / Atom feedFrom: Richard Guo <[email protected]>
To: PostgreSQL-development <[email protected]>
Subject: Re: Check lateral references within PHVs for memoize cache keys
Date: Tue, 4 Jul 2023 15:33:22 +0800
Message-ID: <CAMbWs4_E2HcTJ0b2srM3hwnQbH5SogkZMM399WUG4gTQSa1FOQ@mail.gmail.com> (raw)
In-Reply-To: <CAMbWs48TGnh8s=9-hJge9Et4Crf9q+_oU4FVekLoX4ommi4CVg@mail.gmail.com>
References: <CAMbWs48jLxn0pAPZpJ50EThZ569Xrw+=4Ac3QvkpQvNszbeoNg@mail.gmail.com>
<CAMbWs48TGnh8s=9-hJge9Et4Crf9q+_oU4FVekLoX4ommi4CVg@mail.gmail.com>
On Fri, Dec 30, 2022 at 11:00 AM Richard Guo <[email protected]> wrote:
> On Fri, Dec 9, 2022 at 5:16 PM Richard Guo <[email protected]> wrote:
>
>> Actually we do have checked PHVs for lateral references, earlier in
>> create_lateral_join_info. But that time we only marked lateral_relids
>> and direct_lateral_relids, without remembering the lateral expressions.
>> So I'm wondering whether we can fix that by fetching Vars (or PHVs) of
>> lateral references within PlaceHolderVars and remembering them in the
>> baserel's lateral_vars.
>>
>> Attach a draft patch to show my thoughts.
>>
>
> Update the patch to fix test failures.
>
Rebase the patch on HEAD as cfbot reminds.
Thanks
Richard
Attachments:
[application/octet-stream] v3-0001-Check-lateral-refs-within-PHVs-for-memoize-cache-keys.patch (7.0K, ../CAMbWs4_E2HcTJ0b2srM3hwnQbH5SogkZMM399WUG4gTQSa1FOQ@mail.gmail.com/3-v3-0001-Check-lateral-refs-within-PHVs-for-memoize-cache-keys.patch)
download | inline diff:
From 1fa67fc11c3483e5cf7e9c3bbc44924abce05bb7 Mon Sep 17 00:00:00 2001
From: Richard Guo <[email protected]>
Date: Fri, 30 Dec 2022 10:35:36 +0800
Subject: [PATCH v3] Check lateral refs within PHVs for memoize cache keys
---
.../postgres_fdw/expected/postgres_fdw.out | 12 +++--
src/backend/optimizer/plan/initsplan.c | 51 +++++++++++++++++++
src/test/regress/expected/memoize.out | 31 +++++++++++
src/test/regress/sql/memoize.sql | 11 ++++
4 files changed, 101 insertions(+), 4 deletions(-)
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index 852b5b4707..4e654ffa3a 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -3687,15 +3687,19 @@ ORDER BY ref_0."C 1";
-> Index Scan using t1_pkey on "S 1"."T 1" ref_0
Output: ref_0."C 1", ref_0.c2, ref_0.c3, ref_0.c4, ref_0.c5, ref_0.c6, ref_0.c7, ref_0.c8
Index Cond: (ref_0."C 1" < 10)
- -> Foreign Scan on public.ft1 ref_1
- Output: ref_1.c3, ref_0.c2
- Remote SQL: SELECT c3 FROM "S 1"."T 1" WHERE ((c3 = '00001'))
+ -> Memoize
+ Output: ref_1.c3, (ref_0.c2)
+ Cache Key: ref_0.c2
+ Cache Mode: binary
+ -> Foreign Scan on public.ft1 ref_1
+ Output: ref_1.c3, ref_0.c2
+ Remote SQL: SELECT c3 FROM "S 1"."T 1" WHERE ((c3 = '00001'))
-> Materialize
Output: ref_3.c3
-> Foreign Scan on public.ft2 ref_3
Output: ref_3.c3
Remote SQL: SELECT c3 FROM "S 1"."T 1" WHERE ((c3 = '00001'))
-(15 rows)
+(19 rows)
SELECT ref_0.c2, subq_1.*
FROM
diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c
index b31d892121..80f2902e12 100644
--- a/src/backend/optimizer/plan/initsplan.c
+++ b/src/backend/optimizer/plan/initsplan.c
@@ -582,6 +582,9 @@ create_lateral_join_info(PlannerInfo *root)
Relids eval_at = phinfo->ph_eval_at;
Relids lateral_refs;
int varno;
+ List *vars;
+ List *ph_lateral_vars;
+ ListCell *cell;
if (phinfo->ph_lateral == NULL)
continue; /* PHV is uninteresting if no lateral refs */
@@ -597,6 +600,40 @@ create_lateral_join_info(PlannerInfo *root)
lateral_refs = bms_intersect(phinfo->ph_lateral, root->all_baserels);
Assert(!bms_is_empty(lateral_refs));
+ /* Fetch Vars and PHVs of lateral references within PlaceHolderVars */
+ vars = pull_vars_of_level((Node *) phinfo->ph_var->phexpr, 0);
+
+ ph_lateral_vars = NIL;
+ foreach(cell, vars)
+ {
+ Node *node = (Node *) lfirst(cell);
+
+ node = copyObject(node);
+ if (IsA(node, Var))
+ {
+ Var *var = (Var *) node;
+
+ Assert(var->varlevelsup == 0);
+
+ if (bms_is_member(var->varno, phinfo->ph_lateral))
+ ph_lateral_vars = lappend(ph_lateral_vars, node);
+ }
+ else if (IsA(node, PlaceHolderVar))
+ {
+ PlaceHolderVar *phv = (PlaceHolderVar *) node;
+
+ Assert(phv->phlevelsup == 0);
+
+ if (bms_is_subset(find_placeholder_info(root, phv)->ph_eval_at,
+ phinfo->ph_lateral))
+ ph_lateral_vars = lappend(ph_lateral_vars, node);
+ }
+ else
+ Assert(false);
+ }
+
+ list_free(vars);
+
if (bms_get_singleton_member(eval_at, &varno))
{
/* Evaluation site is a baserel */
@@ -608,6 +645,13 @@ create_lateral_join_info(PlannerInfo *root)
brel->lateral_relids =
bms_add_members(brel->lateral_relids,
lateral_refs);
+
+ /*
+ * Remember the lateral references. We need this info for searching
+ * memoize cache keys.
+ */
+ brel->lateral_vars =
+ list_concat(brel->lateral_vars, ph_lateral_vars);
}
else
{
@@ -621,6 +665,13 @@ create_lateral_join_info(PlannerInfo *root)
continue; /* ignore outer joins in eval_at */
brel->lateral_relids = bms_add_members(brel->lateral_relids,
lateral_refs);
+
+ /*
+ * Remember the lateral references. We need this info for
+ * searching memoize cache keys.
+ */
+ brel->lateral_vars = list_concat(brel->lateral_vars,
+ ph_lateral_vars);
}
}
}
diff --git a/src/test/regress/expected/memoize.out b/src/test/regress/expected/memoize.out
index f5202430f8..1f59777903 100644
--- a/src/test/regress/expected/memoize.out
+++ b/src/test/regress/expected/memoize.out
@@ -92,6 +92,37 @@ WHERE t1.unique1 < 1000;
1000 | 9.5000000000000000
(1 row)
+-- Try with LATERAL references within PlaceHolderVars
+SELECT explain_memoize('
+SELECT COUNT(*),AVG(t1.twenty) FROM tenk1 t1 LEFT JOIN
+LATERAL (SELECT t1.twenty as c1, t2.unique1 as c2 FROM tenk1 t2) s on true
+WHERE s.c1 = s.c2 and t1.unique1 < 1000;', false);
+ explain_memoize
+-------------------------------------------------------------------------------------------
+ Aggregate (actual rows=1 loops=N)
+ -> Nested Loop (actual rows=1000 loops=N)
+ -> Seq Scan on tenk1 t1 (actual rows=1000 loops=N)
+ Filter: (unique1 < 1000)
+ Rows Removed by Filter: 9000
+ -> Memoize (actual rows=1 loops=N)
+ Cache Key: t1.twenty
+ Cache Mode: binary
+ Hits: 980 Misses: 20 Evictions: Zero Overflows: 0 Memory Usage: NkB
+ -> Index Only Scan using tenk1_unique1 on tenk1 t2 (actual rows=1 loops=N)
+ Filter: (t1.twenty = unique1)
+ Rows Removed by Filter: 9999
+ Heap Fetches: N
+(13 rows)
+
+-- And check we get the expected results.
+SELECT COUNT(*),AVG(t1.twenty) FROM tenk1 t1 LEFT JOIN
+LATERAL (SELECT t1.twenty as c1, t2.unique1 as c2 FROM tenk1 t2) s on true
+WHERE s.c1 = s.c2 and t1.unique1 < 1000;
+ count | avg
+-------+--------------------
+ 1000 | 9.5000000000000000
+(1 row)
+
-- Reduce work_mem and hash_mem_multiplier so that we see some cache evictions
SET work_mem TO '64kB';
SET hash_mem_multiplier TO 1.0;
diff --git a/src/test/regress/sql/memoize.sql b/src/test/regress/sql/memoize.sql
index 29ab1ea62d..86f1aae0c7 100644
--- a/src/test/regress/sql/memoize.sql
+++ b/src/test/regress/sql/memoize.sql
@@ -57,6 +57,17 @@ LATERAL (SELECT t2.unique1 FROM tenk1 t2
WHERE t1.twenty = t2.unique1 OFFSET 0) t2
WHERE t1.unique1 < 1000;
+-- Try with LATERAL references within PlaceHolderVars
+SELECT explain_memoize('
+SELECT COUNT(*),AVG(t1.twenty) FROM tenk1 t1 LEFT JOIN
+LATERAL (SELECT t1.twenty as c1, t2.unique1 as c2 FROM tenk1 t2) s on true
+WHERE s.c1 = s.c2 and t1.unique1 < 1000;', false);
+
+-- And check we get the expected results.
+SELECT COUNT(*),AVG(t1.twenty) FROM tenk1 t1 LEFT JOIN
+LATERAL (SELECT t1.twenty as c1, t2.unique1 as c2 FROM tenk1 t2) s on true
+WHERE s.c1 = s.c2 and t1.unique1 < 1000;
+
-- Reduce work_mem and hash_mem_multiplier so that we see some cache evictions
SET work_mem TO '64kB';
SET hash_mem_multiplier TO 1.0;
--
2.31.0
view thread (9+ messages) latest in thread
reply
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: [email protected]
Cc: [email protected]
Subject: Re: Check lateral references within PHVs for memoize cache keys
In-Reply-To: <CAMbWs4_E2HcTJ0b2srM3hwnQbH5SogkZMM399WUG4gTQSa1FOQ@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox