From 3cbcb78a3dc1f716c1dab3dc7cac3c405b210533 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Fri, 18 Feb 2000 23:47:31 +0000 Subject: [PATCH] Plug some more memory leaks in the planner. It still leaks like a sieve, but this is as good as it'll get for this release... --- src/backend/optimizer/path/joinpath.c | 59 ++++++++++++++++--------- src/backend/optimizer/path/pathkeys.c | 18 ++++++-- src/backend/optimizer/plan/createplan.c | 15 ++++--- src/backend/optimizer/util/pathnode.c | 6 ++- src/backend/optimizer/util/relnode.c | 7 +-- src/include/nodes/relation.h | 33 +++++++++----- 6 files changed, 89 insertions(+), 49 deletions(-) diff --git a/src/backend/optimizer/path/joinpath.c b/src/backend/optimizer/path/joinpath.c index 091e2e40c7..9261437496 100644 --- a/src/backend/optimizer/path/joinpath.c +++ b/src/backend/optimizer/path/joinpath.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/optimizer/path/joinpath.c,v 1.52 2000/02/15 20:49:17 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/optimizer/path/joinpath.c,v 1.53 2000/02/18 23:47:19 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -171,9 +171,13 @@ sort_inner_and_outer(Query *root, List *merge_pathkeys; /* Make a mergeclause list with this guy first. */ - curclause_list = lcons(restrictinfo, - lremove(restrictinfo, - listCopy(mergeclause_list))); + if (i != mergeclause_list) + curclause_list = lcons(restrictinfo, + lremove(restrictinfo, + listCopy(mergeclause_list))); + else + curclause_list = mergeclause_list; /* no work at first one... */ + /* Build sort pathkeys for both sides. * * Note: it's possible that the cheapest paths will already be @@ -203,7 +207,7 @@ sort_inner_and_outer(Query *root, innerrel->cheapest_total_path, restrictlist, merge_pathkeys, - get_actual_clauses(curclause_list), + curclause_list, outerkeys, innerkeys)); } @@ -265,6 +269,7 @@ match_unsorted_outer(Query *root, List *trialsortkeys; Path *cheapest_startup_inner; Path *cheapest_total_inner; + int num_mergeclauses; int clausecnt; /* @@ -325,7 +330,7 @@ match_unsorted_outer(Query *root, innerrel->cheapest_total_path, restrictlist, merge_pathkeys, - get_actual_clauses(mergeclauses), + mergeclauses, NIL, innersortkeys)); @@ -337,10 +342,12 @@ match_unsorted_outer(Query *root, trialsortkeys = listCopy(innersortkeys); /* modifiable copy */ cheapest_startup_inner = NULL; cheapest_total_inner = NULL; + num_mergeclauses = length(mergeclauses); - for (clausecnt = length(mergeclauses); clausecnt > 0; clausecnt--) + for (clausecnt = num_mergeclauses; clausecnt > 0; clausecnt--) { Path *innerpath; + List *newclauses = NIL; /* Look for an inner path ordered well enough to merge with * the first 'clausecnt' mergeclauses. NB: trialsortkeys list @@ -356,10 +363,11 @@ match_unsorted_outer(Query *root, TOTAL_COST) < 0)) { /* Found a cheap (or even-cheaper) sorted path */ - List *newclauses; - - newclauses = ltruncate(clausecnt, - get_actual_clauses(mergeclauses)); + if (clausecnt < num_mergeclauses) + newclauses = ltruncate(clausecnt, + listCopy(mergeclauses)); + else + newclauses = mergeclauses; add_path(joinrel, (Path *) create_mergejoin_path(joinrel, outerpath, @@ -383,10 +391,17 @@ match_unsorted_outer(Query *root, /* Found a cheap (or even-cheaper) sorted path */ if (innerpath != cheapest_total_inner) { - List *newclauses; - - newclauses = ltruncate(clausecnt, - get_actual_clauses(mergeclauses)); + /* Avoid rebuilding clause list if we already made one; + * saves memory in big join trees... + */ + if (newclauses == NIL) + { + if (clausecnt < num_mergeclauses) + newclauses = ltruncate(clausecnt, + listCopy(mergeclauses)); + else + newclauses = mergeclauses; + } add_path(joinrel, (Path *) create_mergejoin_path(joinrel, outerpath, @@ -461,7 +476,7 @@ match_unsorted_inner(Query *root, innerpath, restrictlist, merge_pathkeys, - get_actual_clauses(mergeclauses), + mergeclauses, outersortkeys, NIL)); /* @@ -487,7 +502,7 @@ match_unsorted_inner(Query *root, innerpath, restrictlist, merge_pathkeys, - get_actual_clauses(mergeclauses), + mergeclauses, NIL, NIL)); @@ -505,7 +520,7 @@ match_unsorted_inner(Query *root, innerpath, restrictlist, merge_pathkeys, - get_actual_clauses(mergeclauses), + mergeclauses, NIL, NIL)); } @@ -552,6 +567,7 @@ hash_inner_and_outer(Query *root, Var *left, *right, *inner; + List *hashclauses; Selectivity innerdisbursion; if (restrictinfo->hashjoinoperator == InvalidOid) @@ -572,6 +588,9 @@ hash_inner_and_outer(Query *root, else continue; /* no good for these input relations */ + /* always a one-element list of hash clauses */ + hashclauses = lcons(restrictinfo, NIL); + /* estimate disbursion of inner var for costing purposes */ innerdisbursion = estimate_disbursion(root, inner); @@ -585,7 +604,7 @@ hash_inner_and_outer(Query *root, outerrel->cheapest_total_path, innerrel->cheapest_total_path, restrictlist, - lcons(clause, NIL), + hashclauses, innerdisbursion)); if (outerrel->cheapest_startup_path != outerrel->cheapest_total_path) add_path(joinrel, (Path *) @@ -593,7 +612,7 @@ hash_inner_and_outer(Query *root, outerrel->cheapest_startup_path, innerrel->cheapest_total_path, restrictlist, - lcons(clause, NIL), + hashclauses, innerdisbursion)); } } diff --git a/src/backend/optimizer/path/pathkeys.c b/src/backend/optimizer/path/pathkeys.c index b578e33f5c..d5fbf82eb5 100644 --- a/src/backend/optimizer/path/pathkeys.c +++ b/src/backend/optimizer/path/pathkeys.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/optimizer/path/pathkeys.c,v 1.19 2000/02/15 20:49:17 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/optimizer/path/pathkeys.c,v 1.20 2000/02/18 23:47:19 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -778,6 +778,7 @@ make_pathkeys_for_mergeclauses(Query *root, Node *key; Oid sortop; PathKeyItem *item; + List *pathkey; Assert(restrictinfo->mergejoinoperator != InvalidOid); @@ -791,10 +792,21 @@ make_pathkeys_for_mergeclauses(Query *root, key = (Node *) get_leftop(restrictinfo->clause); sortop = restrictinfo->left_sortop; /* - * Add a pathkey sublist for this sort item + * Find pathkey sublist for this sort item. We expect to find + * the canonical set including the mergeclause's left and right + * sides; if we get back just the one item, something is rotten. */ item = makePathKeyItem(key, sortop); - pathkeys = lappend(pathkeys, make_canonical_pathkey(root, item)); + pathkey = make_canonical_pathkey(root, item); + Assert(length(pathkey) > 1); + /* + * Since the item we just made is not in the returned canonical set, + * we can free it --- this saves a useful amount of storage in a + * big join tree. + */ + pfree(item); + + pathkeys = lappend(pathkeys, pathkey); } return pathkeys; diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c index 55af1426fd..8d4c836fa5 100644 --- a/src/backend/optimizer/plan/createplan.c +++ b/src/backend/optimizer/plan/createplan.c @@ -10,7 +10,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/optimizer/plan/createplan.c,v 1.85 2000/02/15 20:49:18 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/optimizer/plan/createplan.c,v 1.86 2000/02/18 23:47:21 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -626,13 +626,14 @@ create_mergejoin_node(MergePath *best_path, *mergeclauses; MergeJoin *join_node; + mergeclauses = get_actual_clauses(best_path->path_mergeclauses); + /* * Remove the mergeclauses from the list of join qual clauses, * leaving the list of quals that must be checked as qpquals. * Set those clauses to contain INNER/OUTER var references. */ - qpqual = join_references(set_difference(clauses, - best_path->path_mergeclauses), + qpqual = join_references(set_difference(clauses, mergeclauses), outer_tlist, inner_tlist, (Index) 0); @@ -641,7 +642,7 @@ create_mergejoin_node(MergePath *best_path, * Now set the references in the mergeclauses and rearrange them so * that the outer variable is always on the left. */ - mergeclauses = switch_outer(join_references(best_path->path_mergeclauses, + mergeclauses = switch_outer(join_references(mergeclauses, outer_tlist, inner_tlist, (Index) 0)); @@ -692,14 +693,14 @@ create_hashjoin_node(HashPath *best_path, * We represent it as a list anyway, for convenience with routines * that want to work on lists of clauses. */ + hashclauses = get_actual_clauses(best_path->path_hashclauses); /* * Remove the hashclauses from the list of join qual clauses, * leaving the list of quals that must be checked as qpquals. * Set those clauses to contain INNER/OUTER var references. */ - qpqual = join_references(set_difference(clauses, - best_path->path_hashclauses), + qpqual = join_references(set_difference(clauses, hashclauses), outer_tlist, inner_tlist, (Index) 0); @@ -708,7 +709,7 @@ create_hashjoin_node(HashPath *best_path, * Now set the references in the hashclauses and rearrange them so * that the outer variable is always on the left. */ - hashclauses = switch_outer(join_references(best_path->path_hashclauses, + hashclauses = switch_outer(join_references(hashclauses, outer_tlist, inner_tlist, (Index) 0)); diff --git a/src/backend/optimizer/util/pathnode.c b/src/backend/optimizer/util/pathnode.c index ba991388de..6f6ef23128 100644 --- a/src/backend/optimizer/util/pathnode.c +++ b/src/backend/optimizer/util/pathnode.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/optimizer/util/pathnode.c,v 1.60 2000/02/15 20:49:20 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/optimizer/util/pathnode.c,v 1.61 2000/02/18 23:47:30 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -416,7 +416,8 @@ create_nestloop_path(RelOptInfo *joinrel, * 'inner_path' is the inner path * 'restrict_clauses' are the RestrictInfo nodes to apply at the join * 'pathkeys' are the path keys of the new join path - * 'mergeclauses' are the applicable join/restriction clauses + * 'mergeclauses' are the RestrictInfo nodes to use as merge clauses + * (this should be a subset of the restrict_clauses list) * 'outersortkeys' are the sort varkeys for the outer relation * 'innersortkeys' are the sort varkeys for the inner relation * @@ -473,6 +474,7 @@ create_mergejoin_path(RelOptInfo *joinrel, * 'inner_path' is the cheapest inner path * 'restrict_clauses' are the RestrictInfo nodes to apply at the join * 'hashclauses' is a list of the hash join clause (always a 1-element list) + * (this should be a subset of the restrict_clauses list) * 'innerdisbursion' is an estimate of the disbursion of the inner hash key * */ diff --git a/src/backend/optimizer/util/relnode.c b/src/backend/optimizer/util/relnode.c index f11dd60d24..694a1b905e 100644 --- a/src/backend/optimizer/util/relnode.c +++ b/src/backend/optimizer/util/relnode.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/optimizer/util/relnode.c,v 1.24 2000/02/15 20:49:21 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/optimizer/util/relnode.c,v 1.25 2000/02/18 23:47:31 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -345,11 +345,8 @@ subbuild_joinrel_restrictlist(RelOptInfo *joinrel, foreach(xjoininfo, joininfo_list) { JoinInfo *joininfo = (JoinInfo *) lfirst(xjoininfo); - Relids new_unjoined_relids; - new_unjoined_relids = set_differencei(joininfo->unjoined_relids, - joinrel->relids); - if (new_unjoined_relids == NIL) + if (is_subseti(joininfo->unjoined_relids, joinrel->relids)) { /* * Clauses in this JoinInfo list become restriction clauses diff --git a/src/include/nodes/relation.h b/src/include/nodes/relation.h index 3efdaa5b32..ac6dc764ce 100644 --- a/src/include/nodes/relation.h +++ b/src/include/nodes/relation.h @@ -7,7 +7,7 @@ * Portions Copyright (c) 1996-2000, PostgreSQL, Inc * Portions Copyright (c) 1994, Regents of the University of California * - * $Id: relation.h,v 1.44 2000/02/15 20:49:25 tgl Exp $ + * $Id: relation.h,v 1.45 2000/02/18 23:47:17 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -237,6 +237,8 @@ typedef struct Path * are usable as indexquals (as determined by indxpath.c) may appear here. * NOTE that the semantics of the top-level list in 'indexqual' is OR * combination, while the sublists are implicitly AND combinations! + * Also note that indexquals lists do not contain RestrictInfo nodes, + * just bare clause expressions. * * 'indexscandir' is one of: * ForwardScanDirection: forward scan of an ordered index @@ -293,32 +295,39 @@ typedef JoinPath NestPath; /* * A mergejoin path has these fields. * + * path_mergeclauses lists the clauses (in the form of RestrictInfos) + * that will be used in the merge. (Before 7.0, this was a list of + * bare clause expressions, but we can save on list memory by leaving + * it in the form of a RestrictInfo list.) + * * Note that the mergeclauses are a subset of the parent relation's * restriction-clause list. Any join clauses that are not mergejoinable * appear only in the parent's restrict list, and must be checked by a * qpqual at execution time. + * + * outersortkeys (resp. innersortkeys) is NIL if the outer path + * (resp. inner path) is already ordered appropriately for the + * mergejoin. If it is not NIL then it is a PathKeys list describing + * the ordering that must be created by an explicit sort step. */ typedef struct MergePath { JoinPath jpath; - List *path_mergeclauses; /* join clauses used for merge */ - /* - * outersortkeys (resp. innersortkeys) is NIL if the outer path - * (resp. inner path) is already ordered appropriately for the - * mergejoin. If it is not NIL then it is a PathKeys list describing - * the ordering that must be created by an explicit sort step. - */ - List *outersortkeys; - List *innersortkeys; + List *path_mergeclauses; /* join clauses to be used for merge */ + List *outersortkeys; /* keys for explicit sort, if any */ + List *innersortkeys; /* keys for explicit sort, if any */ } MergePath; /* * A hashjoin path has these fields. * * The remarks above for mergeclauses apply for hashclauses as well. - * However, hashjoin does not care what order its inputs appear in, - * so we have no need for sortkeys. + * (But note that path_hashclauses will always be a one-element list, + * since we only hash on one hashable clause.) + * + * Hashjoin does not care what order its inputs appear in, so we have + * no need for sortkeys. */ typedef struct HashPath