Hi Tom,
On Tue, Aug 22, 2023 at 2:18 AM Tom Lane <t...@sss.pgh.pa.us> wrote:
>
> (BTW, I did look at Ashutosh's idea of merging the
> reparameterize_path_by_child and path_is_reparameterizable_by_child
> functions, but I didn't think that would be an improvement,
> because we'd have to clutter reparameterize_path_by_child with
> a lot of should-I-skip-this-step tests. Some of that could be
> hidden in the macros, but a lot would not be. Another issue
> is that I do not think we can change reparameterize_path_by_child's
> API contract in the back branches, because we advertise it as
> something that FDWs and custom scan providers can use.)
Here are two patches
0001 same as your v5 patch
0002 implements what I had in mind - combining the assessment and
reparameterization in a single function.
I don't know whether you had something similar to 0002 when you
considered that approach. Hence implemented it quickly. The names of
the functions and arguments can be changed. But I think this version
will help us keep assessment and actual reparameterization in sync,
very tightly. Most of the conditionals have been pushed into macros,
except for two which need to be outside the macros and actually look
better to me. Let me know what you think of this.
FWIW I ran make check and all the tests pass.
I agree that we can not consider this for backbranches but we are
anyway thinking of disabling reparameterization for tablesample scans
anyway.
--
Best Wishes,
Ashutosh Bapat
From 15735df5693a962b51e0a8e384154db802c7870d Mon Sep 17 00:00:00 2001
From: Ashutosh Bapat <ashutosh.ba...@enterprisedb.com>
Date: Tue, 22 Aug 2023 17:07:37 +0530
Subject: [PATCH 1/2] Delay reparameterization of paths till create_plan()
v5 patch from Tom Lane as is
---
src/backend/optimizer/path/joinpath.c | 67 +++----
src/backend/optimizer/plan/createplan.c | 17 ++
src/backend/optimizer/util/pathnode.c | 195 ++++++++++++++++++-
src/include/optimizer/pathnode.h | 2 +
src/test/regress/expected/partition_join.out | 60 ++++++
src/test/regress/sql/partition_join.sql | 12 ++
6 files changed, 310 insertions(+), 43 deletions(-)
diff --git a/src/backend/optimizer/path/joinpath.c b/src/backend/optimizer/path/joinpath.c
index 821d282497..e2ecf5b14b 100644
--- a/src/backend/optimizer/path/joinpath.c
+++ b/src/backend/optimizer/path/joinpath.c
@@ -30,8 +30,9 @@
set_join_pathlist_hook_type set_join_pathlist_hook = NULL;
/*
- * Paths parameterized by the parent can be considered to be parameterized by
- * any of its child.
+ * Paths parameterized by a parent rel can be considered to be parameterized
+ * by any of its children, when we are performing partitionwise joins. These
+ * macros simplify checking for such cases. Beware multiple eval of args.
*/
#define PATH_PARAM_BY_PARENT(path, rel) \
((path)->param_info && bms_overlap(PATH_REQ_OUTER(path), \
@@ -762,6 +763,20 @@ try_nestloop_path(PlannerInfo *root,
/* If we got past that, we shouldn't have any unsafe outer-join refs */
Assert(!have_unsafe_outer_join_ref(root, outerrelids, inner_paramrels));
+ /*
+ * If the inner path is parameterized, it is parameterized by the topmost
+ * parent of the outer rel, not the outer rel itself. We will need to
+ * translate the parameterization, if this path is chosen, during
+ * create_plan(). Here we just check whether we will be able to perform
+ * the translation, and if not avoid creating a nestloop path.
+ */
+ if (PATH_PARAM_BY_PARENT(inner_path, outer_path->parent) &&
+ !path_is_reparameterizable_by_child(inner_path, outer_path->parent))
+ {
+ bms_free(required_outer);
+ return;
+ }
+
/*
* Do a precheck to quickly eliminate obviously-inferior paths. We
* calculate a cheap lower bound on the path's cost and then use
@@ -778,27 +793,6 @@ try_nestloop_path(PlannerInfo *root,
workspace.startup_cost, workspace.total_cost,
pathkeys, required_outer))
{
- /*
- * If the inner path is parameterized, it is parameterized by the
- * topmost parent of the outer rel, not the outer rel itself. Fix
- * that.
- */
- if (PATH_PARAM_BY_PARENT(inner_path, outer_path->parent))
- {
- inner_path = reparameterize_path_by_child(root, inner_path,
- outer_path->parent);
-
- /*
- * If we could not translate the path, we can't create nest loop
- * path.
- */
- if (!inner_path)
- {
- bms_free(required_outer);
- return;
- }
- }
-
add_path(joinrel, (Path *)
create_nestloop_path(root,
joinrel,
@@ -861,6 +855,17 @@ try_partial_nestloop_path(PlannerInfo *root,
return;
}
+ /*
+ * If the inner path is parameterized, it is parameterized by the topmost
+ * parent of the outer rel, not the outer rel itself. We will need to
+ * translate the parameterization, if this path is chosen, during
+ * create_plan(). Here we just check whether we will be able to perform
+ * the translation, and if not avoid creating a nestloop path.
+ */
+ if (PATH_PARAM_BY_PARENT(inner_path, outer_path->parent) &&
+ !path_is_reparameterizable_by_child(inner_path, outer_path->parent))
+ return;
+
/*
* Before creating a path, get a quick lower bound on what it is likely to
* cost. Bail out right away if it looks terrible.
@@ -870,22 +875,6 @@ try_partial_nestloop_path(PlannerInfo *root,
if (!add_partial_path_precheck(joinrel, workspace.total_cost, pathkeys))
return;
- /*
- * If the inner path is parameterized, it is parameterized by the topmost
- * parent of the outer rel, not the outer rel itself. Fix that.
- */
- if (PATH_PARAM_BY_PARENT(inner_path, outer_path->parent))
- {
- inner_path = reparameterize_path_by_child(root, inner_path,
- outer_path->parent);
-
- /*
- * If we could not translate the path, we can't create nest loop path.
- */
- if (!inner_path)
- return;
- }
-
/* Might be good enough to be worth trying, so let's try it. */
add_partial_path(joinrel, (Path *)
create_nestloop_path(root,
diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index 34ca6d4ac2..38bd179f4f 100644
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -29,6 +29,7 @@
#include "optimizer/cost.h"
#include "optimizer/optimizer.h"
#include "optimizer/paramassign.h"
+#include "optimizer/pathnode.h"
#include "optimizer/paths.h"
#include "optimizer/placeholder.h"
#include "optimizer/plancat.h"
@@ -4346,6 +4347,22 @@ create_nestloop_plan(PlannerInfo *root,
List *nestParams;
Relids saveOuterRels = root->curOuterRels;
+ /*
+ * If the inner path is parameterized by the topmost parent of the outer
+ * rel rather than the outer rel itself, fix that. (Nothing happens here
+ * if it is not so parameterized.)
+ */
+ best_path->jpath.innerjoinpath =
+ reparameterize_path_by_child(root,
+ best_path->jpath.innerjoinpath,
+ best_path->jpath.outerjoinpath->parent);
+
+ /*
+ * Failure here probably means that reparameterize_path_by_child() is not
+ * in sync with path_is_reparameterizable_by_child().
+ */
+ Assert(best_path->jpath.innerjoinpath != NULL);
+
/* NestLoop can project, so no need to be picky about child tlists */
outer_plan = create_plan_recurse(root, best_path->jpath.outerjoinpath, 0);
diff --git a/src/backend/optimizer/util/pathnode.c b/src/backend/optimizer/util/pathnode.c
index 211ba65389..21d002243c 100644
--- a/src/backend/optimizer/util/pathnode.c
+++ b/src/backend/optimizer/util/pathnode.c
@@ -56,6 +56,8 @@ static int append_startup_cost_compare(const ListCell *a, const ListCell *b);
static List *reparameterize_pathlist_by_child(PlannerInfo *root,
List *pathlist,
RelOptInfo *child_rel);
+static bool pathlist_is_reparameterizable_by_child(List *pathlist,
+ RelOptInfo *child_rel);
/*****************************************************************************
@@ -2436,6 +2438,16 @@ create_nestloop_path(PlannerInfo *root,
{
NestPath *pathnode = makeNode(NestPath);
Relids inner_req_outer = PATH_REQ_OUTER(inner_path);
+ Relids outerrelids;
+
+ /*
+ * Paths are parameterized by top-level parents, so run parameterization
+ * tests on the parent relids.
+ */
+ if (outer_path->parent->top_parent_relids)
+ outerrelids = outer_path->parent->top_parent_relids;
+ else
+ outerrelids = outer_path->parent->relids;
/*
* If the inner path is parameterized by the outer, we must drop any
@@ -2445,7 +2457,7 @@ create_nestloop_path(PlannerInfo *root,
* estimates for this path. We detect such clauses by checking for serial
* number match to clauses already enforced in the inner path.
*/
- if (bms_overlap(inner_req_outer, outer_path->parent->relids))
+ if (bms_overlap(inner_req_outer, outerrelids))
{
Bitmapset *enforced_serials = get_param_path_clause_serials(inner_path);
List *jclauses = NIL;
@@ -4042,6 +4054,12 @@ reparameterize_path(PlannerInfo *root, Path *path,
*
* Currently, only a few path types are supported here, though more could be
* added at need. We return NULL if we can't reparameterize the given path.
+ *
+ * Note that this function can change referenced RTEs as well as the Path
+ * structures. Therefore, it's only safe to call during create_plan(),
+ * when we have made a final choice of which Path to use for each RTE.
+ *
+ * Keep this code in sync with path_is_reparameterizable_by_child()!
*/
Path *
reparameterize_path_by_child(PlannerInfo *root, Path *path,
@@ -4054,7 +4072,7 @@ reparameterize_path_by_child(PlannerInfo *root, Path *path,
#define ADJUST_CHILD_ATTRS(node) \
((node) = \
- (List *) adjust_appendrel_attrs_multilevel(root, (Node *) (node), \
+ (void *) adjust_appendrel_attrs_multilevel(root, (Node *) (node), \
child_rel, \
child_rel->top_parent))
@@ -4082,8 +4100,8 @@ do { \
Relids required_outer;
/*
- * If the path is not parameterized by parent of the given relation, it
- * doesn't need reparameterization.
+ * If the path is not parameterized by the parent of the given relation,
+ * it doesn't need reparameterization.
*/
if (!path->param_info ||
!bms_overlap(PATH_REQ_OUTER(path), child_rel->top_parent_relids))
@@ -4105,6 +4123,19 @@ do { \
{
case T_Path:
FLAT_COPY_PATH(new_path, path, Path);
+ if (path->pathtype == T_SampleScan)
+ {
+ Index scan_relid = path->parent->relid;
+ RangeTblEntry *rte;
+
+ /* it should be a base rel with a tablesample clause... */
+ Assert(scan_relid > 0);
+ rte = planner_rt_fetch(scan_relid, root);
+ Assert(rte->rtekind == RTE_RELATION);
+ Assert(rte->tablesample != NULL);
+
+ ADJUST_CHILD_ATTRS(rte->tablesample);
+ }
break;
case T_IndexPath:
@@ -4335,9 +4366,145 @@ do { \
return new_path;
}
+/*
+ * path_is_reparameterizable_by_child
+ * Given a path parameterized by the parent of the given child relation,
+ * see if it can be translated to be parameterized by the child relation.
+ *
+ * This must return true if and only if reparameterize_path_by_child()
+ * would succeed on this path. Currently it's sufficient to verify that
+ * the path and all of its subpaths (if any) are of the types handled by
+ * that function. However, sub-paths that are not parameterized can be
+ * disregarded since they won't require translation.
+ */
+bool
+path_is_reparameterizable_by_child(Path *path, RelOptInfo *child_rel)
+{
+
+#define CHILD_PATH_IS_REPARAMETERIZABLE(path) \
+do { \
+ if (!path_is_reparameterizable_by_child(path, child_rel)) \
+ return false; \
+} while(0)
+
+#define CHILD_PATH_LIST_IS_REPARAMETERIZABLE(pathlist) \
+do { \
+ if (!pathlist_is_reparameterizable_by_child(pathlist, child_rel)) \
+ return false; \
+} while(0)
+
+ /*
+ * If the path is not parameterized by the parent of the given relation,
+ * it doesn't need reparameterization.
+ */
+ if (!path->param_info ||
+ !bms_overlap(PATH_REQ_OUTER(path), child_rel->top_parent_relids))
+ return true;
+
+ switch (nodeTag(path))
+ {
+ case T_Path:
+ case T_IndexPath:
+ break;
+
+ case T_BitmapHeapPath:
+ {
+ BitmapHeapPath *bhpath = (BitmapHeapPath *) path;
+
+ CHILD_PATH_IS_REPARAMETERIZABLE(bhpath->bitmapqual);
+ }
+ break;
+
+ case T_BitmapAndPath:
+ {
+ BitmapAndPath *bapath = (BitmapAndPath *) path;
+
+ CHILD_PATH_LIST_IS_REPARAMETERIZABLE(bapath->bitmapquals);
+ }
+ break;
+
+ case T_BitmapOrPath:
+ {
+ BitmapOrPath *bopath = (BitmapOrPath *) path;
+
+ CHILD_PATH_LIST_IS_REPARAMETERIZABLE(bopath->bitmapquals);
+ }
+ break;
+
+ case T_ForeignPath:
+ {
+ ForeignPath *fpath = (ForeignPath *) path;
+
+ if (fpath->fdw_outerpath)
+ CHILD_PATH_IS_REPARAMETERIZABLE(fpath->fdw_outerpath);
+ }
+ break;
+
+ case T_CustomPath:
+ {
+ CustomPath *cpath = (CustomPath *) path;
+
+ CHILD_PATH_LIST_IS_REPARAMETERIZABLE(cpath->custom_paths);
+ }
+ break;
+
+ case T_NestPath:
+ case T_MergePath:
+ case T_HashPath:
+ {
+ JoinPath *jpath = (JoinPath *) path;
+
+ CHILD_PATH_IS_REPARAMETERIZABLE(jpath->outerjoinpath);
+ CHILD_PATH_IS_REPARAMETERIZABLE(jpath->innerjoinpath);
+ }
+ break;
+
+ case T_AppendPath:
+ {
+ AppendPath *apath = (AppendPath *) path;
+
+ CHILD_PATH_LIST_IS_REPARAMETERIZABLE(apath->subpaths);
+ }
+ break;
+
+ case T_MaterialPath:
+ {
+ MaterialPath *mpath = (MaterialPath *) path;
+
+ CHILD_PATH_IS_REPARAMETERIZABLE(mpath->subpath);
+ }
+ break;
+
+ case T_MemoizePath:
+ {
+ MemoizePath *mpath = (MemoizePath *) path;
+
+ CHILD_PATH_IS_REPARAMETERIZABLE(mpath->subpath);
+ }
+ break;
+
+ case T_GatherPath:
+ {
+ GatherPath *gpath = (GatherPath *) path;
+
+ CHILD_PATH_IS_REPARAMETERIZABLE(gpath->subpath);
+ }
+ break;
+
+ default:
+
+ /* We don't know how to reparameterize this path. */
+ return false;
+ }
+
+ return true;
+}
+
/*
* reparameterize_pathlist_by_child
* Helper function to reparameterize a list of paths by given child rel.
+ *
+ * Returns NIL to indicate failure, so pathlist had better not be NIL.
*/
static List *
reparameterize_pathlist_by_child(PlannerInfo *root,
@@ -4363,3 +4530,23 @@ reparameterize_pathlist_by_child(PlannerInfo *root,
return result;
}
+
+/*
+ * pathlist_is_reparameterizable_by_child
+ * Helper function to check if a list of paths can be reparameterized.
+ */
+static bool
+pathlist_is_reparameterizable_by_child(List *pathlist, RelOptInfo *child_rel)
+{
+ ListCell *lc;
+
+ foreach(lc, pathlist)
+ {
+ Path *path = (Path *) lfirst(lc);
+
+ if (!path_is_reparameterizable_by_child(path, child_rel))
+ return false;
+ }
+
+ return true;
+}
diff --git a/src/include/optimizer/pathnode.h b/src/include/optimizer/pathnode.h
index 6e557bebc4..f5f8cbcfb4 100644
--- a/src/include/optimizer/pathnode.h
+++ b/src/include/optimizer/pathnode.h
@@ -298,6 +298,8 @@ extern Path *reparameterize_path(PlannerInfo *root, Path *path,
double loop_count);
extern Path *reparameterize_path_by_child(PlannerInfo *root, Path *path,
RelOptInfo *child_rel);
+extern bool path_is_reparameterizable_by_child(Path *path,
+ RelOptInfo *child_rel);
/*
* prototypes for relnode.c
diff --git a/src/test/regress/expected/partition_join.out b/src/test/regress/expected/partition_join.out
index 6560fe2416..a11f738411 100644
--- a/src/test/regress/expected/partition_join.out
+++ b/src/test/regress/expected/partition_join.out
@@ -505,6 +505,31 @@ SELECT t1.a, ss.t2a, ss.t2c FROM prt1 t1 LEFT JOIN LATERAL
550 | |
(12 rows)
+-- lateral reference in sample scan
+EXPLAIN (COSTS OFF)
+SELECT * FROM prt1 t1 JOIN LATERAL
+ (SELECT * FROM prt1 t2 TABLESAMPLE SYSTEM (t1.a) REPEATABLE(t1.b)) s
+ ON t1.a = s.a;
+ QUERY PLAN
+-------------------------------------------------------------
+ Append
+ -> Nested Loop
+ -> Seq Scan on prt1_p1 t1_1
+ -> Sample Scan on prt1_p1 t2_1
+ Sampling: system (t1_1.a) REPEATABLE (t1_1.b)
+ Filter: (t1_1.a = a)
+ -> Nested Loop
+ -> Seq Scan on prt1_p2 t1_2
+ -> Sample Scan on prt1_p2 t2_2
+ Sampling: system (t1_2.a) REPEATABLE (t1_2.b)
+ Filter: (t1_2.a = a)
+ -> Nested Loop
+ -> Seq Scan on prt1_p3 t1_3
+ -> Sample Scan on prt1_p3 t2_3
+ Sampling: system (t1_3.a) REPEATABLE (t1_3.b)
+ Filter: (t1_3.a = a)
+(16 rows)
+
-- bug with inadequate sort key representation
SET enable_partitionwise_aggregate TO true;
SET enable_hashjoin TO false;
@@ -1944,6 +1969,41 @@ SELECT * FROM prt1_l t1 LEFT JOIN LATERAL
550 | 0 | 0002 | | | | |
(12 rows)
+-- partitionwise join with lateral reference in sample scan
+EXPLAIN (COSTS OFF)
+SELECT * FROM prt1_l t1 JOIN LATERAL
+ (SELECT * FROM prt1_l t2 TABLESAMPLE SYSTEM (t1.a) REPEATABLE(t1.b)) s ON
+ t1.a = s.a AND t1.b = s.b AND t1.c = s.c;
+ QUERY PLAN
+----------------------------------------------------------------------------------------
+ Append
+ -> Nested Loop
+ -> Seq Scan on prt1_l_p1 t1_1
+ -> Sample Scan on prt1_l_p1 t2_1
+ Sampling: system (t1_1.a) REPEATABLE (t1_1.b)
+ Filter: ((t1_1.a = a) AND (t1_1.b = b) AND ((t1_1.c)::text = (c)::text))
+ -> Nested Loop
+ -> Seq Scan on prt1_l_p2_p1 t1_2
+ -> Sample Scan on prt1_l_p2_p1 t2_2
+ Sampling: system (t1_2.a) REPEATABLE (t1_2.b)
+ Filter: ((t1_2.a = a) AND (t1_2.b = b) AND ((t1_2.c)::text = (c)::text))
+ -> Nested Loop
+ -> Seq Scan on prt1_l_p2_p2 t1_3
+ -> Sample Scan on prt1_l_p2_p2 t2_3
+ Sampling: system (t1_3.a) REPEATABLE (t1_3.b)
+ Filter: ((t1_3.a = a) AND (t1_3.b = b) AND ((t1_3.c)::text = (c)::text))
+ -> Nested Loop
+ -> Seq Scan on prt1_l_p3_p1 t1_4
+ -> Sample Scan on prt1_l_p3_p1 t2_4
+ Sampling: system (t1_4.a) REPEATABLE (t1_4.b)
+ Filter: ((t1_4.a = a) AND (t1_4.b = b) AND ((t1_4.c)::text = (c)::text))
+ -> Nested Loop
+ -> Seq Scan on prt1_l_p3_p2 t1_5
+ -> Sample Scan on prt1_l_p3_p2 t2_5
+ Sampling: system (t1_5.a) REPEATABLE (t1_5.b)
+ Filter: ((t1_5.a = a) AND (t1_5.b = b) AND ((t1_5.c)::text = (c)::text))
+(26 rows)
+
-- join with one side empty
EXPLAIN (COSTS OFF)
SELECT t1.a, t1.c, t2.b, t2.c FROM (SELECT * FROM prt1_l WHERE a = 1 AND a = 2) t1 RIGHT JOIN prt2_l t2 ON t1.a = t2.b AND t1.b = t2.a AND t1.c = t2.c;
diff --git a/src/test/regress/sql/partition_join.sql b/src/test/regress/sql/partition_join.sql
index 48daf3aee3..e2daab03fb 100644
--- a/src/test/regress/sql/partition_join.sql
+++ b/src/test/regress/sql/partition_join.sql
@@ -100,6 +100,12 @@ SELECT t1.a, ss.t2a, ss.t2c FROM prt1 t1 LEFT JOIN LATERAL
(SELECT t2.a AS t2a, t3.a AS t3a, t2.b t2b, t2.c t2c, least(t1.a,t2.a,t3.a) FROM prt1 t2 JOIN prt2 t3 ON (t2.a = t3.b)) ss
ON t1.c = ss.t2c WHERE (t1.b + coalesce(ss.t2b, 0)) = 0 ORDER BY t1.a;
+-- lateral reference in sample scan
+EXPLAIN (COSTS OFF)
+SELECT * FROM prt1 t1 JOIN LATERAL
+ (SELECT * FROM prt1 t2 TABLESAMPLE SYSTEM (t1.a) REPEATABLE(t1.b)) s
+ ON t1.a = s.a;
+
-- bug with inadequate sort key representation
SET enable_partitionwise_aggregate TO true;
SET enable_hashjoin TO false;
@@ -387,6 +393,12 @@ SELECT * FROM prt1_l t1 LEFT JOIN LATERAL
(SELECT t2.a AS t2a, t2.c AS t2c, t2.b AS t2b, t3.b AS t3b, least(t1.a,t2.a,t3.b) FROM prt1_l t2 JOIN prt2_l t3 ON (t2.a = t3.b AND t2.c = t3.c)) ss
ON t1.a = ss.t2a AND t1.c = ss.t2c WHERE t1.b = 0 ORDER BY t1.a;
+-- partitionwise join with lateral reference in sample scan
+EXPLAIN (COSTS OFF)
+SELECT * FROM prt1_l t1 JOIN LATERAL
+ (SELECT * FROM prt1_l t2 TABLESAMPLE SYSTEM (t1.a) REPEATABLE(t1.b)) s ON
+ t1.a = s.a AND t1.b = s.b AND t1.c = s.c;
+
-- join with one side empty
EXPLAIN (COSTS OFF)
SELECT t1.a, t1.c, t2.b, t2.c FROM (SELECT * FROM prt1_l WHERE a = 1 AND a = 2) t1 RIGHT JOIN prt2_l t2 ON t1.a = t2.b AND t1.b = t2.a AND t1.c = t2.c;
--
2.25.1
From 454ad31f88b51d6f821cf22a55008e7b5fdb4ce3 Mon Sep 17 00:00:00 2001
From: Ashutosh Bapat <ashutosh.ba...@enterprisedb.com>
Date: Tue, 22 Aug 2023 21:27:39 +0530
Subject: [PATCH 2/2] Combine reparameterize_path_by_child and
path_is_reparameterizable_by_child
reparameterize_path_by_child works in two modes 1. to assess whether a given path can be reparameterized by child 2. actually reparameterizing it.
---
src/backend/optimizer/path/joinpath.c | 6 +-
src/backend/optimizer/plan/createplan.c | 3 +-
src/backend/optimizer/util/pathnode.c | 208 +++++++-----------------
src/include/foreign/fdwapi.h | 3 +-
src/include/nodes/extensible.h | 3 +-
src/include/optimizer/pathnode.h | 5 +-
6 files changed, 70 insertions(+), 158 deletions(-)
diff --git a/src/backend/optimizer/path/joinpath.c b/src/backend/optimizer/path/joinpath.c
index e2ecf5b14b..6bf16213fe 100644
--- a/src/backend/optimizer/path/joinpath.c
+++ b/src/backend/optimizer/path/joinpath.c
@@ -771,7 +771,8 @@ try_nestloop_path(PlannerInfo *root,
* the translation, and if not avoid creating a nestloop path.
*/
if (PATH_PARAM_BY_PARENT(inner_path, outer_path->parent) &&
- !path_is_reparameterizable_by_child(inner_path, outer_path->parent))
+ !path_is_reparameterizable_by_child(root, inner_path,
+ outer_path->parent))
{
bms_free(required_outer);
return;
@@ -863,7 +864,8 @@ try_partial_nestloop_path(PlannerInfo *root,
* the translation, and if not avoid creating a nestloop path.
*/
if (PATH_PARAM_BY_PARENT(inner_path, outer_path->parent) &&
- !path_is_reparameterizable_by_child(inner_path, outer_path->parent))
+ !path_is_reparameterizable_by_child(root, inner_path,
+ outer_path->parent))
return;
/*
diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index 38bd179f4f..29b7af94a8 100644
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -4355,7 +4355,8 @@ create_nestloop_plan(PlannerInfo *root,
best_path->jpath.innerjoinpath =
reparameterize_path_by_child(root,
best_path->jpath.innerjoinpath,
- best_path->jpath.outerjoinpath->parent);
+ best_path->jpath.outerjoinpath->parent,
+ false);
/*
* Failure here probably means that reparameterize_path_by_child() is not
diff --git a/src/backend/optimizer/util/pathnode.c b/src/backend/optimizer/util/pathnode.c
index 21d002243c..6eec0bf444 100644
--- a/src/backend/optimizer/util/pathnode.c
+++ b/src/backend/optimizer/util/pathnode.c
@@ -55,9 +55,8 @@ static int append_total_cost_compare(const ListCell *a, const ListCell *b);
static int append_startup_cost_compare(const ListCell *a, const ListCell *b);
static List *reparameterize_pathlist_by_child(PlannerInfo *root,
List *pathlist,
- RelOptInfo *child_rel);
-static bool pathlist_is_reparameterizable_by_child(List *pathlist,
- RelOptInfo *child_rel);
+ RelOptInfo *child_rel,
+ bool check_only);
/*****************************************************************************
@@ -4059,26 +4058,45 @@ reparameterize_path(PlannerInfo *root, Path *path,
* structures. Therefore, it's only safe to call during create_plan(),
* when we have made a final choice of which Path to use for each RTE.
*
+ * If check_only is true, the path is returned as is without any
+ * reparameterization if reparameterization is possible. If check_only is
+ * false, a reparameterized path is returned.
+ *
* Keep this code in sync with path_is_reparameterizable_by_child()!
*/
Path *
reparameterize_path_by_child(PlannerInfo *root, Path *path,
- RelOptInfo *child_rel)
+ RelOptInfo *child_rel, bool check_only)
{
#define FLAT_COPY_PATH(newnode, node, nodetype) \
- ( (newnode) = makeNode(nodetype), \
- memcpy((newnode), (node), sizeof(nodetype)) )
+do { \
+ if (!(check_only)) \
+ { \
+ (newnode) = makeNode(nodetype); \
+ memcpy((newnode), (node), sizeof(nodetype)); \
+ } \
+ else \
+ { \
+ (newnode) = (nodetype *) (node); \
+ } \
+} while (0)
#define ADJUST_CHILD_ATTRS(node) \
- ((node) = \
- (void *) adjust_appendrel_attrs_multilevel(root, (Node *) (node), \
+do { \
+ if (!(check_only)) \
+ { \
+ (node) = \
+ (void *) adjust_appendrel_attrs_multilevel(root, (Node *) (node), \
child_rel, \
- child_rel->top_parent))
+ child_rel->top_parent); \
+ } \
+} while (0)
#define REPARAMETERIZE_CHILD_PATH(path) \
do { \
- (path) = reparameterize_path_by_child(root, (path), child_rel); \
+ (path) = \
+ reparameterize_path_by_child(root, (path), child_rel, (check_only)); \
if ((path) == NULL) \
return NULL; \
} while(0)
@@ -4088,7 +4106,8 @@ do { \
if ((pathlist) != NIL) \
{ \
(pathlist) = reparameterize_pathlist_by_child(root, (pathlist), \
- child_rel); \
+ child_rel, \
+ (check_only)); \
if ((pathlist) == NIL) \
return NULL; \
} \
@@ -4123,7 +4142,7 @@ do { \
{
case T_Path:
FLAT_COPY_PATH(new_path, path, Path);
- if (path->pathtype == T_SampleScan)
+ if (path->pathtype == T_SampleScan && !check_only)
{
Index scan_relid = path->parent->relid;
RangeTblEntry *rte;
@@ -4194,7 +4213,7 @@ do { \
path->parent->fdwroutine->ReparameterizeForeignPathByChild;
if (rfpc_func)
fpath->fdw_private = rfpc_func(root, fpath->fdw_private,
- child_rel);
+ child_rel, check_only);
new_path = (Path *) fpath;
}
break;
@@ -4212,7 +4231,8 @@ do { \
cpath->custom_private =
cpath->methods->ReparameterizeCustomPathByChild(root,
cpath->custom_private,
- child_rel);
+ child_rel,
+ check_only);
new_path = (Path *) cpath;
}
break;
@@ -4311,6 +4331,16 @@ do { \
return NULL;
}
+ /*
+ * If we are here to only assess whether reparameterization is possible, we
+ * are done. In this mode we shouldn't be creating a new path node.
+ */
+ if (check_only)
+ {
+ Assert(new_path == path);
+ return new_path;
+ }
+
/*
* Adjust the parameterization information, which refers to the topmost
* parent. The topmost parent can be multiple levels away from the given
@@ -4378,125 +4408,20 @@ do { \
* disregarded since they won't require translation.
*/
bool
-path_is_reparameterizable_by_child(Path *path, RelOptInfo *child_rel)
+path_is_reparameterizable_by_child(PlannerInfo *root, Path *path,
+ RelOptInfo *child_rel)
{
+ Path *result;
-#define CHILD_PATH_IS_REPARAMETERIZABLE(path) \
-do { \
- if (!path_is_reparameterizable_by_child(path, child_rel)) \
- return false; \
-} while(0)
-
-#define CHILD_PATH_LIST_IS_REPARAMETERIZABLE(pathlist) \
-do { \
- if (!pathlist_is_reparameterizable_by_child(pathlist, child_rel)) \
- return false; \
-} while(0)
+ result = reparameterize_path_by_child(root, path, child_rel, true);
+ if (!result)
+ return false;
/*
- * If the path is not parameterized by the parent of the given relation,
- * it doesn't need reparameterization.
+ * Original path should be returned as is when we are assessing possibility
+ * of reparameterization.
*/
- if (!path->param_info ||
- !bms_overlap(PATH_REQ_OUTER(path), child_rel->top_parent_relids))
- return true;
-
- switch (nodeTag(path))
- {
- case T_Path:
- case T_IndexPath:
- break;
-
- case T_BitmapHeapPath:
- {
- BitmapHeapPath *bhpath = (BitmapHeapPath *) path;
-
- CHILD_PATH_IS_REPARAMETERIZABLE(bhpath->bitmapqual);
- }
- break;
-
- case T_BitmapAndPath:
- {
- BitmapAndPath *bapath = (BitmapAndPath *) path;
-
- CHILD_PATH_LIST_IS_REPARAMETERIZABLE(bapath->bitmapquals);
- }
- break;
-
- case T_BitmapOrPath:
- {
- BitmapOrPath *bopath = (BitmapOrPath *) path;
-
- CHILD_PATH_LIST_IS_REPARAMETERIZABLE(bopath->bitmapquals);
- }
- break;
-
- case T_ForeignPath:
- {
- ForeignPath *fpath = (ForeignPath *) path;
-
- if (fpath->fdw_outerpath)
- CHILD_PATH_IS_REPARAMETERIZABLE(fpath->fdw_outerpath);
- }
- break;
-
- case T_CustomPath:
- {
- CustomPath *cpath = (CustomPath *) path;
-
- CHILD_PATH_LIST_IS_REPARAMETERIZABLE(cpath->custom_paths);
- }
- break;
-
- case T_NestPath:
- case T_MergePath:
- case T_HashPath:
- {
- JoinPath *jpath = (JoinPath *) path;
-
- CHILD_PATH_IS_REPARAMETERIZABLE(jpath->outerjoinpath);
- CHILD_PATH_IS_REPARAMETERIZABLE(jpath->innerjoinpath);
- }
- break;
-
- case T_AppendPath:
- {
- AppendPath *apath = (AppendPath *) path;
-
- CHILD_PATH_LIST_IS_REPARAMETERIZABLE(apath->subpaths);
- }
- break;
-
- case T_MaterialPath:
- {
- MaterialPath *mpath = (MaterialPath *) path;
-
- CHILD_PATH_IS_REPARAMETERIZABLE(mpath->subpath);
- }
- break;
-
- case T_MemoizePath:
- {
- MemoizePath *mpath = (MemoizePath *) path;
-
- CHILD_PATH_IS_REPARAMETERIZABLE(mpath->subpath);
- }
- break;
-
- case T_GatherPath:
- {
- GatherPath *gpath = (GatherPath *) path;
-
- CHILD_PATH_IS_REPARAMETERIZABLE(gpath->subpath);
- }
- break;
-
- default:
-
- /* We don't know how to reparameterize this path. */
- return false;
- }
-
+ Assert(result == path);
return true;
}
@@ -4509,7 +4434,8 @@ do { \
static List *
reparameterize_pathlist_by_child(PlannerInfo *root,
List *pathlist,
- RelOptInfo *child_rel)
+ RelOptInfo *child_rel,
+ bool check_only)
{
ListCell *lc;
List *result = NIL;
@@ -4517,7 +4443,7 @@ reparameterize_pathlist_by_child(PlannerInfo *root,
foreach(lc, pathlist)
{
Path *path = reparameterize_path_by_child(root, lfirst(lc),
- child_rel);
+ child_rel, check_only);
if (path == NULL)
{
@@ -4529,24 +4455,4 @@ reparameterize_pathlist_by_child(PlannerInfo *root,
}
return result;
-}
-
-/*
- * pathlist_is_reparameterizable_by_child
- * Helper function to check if a list of paths can be reparameterized.
- */
-static bool
-pathlist_is_reparameterizable_by_child(List *pathlist, RelOptInfo *child_rel)
-{
- ListCell *lc;
-
- foreach(lc, pathlist)
- {
- Path *path = (Path *) lfirst(lc);
-
- if (!path_is_reparameterizable_by_child(path, child_rel))
- return false;
- }
-
- return true;
-}
+}
\ No newline at end of file
diff --git a/src/include/foreign/fdwapi.h b/src/include/foreign/fdwapi.h
index 996c62e305..5715394fde 100644
--- a/src/include/foreign/fdwapi.h
+++ b/src/include/foreign/fdwapi.h
@@ -181,7 +181,8 @@ typedef bool (*IsForeignScanParallelSafe_function) (PlannerInfo *root,
RangeTblEntry *rte);
typedef List *(*ReparameterizeForeignPathByChild_function) (PlannerInfo *root,
List *fdw_private,
- RelOptInfo *child_rel);
+ RelOptInfo *child_rel,
+ bool check_only);
typedef bool (*IsForeignPathAsyncCapable_function) (ForeignPath *path);
diff --git a/src/include/nodes/extensible.h b/src/include/nodes/extensible.h
index 7d51c6033e..b7a8b6b089 100644
--- a/src/include/nodes/extensible.h
+++ b/src/include/nodes/extensible.h
@@ -102,7 +102,8 @@ typedef struct CustomPathMethods
List *custom_plans);
struct List *(*ReparameterizeCustomPathByChild) (PlannerInfo *root,
List *custom_private,
- RelOptInfo *child_rel);
+ RelOptInfo *child_rel,
+ bool check_only);
} CustomPathMethods;
/*
diff --git a/src/include/optimizer/pathnode.h b/src/include/optimizer/pathnode.h
index f5f8cbcfb4..b51c660c67 100644
--- a/src/include/optimizer/pathnode.h
+++ b/src/include/optimizer/pathnode.h
@@ -297,8 +297,9 @@ extern Path *reparameterize_path(PlannerInfo *root, Path *path,
Relids required_outer,
double loop_count);
extern Path *reparameterize_path_by_child(PlannerInfo *root, Path *path,
- RelOptInfo *child_rel);
-extern bool path_is_reparameterizable_by_child(Path *path,
+ RelOptInfo *child_rel,
+ bool check_only);
+extern bool path_is_reparameterizable_by_child(PlannerInfo *root, Path *path,
RelOptInfo *child_rel);
/*
--
2.25.1
