public inbox for [email protected]
help / color / mirror / Atom feedFrom: Richard Guo <[email protected]>
To: PostgreSQL-development <[email protected]>
Subject: Re: A problem about ParamPathInfo for an AppendPath
Date: Mon, 12 Dec 2022 17:55:01 +0800
Message-ID: <CAMbWs49kv-yVOGKuPt6qZYHeDneCCPL-KS5uMDkcB3uJgHDtSA@mail.gmail.com> (raw)
In-Reply-To: <CAMbWs4_ABSu4PWG2rE1q10tJugEXHWgru3U8dAgkoFvgrb6aEA@mail.gmail.com>
References: <CAMbWs4_ABSu4PWG2rE1q10tJugEXHWgru3U8dAgkoFvgrb6aEA@mail.gmail.com>
On Tue, Dec 6, 2022 at 5:00 PM Richard Guo <[email protected]> wrote:
> As we can see, MemoizePath can be generated for partitioned AppendPath
> but not for union-all AppendPath.
>
> For the fix I think we can relax the check in create_append_path and
> always use get_baserel_parampathinfo if the parent is a baserel.
>
BTW, IIUC currently we don't generate any parameterized MergeAppend
paths, as explained in generate_orderedappend_paths. So the codes that
gather information from a MergeAppend path's param_info for run-time
partition pruning in create_merge_append_plan seem unnecessary.
Attached is a patch for this change and the changes described upthread.
Thanks
Richard
Attachments:
[application/octet-stream] v1-0001-Fix-ParamPathInfo-for-AppendPath.patch (4.9K, ../CAMbWs49kv-yVOGKuPt6qZYHeDneCCPL-KS5uMDkcB3uJgHDtSA@mail.gmail.com/3-v1-0001-Fix-ParamPathInfo-for-AppendPath.patch)
download | inline diff:
From 53d506973e108e0dfbb6b80485d417bb5a863fa5 Mon Sep 17 00:00:00 2001
From: Richard Guo <[email protected]>
Date: Mon, 12 Dec 2022 11:29:58 +0800
Subject: [PATCH v1] Fix ParamPathInfo for AppendPath
---
src/backend/optimizer/plan/createplan.c | 12 ++----------
src/backend/optimizer/util/pathnode.c | 8 +++++++-
src/test/regress/expected/memoize.out | 24 ++++++++++++++++++++++++
src/test/regress/sql/memoize.sql | 8 ++++++++
4 files changed, 41 insertions(+), 11 deletions(-)
diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index 66139928e8..9124d09b91 100644
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -1531,16 +1531,8 @@ create_merge_append_plan(PlannerInfo *root, MergeAppendPath *best_path,
prunequal = extract_actual_clauses(rel->baserestrictinfo, false);
- if (best_path->path.param_info)
- {
- List *prmquals = best_path->path.param_info->ppi_clauses;
-
- prmquals = extract_actual_clauses(prmquals, false);
- prmquals = (List *) replace_nestloop_params(root,
- (Node *) prmquals);
-
- prunequal = list_concat(prunequal, prmquals);
- }
+ /* We don't currently generate any parameterized MergeAppend paths */
+ Assert(best_path->path.param_info == NULL);
if (prunequal != NIL)
node->part_prune_index = make_partition_pruneinfo(root, rel,
diff --git a/src/backend/optimizer/util/pathnode.c b/src/backend/optimizer/util/pathnode.c
index 55deee555a..db23818e41 100644
--- a/src/backend/optimizer/util/pathnode.c
+++ b/src/backend/optimizer/util/pathnode.c
@@ -1258,6 +1258,9 @@ create_append_path(PlannerInfo *root,
pathnode->path.pathtarget = rel->reltarget;
/*
+ * There are several reasons why we might need parameterized quals for
+ * baserel.
+ *
* When generating an Append path for a partitioned table, there may be
* parameterized quals that are useful for run-time pruning. Hence,
* compute path.param_info the same way as for any other baserel, so that
@@ -1265,8 +1268,11 @@ create_append_path(PlannerInfo *root,
* would not work right for a non-baserel, ie a scan on a non-leaf child
* partition, and it's not necessary anyway in that case. Must skip it if
* we don't have "root", too.)
+ *
+ * Another reason is that we need parameterized quals to search for memoize
+ * cache keys when generating a Memoize path atop an Append path.
*/
- if (root && rel->reloptkind == RELOPT_BASEREL && IS_PARTITIONED_REL(rel))
+ if (root && rel->reloptkind == RELOPT_BASEREL)
pathnode->path.param_info = get_baserel_parampathinfo(root,
rel,
required_outer);
diff --git a/src/test/regress/expected/memoize.out b/src/test/regress/expected/memoize.out
index de43afa76e..2b82a27494 100644
--- a/src/test/regress/expected/memoize.out
+++ b/src/test/regress/expected/memoize.out
@@ -234,6 +234,30 @@ SELECT * FROM prt t1 INNER JOIN prt t2 ON t1.a = t2.a;', false);
Heap Fetches: N
(21 rows)
+-- Ensure memoize works with parameterized union-all Append path
+SET enable_partitionwise_join TO off;
+SELECT explain_memoize('
+SELECT * FROM prt_p1 t1 INNER JOIN
+(SELECT * FROM prt_p1 UNION ALL SELECT * FROM prt_p2) t2
+ON t1.a = t2.a;', false);
+ explain_memoize
+-------------------------------------------------------------------------------------
+ Nested Loop (actual rows=16 loops=N)
+ -> Index Only Scan using iprt_p1_a on prt_p1 t1 (actual rows=4 loops=N)
+ Heap Fetches: N
+ -> Memoize (actual rows=4 loops=N)
+ Cache Key: t1.a
+ Cache Mode: logical
+ Hits: 3 Misses: 1 Evictions: Zero Overflows: 0 Memory Usage: NkB
+ -> Append (actual rows=4 loops=N)
+ -> Index Only Scan using iprt_p1_a on prt_p1 (actual rows=4 loops=N)
+ Index Cond: (a = t1.a)
+ Heap Fetches: N
+ -> Index Only Scan using iprt_p2_a on prt_p2 (actual rows=0 loops=N)
+ Index Cond: (a = t1.a)
+ Heap Fetches: N
+(14 rows)
+
DROP TABLE prt;
RESET enable_partitionwise_join;
-- Exercise Memoize code that flushes the cache when a parameter changes which
diff --git a/src/test/regress/sql/memoize.sql b/src/test/regress/sql/memoize.sql
index 17c5b4bfab..a78c38ba6b 100644
--- a/src/test/regress/sql/memoize.sql
+++ b/src/test/regress/sql/memoize.sql
@@ -119,6 +119,14 @@ ANALYZE prt;
SELECT explain_memoize('
SELECT * FROM prt t1 INNER JOIN prt t2 ON t1.a = t2.a;', false);
+-- Ensure memoize works with parameterized union-all Append path
+SET enable_partitionwise_join TO off;
+
+SELECT explain_memoize('
+SELECT * FROM prt_p1 t1 INNER JOIN
+(SELECT * FROM prt_p1 UNION ALL SELECT * FROM prt_p2) t2
+ON t1.a = t2.a;', false);
+
DROP TABLE prt;
RESET enable_partitionwise_join;
--
2.31.0
view thread (2+ messages)
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: A problem about ParamPathInfo for an AppendPath
In-Reply-To: <CAMbWs49kv-yVOGKuPt6qZYHeDneCCPL-KS5uMDkcB3uJgHDtSA@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