From 454ad31f88b51d6f821cf22a55008e7b5fdb4ce3 Mon Sep 17 00:00:00 2001 From: Ashutosh Bapat 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