From b62fdc13f0f01e1f311633882542558a26e7f9ec Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 27 Jul 1999 06:23:12 +0000 Subject: [PATCH] Correct bug in best_innerjoin(): it should check all the rels that the inner path needs to join to, but it was only checking for the first one. Failure could only have been observed with an OR-clause that mentions 3 or more tables, and then only if the bogus path was actually selected as cheapest ... --- src/backend/optimizer/path/joinpath.c | 19 ++++--- src/backend/optimizer/path/joinrels.c | 74 ++++++++++++++------------- src/include/optimizer/paths.h | 24 +++++---- 3 files changed, 64 insertions(+), 53 deletions(-) diff --git a/src/backend/optimizer/path/joinpath.c b/src/backend/optimizer/path/joinpath.c index 9b5c226b55..667cdce4f2 100644 --- a/src/backend/optimizer/path/joinpath.c +++ b/src/backend/optimizer/path/joinpath.c @@ -7,7 +7,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/optimizer/path/joinpath.c,v 1.41 1999/07/16 04:59:15 momjian Exp $ + * $Header: /cvsroot/pgsql/src/backend/optimizer/path/joinpath.c,v 1.42 1999/07/27 06:23:12 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -147,13 +147,14 @@ update_rels_pathlist_for_joins(Query *root, List *joinrels) /* * best_innerjoin * Find the cheapest index path that has already been identified by - * (indexable_joinclauses) as being a possible inner path for the given - * outer relation in a nestloop join. + * indexable_joinclauses() as being a possible inner path for the given + * outer relation(s) in a nestloop join. * - * 'join_paths' is a list of join nodes - * 'outer_relid' is the relid of the outer join relation + * 'join_paths' is a list of potential inner indexscan join paths + * 'outer_relids' is the relid list of the outer join relation * - * Returns the pathnode of the selected path. + * Returns the pathnode of the best path, or NULL if there's no + * usable path. */ static Path * best_innerjoin(List *join_paths, Relids outer_relids) @@ -165,7 +166,11 @@ best_innerjoin(List *join_paths, Relids outer_relids) { Path *path = (Path *) lfirst(join_path); - if (intMember(lfirsti(path->joinid), outer_relids) && + /* path->joinid is the set of base rels that must be part of + * outer_relids in order to use this inner path, because those + * rels are used in the index join quals of this inner path. + */ + if (is_subset(path->joinid, outer_relids) && (cheapest == NULL || path_is_cheaper(path, cheapest))) cheapest = path; diff --git a/src/backend/optimizer/path/joinrels.c b/src/backend/optimizer/path/joinrels.c index 9fba28759a..1eac677074 100644 --- a/src/backend/optimizer/path/joinrels.c +++ b/src/backend/optimizer/path/joinrels.c @@ -7,7 +7,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/optimizer/path/joinrels.c,v 1.37 1999/07/16 04:59:15 momjian Exp $ + * $Header: /cvsroot/pgsql/src/backend/optimizer/path/joinrels.c,v 1.38 1999/07/27 06:23:12 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -21,8 +21,6 @@ #include "optimizer/tlist.h" static List *new_joininfo_list(List *joininfo_list, Relids join_relids); -static bool nonoverlap_sets(List *s1, List *s2); -static bool is_subset(List *s1, List *s2); static void set_joinrel_size(RelOptInfo *joinrel, RelOptInfo *outer_rel, RelOptInfo *inner_rel, JoinInfo *jinfo); static RelOptInfo *make_join_rel(RelOptInfo *outer_rel, RelOptInfo *inner_rel, @@ -373,8 +371,8 @@ new_joininfo_list(List *joininfo_list, Relids join_relids) RelOptInfo * get_cheapest_complete_rel(List *join_rel_list) { - List *xrel = NIL; RelOptInfo *final_rel = NULL; + List *xrel; /* * find the relations that have no further joins, i.e., its joininfos @@ -383,8 +381,8 @@ get_cheapest_complete_rel(List *join_rel_list) foreach(xrel, join_rel_list) { RelOptInfo *rel = (RelOptInfo *) lfirst(xrel); - List *xjoininfo = NIL; bool final = true; + List *xjoininfo; foreach(xjoininfo, rel->joininfo) { @@ -405,36 +403,6 @@ get_cheapest_complete_rel(List *join_rel_list) return final_rel; } -static bool -nonoverlap_sets(List *s1, List *s2) -{ - List *x = NIL; - - foreach(x, s1) - { - int e = lfirsti(x); - - if (intMember(e, s2)) - return false; - } - return true; -} - -static bool -is_subset(List *s1, List *s2) -{ - List *x = NIL; - - foreach(x, s1) - { - int e = lfirsti(x); - - if (!intMember(e, s2)) - return false; - } - return true; -} - static void set_joinrel_size(RelOptInfo *joinrel, RelOptInfo *outer_rel, RelOptInfo *inner_rel, JoinInfo *jinfo) { @@ -466,3 +434,39 @@ set_joinrel_size(RelOptInfo *joinrel, RelOptInfo *outer_rel, RelOptInfo *inner_r joinrel->tuples = ntuples; } + +/* + * Subset-inclusion tests on integer lists. + * + * XXX these probably ought to be in nodes/list.c or some such place. + */ + +bool +nonoverlap_sets(List *s1, List *s2) +{ + List *x; + + foreach(x, s1) + { + int e = lfirsti(x); + + if (intMember(e, s2)) + return false; + } + return true; +} + +bool +is_subset(List *s1, List *s2) +{ + List *x; + + foreach(x, s1) + { + int e = lfirsti(x); + + if (!intMember(e, s2)) + return false; + } + return true; +} diff --git a/src/include/optimizer/paths.h b/src/include/optimizer/paths.h index 75d9e32845..f074f1eee1 100644 --- a/src/include/optimizer/paths.h +++ b/src/include/optimizer/paths.h @@ -1,13 +1,13 @@ /*------------------------------------------------------------------------- * * paths.h - * prototypes for various files in optimizer/paths (were separate - * header files + * prototypes for various files in optimizer/path (were separate + * header files) * * * Copyright (c) 1994, Regents of the University of California * - * $Id: paths.h,v 1.32 1999/07/27 03:51:01 tgl Exp $ + * $Id: paths.h,v 1.33 1999/07/27 06:23:11 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -17,12 +17,12 @@ #include "nodes/relation.h" /* - * allpaths.h + * allpaths.c */ extern RelOptInfo *make_one_rel(Query *root, List *rels); /* - * indxpath.h + * indxpath.c * routines to generate index paths */ extern List *create_index_paths(Query *root, RelOptInfo *rel, List *indices, @@ -31,26 +31,26 @@ extern List *create_index_paths(Query *root, RelOptInfo *rel, List *indices, extern List *expand_indexqual_conditions(List *indexquals); /* - * joinpath.h + * joinpath.c * routines to create join paths */ extern void update_rels_pathlist_for_joins(Query *root, List *joinrels); /* - * orindxpath.h + * orindxpath.c */ extern List *create_or_index_paths(Query *root, RelOptInfo *rel, List *clauses); /* - * hashutils.h + * hashutils.c * routines to deal with hash keys and clauses */ extern List *group_clauses_by_hashop(List *restrictinfo_list, Relids inner_relids); /* - * joinutils.h + * joinutils.c * generic join method key/clause routines */ extern bool order_joinkeys_by_pathkeys(List *pathkeys, @@ -65,7 +65,7 @@ extern List *new_join_pathkeys(List *outer_pathkeys, List *join_rel_tlist, List *joinclauses); /* - * mergeutils.h + * mergeutils.c * routines to deal with merge keys and clauses */ extern List *group_clauses_by_order(List *restrictinfo_list, @@ -74,7 +74,7 @@ extern MergeInfo *match_order_mergeinfo(PathOrder *ordering, List *mergeinfo_list); /* - * joinrels.h + * joinrels.c * routines to determine which relations to join */ extern List *make_rels_by_joins(Query *root, List *old_rels); @@ -83,6 +83,8 @@ extern List *make_rels_by_clause_joins(Query *root, RelOptInfo *old_rel, extern List *make_rels_by_clauseless_joins(RelOptInfo *old_rel, List *inner_rels); extern RelOptInfo *get_cheapest_complete_rel(List *join_rel_list); +extern bool nonoverlap_sets(List *s1, List *s2); +extern bool is_subset(List *s1, List *s2); /* * prototypes for path/prune.c -- GitLab