From 1b2a42da985fbd0287472ba3b348528d8931ba9d Mon Sep 17 00:00:00 2001 From: "Andrei V. Lepikhov" Date: Thu, 26 Jun 2025 15:11:03 +0200 Subject: [PATCH 1/3] Basic infrastructure to link, unlink, and free pathnodes. add_path and add_partial_path free path nodes that they deem suboptimal. But they just free the uppermost pathnode in the path. The subpaths continue to occupy memory even if they are not used anywhere. The subpath nodes are not freed because other paths may reference them. This commit introduces the infrastructure to track references to paths. As the path nodes are referenced, they are "linked" using link_path() to reference objects. When the path nodes are no longer helpful, they are "unlinked" using unlink_path() from the referencing objects. The paths whose references reach zero during unlinking are freed automatically using the free_path() function. The function unlinks the sub-path nodes and can be called when freeing any path node. When the final path for the join tree is chosen, the paths linked to each participating relation are unlinked, thus ultimately getting freed if not used. TODO The functions free_path(), unlink_path() and link_path() have ereports to catch code paths that do not use these functions correctly. They call errbacktrace(), which is not used anywhere else. These ereport calls will need to be fixed for productization. --- src/backend/optimizer/path/allpaths.c | 83 +++++++++++++++ src/backend/optimizer/util/pathnode.c | 142 ++++++++++++++++++++++++++ src/include/nodes/pathnodes.h | 1 + src/include/optimizer/pathnode.h | 38 +++++++ 4 files changed, 264 insertions(+) diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c index 6cc6966b060..a5d48397ac3 100644 --- a/src/backend/optimizer/path/allpaths.c +++ b/src/backend/optimizer/path/allpaths.c @@ -3418,6 +3418,87 @@ make_rel_from_joinlist(PlannerInfo *root, List *joinlist) } } +/* + * TODO: Need similar code to free paths in upper relations. + */ +static void +free_unused_paths_from_rel(RelOptInfo *rel) +{ + ListCell *lc_path; + int cnt_part; + + foreach(lc_path, rel->pathlist) + { + Path *path = (Path *) lfirst(lc_path); + + /* Free the path if none references it. */ + if (path->ref_count == 1) + { + /* TODO: use routine to unlink path from list */ + rel->pathlist = foreach_delete_current(rel->pathlist, lc_path); + unlink_path(&path); + } + } + + /* Do the same for partial pathlist. */ + foreach(lc_path, rel->partial_pathlist) + { + Path *path = (Path *) lfirst(lc_path); + + /* Free the path if none references it. */ + if (path->ref_count == 1) + { + rel->partial_pathlist = foreach_delete_current(rel->partial_pathlist, lc_path); + unlink_path(&path); + } + } + + /* + * TODO: We can perform this in generate_partitionwise_paths as well since + * by that time all the paths from partitions will be linked if used. + */ + + /* Free unused paths from the partition relations */ + for (cnt_part = 0; cnt_part < rel->nparts; cnt_part++) + { + RelOptInfo *child_rel = rel->part_rels[cnt_part]; + + if (child_rel == NULL || IS_DUMMY_REL(child_rel)) + /* Skip empty entries */ + continue; + + free_unused_paths_from_rel(child_rel); + } +} + +/* + * Free unused paths from all the relations created while building the join tree. + */ +static void +free_unused_paths(PlannerInfo *root, int levels_needed) +{ + int cnt; + + for (cnt = levels_needed - 1; cnt >= 1; cnt--) + { + ListCell *lc; + + foreach(lc, root->join_rel_level[cnt]) + { + free_unused_paths_from_rel(lfirst(lc)); + } + } + + for (cnt = 0; cnt < root->simple_rel_array_size; cnt++) + { + RelOptInfo *rel = root->simple_rel_array[cnt]; + + /* Skip empty slots */ + if (rel != NULL) + free_unused_paths_from_rel(rel); + } +} + /* * standard_join_search * Find possible joinpaths for a query by successively finding ways @@ -3520,6 +3601,8 @@ standard_join_search(PlannerInfo *root, int levels_needed, List *initial_rels) } } + free_unused_paths(root, levels_needed); + /* * We should have a single rel at the final level. */ diff --git a/src/backend/optimizer/util/pathnode.c b/src/backend/optimizer/util/pathnode.c index e0192d4a491..fa13a8f8ff1 100644 --- a/src/backend/optimizer/util/pathnode.c +++ b/src/backend/optimizer/util/pathnode.c @@ -969,6 +969,148 @@ add_partial_path_precheck(RelOptInfo *parent_rel, int disabled_nodes, return true; } +void +free_path(Path *path) +{ + /* + * If the path is still referenced, freeing it would create a dangling + * pointer. + */ + /* TODO: it could just be an Assert? */ + if (path->ref_count != 0) + { + ereport(WARNING, + (errcode(ERRCODE_INTERNAL_ERROR), + errbacktrace(), + errmsg("path node %p of type %d has reference count %d, can not be freed", + path, path->pathtype, path->ref_count))); + return; + } + + /* + * A path referenced in the parent relation's pathlist can't be freed. + * Ideally such a path should have ref_count >= 1. If this happens it + * indicates that the path was not linked somewhere, and yet unlinked + * (usually by free_path()). + */ + if (list_member_ptr(path->parent->pathlist, path)) + { + ereport(WARNING, + (errcode(ERRCODE_INTERNAL_ERROR), + errbacktrace(), + errmsg("path node %p of type %d has reference count %d but is linked in parent's pathlist, can not be freed", + path, path->pathtype, path->ref_count))); + return; + } + + /* Decrement the reference counts of paths referenced by this one. */ + switch (path->pathtype) + { + case T_SeqScan: + case T_IndexScan: + case T_IndexOnlyScan: + /* Simple paths reference no other path. */ + break; + + case T_MergeJoin: + case T_HashJoin: + case T_NestLoop: + { + JoinPath *jpath = (JoinPath *) path; + + unlink_path(&(jpath->outerjoinpath)); + unlink_path(&(jpath->innerjoinpath)); + } + break; + + case T_Append: + case T_MergeAppend: + { + AppendPath *appath = (AppendPath *) path; + ListCell *lc; + + foreach(lc, appath->subpaths) + { + Path *subpath = lfirst(lc); + + /* TODO use a routine to unlink path from list. */ + appath->subpaths = foreach_delete_current(appath->subpaths, lc); + unlink_path(&subpath); + } + } + break; + + case T_Gather: + { + GatherPath *gpath = (GatherPath *) path; + + unlink_path(&(gpath->subpath)); + } + break; + + case T_GatherMerge: + { + GatherMergePath *gmpath = (GatherMergePath *) path; + + unlink_path(&gmpath->subpath); + } + break; + + case T_BitmapHeapScan: + { + BitmapHeapPath *bhpath = (BitmapHeapPath *) path; + + unlink_path(&(bhpath->bitmapqual)); + } + break; + + case T_Material: + { + MaterialPath *mpath = (MaterialPath *) path; + + unlink_path(&(mpath->subpath)); + } + break; + + case T_Memoize: + { + MemoizePath *mpath = (MemoizePath *) path; + + unlink_path(&mpath->subpath); + } + break; + + case T_Result: + { + /* + * All Result paths except ProjectionPath are simple paths + * without any subpath. + */ + if (IsA(path, ProjectionPath)) + { + ProjectionPath *ppath = (ProjectionPath *) path; + + unlink_path(&ppath->subpath); + } + } + break; + + default: + ereport(WARNING, + (errcode(ERRCODE_INTERNAL_ERROR), + errbacktrace(), + errmsg("unrecognized path type %d", path->pathtype))); + break; + } + + /* + * TODO: add_path does not free IndexPaths, but we do that here. Is there + * a hazard? + */ + + /* Now reclaim the memory. */ + pfree(path); +} /***************************************************************************** * PATH NODE CREATION ROUTINES diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h index 6567759595d..c88323a915b 100644 --- a/src/include/nodes/pathnodes.h +++ b/src/include/nodes/pathnodes.h @@ -1797,6 +1797,7 @@ typedef struct Path /* sort ordering of path's output; a List of PathKey nodes; see above */ List *pathkeys; + int ref_count; } Path; /* Macro for extracting a path's parameterization relids; beware double eval */ diff --git a/src/include/optimizer/pathnode.h b/src/include/optimizer/pathnode.h index 60dcdb77e41..22ee1c2261b 100644 --- a/src/include/optimizer/pathnode.h +++ b/src/include/optimizer/pathnode.h @@ -16,6 +16,7 @@ #include "nodes/bitmapset.h" #include "nodes/pathnodes.h" +#include "limits.h" /* @@ -306,6 +307,43 @@ 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); +extern void free_path(Path *path); + +static inline void +link_path(Path **path_link, Path *path) +{ + if (path->ref_count < 0) + ereport(WARNING, + (errcode(ERRCODE_INTERNAL_ERROR), + errbacktrace(), + errmsg("path node %p of type %d has negative reference count %d", + path, path->pathtype, path->ref_count))); + + path->ref_count++; + *path_link = path; + Assert(path->ref_count > 0 && path->ref_count <= INT_MAX); +} + +static inline void +unlink_path(Path **path_link) +{ + Path *path = *path_link; + + /* A path to be unlinked should have been linked before. */ + if (path->ref_count < 0) + ereport(WARNING, + (errcode(ERRCODE_INTERNAL_ERROR), + errbacktrace(), + errmsg("path node %p of type %d had negative reference count %d", + path, path->pathtype, path->ref_count))); + + path->ref_count--; + *path_link = NULL; + + /* Free path if none is referencing it. */ + if (path->ref_count == 0) + free_path(path); +} /* * prototypes for relnode.c -- 2.50.0