diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c index 873a764..7f78704 100644 --- a/src/backend/optimizer/path/allpaths.c +++ b/src/backend/optimizer/path/allpaths.c @@ -67,6 +67,7 @@ join_search_hook_type join_search_hook = NULL; static void set_base_rel_consider_startup(PlannerInfo *root); static void set_base_rel_sizes(PlannerInfo *root); static void set_base_rel_pathlists(PlannerInfo *root); +static void mark_useful_foreign_keys(PlannerInfo *root); static void set_rel_size(PlannerInfo *root, RelOptInfo *rel, Index rti, RangeTblEntry *rte); static void set_rel_pathlist(PlannerInfo *root, RelOptInfo *rel, @@ -168,6 +169,7 @@ make_one_rel(PlannerInfo *root, List *joinlist) */ set_base_rel_sizes(root); set_base_rel_pathlists(root); + mark_useful_foreign_keys(root); /* * Generate access paths for the entire join tree. @@ -301,6 +303,79 @@ set_base_rel_pathlists(PlannerInfo *root) } /* + * mark_useful_foreign_keys + * Determine which of the foreign keys of each relation have a remote + * possibility of being referenced by some join in the query. Any of which + * that have no chance of being referenced are unlikely to be of any use + * during planning, and can most likely be ignored for most cases. + */ +static void +mark_useful_foreign_keys(PlannerInfo *root) +{ + HTAB *htab; + HASHCTL hash_ctl; + Index rti; + ListCell *lc; + + memset(&hash_ctl, 0, sizeof(hash_ctl)); + hash_ctl.keysize = sizeof(Oid); + hash_ctl.entrysize = sizeof(Oid); + hash_ctl.hcxt = CurrentMemoryContext; + htab = hash_create("mark_useful_foreign_keys", + root->simple_rel_array_size, + &hash_ctl, + HASH_ELEM | HASH_BLOBS | HASH_CONTEXT); + + /* build a hash table with all rel OIDs that are in the query */ + for (rti = 1; rti < root->simple_rel_array_size; rti++) + { + RelOptInfo *rel = root->simple_rel_array[rti]; + RangeTblEntry *rte; + + if (rel == NULL) + continue; + + /* ignore RTEs that are "other rels" */ + if (rel->reloptkind != RELOPT_BASEREL) + continue; + + rte = root->simple_rte_array[rti]; + + (void) hash_search(htab, (void *) &(rte->relid), HASH_ENTER, NULL); + } + + /* XXX do we need to add entries for the append_rel_list here? */ + + /* + * Now go over each rel and set each foreign key's 'possibleRef' according + * to if the FKs confrelid was found to be in the query or not. + */ + for (rti = 1; rti < root->simple_rel_array_size; rti++) + { + RelOptInfo *rel = root->simple_rel_array[rti]; + + if (rel == NULL) + continue; + + /* ignore RTEs that are "other rels" */ + if (rel->reloptkind != RELOPT_BASEREL) + continue; + + foreach(lc, rel->fkeylist) + { + ForeignKeyOptInfo *fkinfo = (ForeignKeyOptInfo *) lfirst(lc); + bool found; + hash_search(htab, (void *) &(fkinfo->confrelid), HASH_FIND, &found); + if (found) + fkinfo->possibleRef = true; + } + } + + /* Done with the hash table. */ + hash_destroy(htab); +} + +/* * set_rel_size * Set size estimates for a base relation */ diff --git a/src/backend/optimizer/path/costsize.c b/src/backend/optimizer/path/costsize.c index fcb1873..0dff50f 100644 --- a/src/backend/optimizer/path/costsize.c +++ b/src/backend/optimizer/path/costsize.c @@ -3913,16 +3913,19 @@ quals_match_foreign_key(PlannerInfo *root, ForeignKeyOptInfo *fkinfo, Bitmapset *fkmatches = NULL; /* - * Loop over each column of the foreign key and build a bitmapset - * of each joinqual which matches. Note that we don't stop when we find - * the first match, as the expression could be duplicated in the - * joinquals, and we want to generate a bitmapset which has bits set for - * every matching join qual. + * Loop over each column of the foreign key and build a 0-based bitmapset + * of the list position of each joinqual which matches the foreign key. We + * also maintain the fkmatches bitmapsets to mark which keys have been + * matched. We must do this rather than maintain a simple count of the + * number of keys matched in order to determine at the end of all the keys + * were matched, a count is not good enough as it's possible to match to + * the same key more than once with duplicated join conditions. */ for (i = 0; i < nkeys; i++) { ListCell *lc; int quallstidx = -1; + bool found = false; foreach(lc, joinquals) { @@ -3944,18 +3947,21 @@ quals_match_foreign_key(PlannerInfo *root, ForeignKeyOptInfo *fkinfo, if (i > 0 && bms_is_member(quallstidx, qualmatches)) continue; - /* - * Here since 'usefulquals' only contains bitmap indexes for quals - * of type "var op var" we can safely skip checking this. - */ rinfo = (RestrictInfo *) lfirst(lc); clause = (OpExpr *) rinfo->clause; + /* only OpExprs are useful for consideration */ + if (!IsA(clause, OpExpr)) + continue; + /* * If the operator does not match then there's little point in - * checking the operands. + * checking the operands. We must also check for the commutative + * operator since the condition could be the reverse of the + * foreign key relationship. */ - if (clause->opno != fkinfo->conpfeqop[i]) + if (clause->opno != fkinfo->conpfeqop[i] || + get_commutator(clause->opno) != fkinfo->conpfeqop[i]) continue; leftvar = (Var *) get_leftop((Expr *) clause); @@ -3970,8 +3976,8 @@ quals_match_foreign_key(PlannerInfo *root, ForeignKeyOptInfo *fkinfo, * member of the eclass as rinfo's operands may not belong to the * foreign key. For efficient tracking of which Vars we've found, * since we're only tracking 2 Vars, we use a bitmask. We can - * safely finish searching when both of the least significant bits - * are set. + * safely finish searching when the two least significant bits are + * set. */ if (rinfo->parent_ec) { @@ -4004,6 +4010,7 @@ quals_match_foreign_key(PlannerInfo *root, ForeignKeyOptInfo *fkinfo, { qualmatches = bms_add_member(qualmatches, quallstidx); fkmatches = bms_add_member(fkmatches, i); + found = true; break; } } @@ -4023,6 +4030,7 @@ quals_match_foreign_key(PlannerInfo *root, ForeignKeyOptInfo *fkinfo, { qualmatches = bms_add_member(qualmatches, quallstidx); fkmatches = bms_add_member(fkmatches, i); + found = true; } else if ((foreignrel->relid == rightvar->varno) && (fkrel->relid == leftvar->varno) && @@ -4031,9 +4039,18 @@ quals_match_foreign_key(PlannerInfo *root, ForeignKeyOptInfo *fkinfo, { qualmatches = bms_add_member(qualmatches, quallstidx); fkmatches = bms_add_member(fkmatches, i); + found = true; } } } + + /* + * If any key was not found, then we need not bother searching any + * further. We'll just break out of the loop and let the code below + * clean up for us. + */ + if (!found) + break; } /* can't find more matches than columns in the foreign key */ @@ -4099,6 +4116,13 @@ find_best_foreign_key_quals(PlannerInfo *root, RelOptInfo *fkrel, Bitmapset *qualsmatched; /* + * Skip any foreign keys where we determined the referenced rel was + * not part of this query. + */ + if (!fkinfo->possibleRef) + continue; + + /* * We make no attempt in checking that this foreign key actually * references 'foreignrel', the reasoning here is that we may be able * to match the foreign key to an eclass member Var of a RestrictInfo @@ -4164,8 +4188,13 @@ clauselist_join_selectivity(PlannerInfo *root, List *joinquals, int outermatches; /* - * check which quals are matched by a foreign key referencing the - * innerrel. + * Attempt to find the "best matching" foreign key which backs up + * the join condition. Here we define the "best match" as the + * foreign key with the most keys. We must perform this search + * twice; once for FKs where outerrel referencing innerrel, and + * also the reverse of that.We can perform simply by calling + * find_best_foreign_key_quals() twice and swapping the input + * parameters. */ outermatches = find_best_foreign_key_quals(root, outerrel, innerrel, joinquals, &outer2inner); @@ -4175,15 +4204,25 @@ clauselist_join_selectivity(PlannerInfo *root, List *joinquals, outerrel, joinquals, &inner2outer); /* - * did we find any matches at all? If so we need to see which one is - * the best/longest match + * Check if we found any matches at all and use that match. It's + * perhaps unlikely that a foreign key exists in both directions, + * but we'll handle that case, and just take the longer of the + * two. If both matches happen to be the same length, then we'll + * prefer the inner2outer match, there's little reason for this, + * we simply need to pick something. */ if (outermatches != 0 || innermatches != 0) { double referenced_tuples; bool overlap; - /* either could be zero, but not both. */ + /* + * Determine the best match and add the quals belonging to the + * match to the bitmap of quals found so far. We'll also check + * for any quals which were already matched to a foreign key + * by an earlier check as this determines if we must take into + * account the existing selectivity or start from scratch. + */ if (outermatches < innermatches) { overlap = bms_overlap(foundfkquals, inner2outer); @@ -4199,14 +4238,10 @@ clauselist_join_selectivity(PlannerInfo *root, List *joinquals, referenced_tuples = Max(innerrel->tuples, 1.0); } - /* - * XXX should we ignore these overlapping matches? - * Or perhaps take the Max() or Min()? - */ if (overlap) { if (jointype == JOIN_SEMI || jointype == JOIN_ANTI) - sel = Min(sel,Min(1.0 / (outerrel->tuples / Max(innerrel->tuples, 1.0)), 1.0)); + sel = Min(sel, Min(1.0 / (outerrel->tuples / Max(innerrel->tuples, 1.0)), 1.0)); else sel = Min(sel, 1.0 / referenced_tuples); } diff --git a/src/include/nodes/relation.h b/src/include/nodes/relation.h index 45739c3..ba359be 100644 --- a/src/include/nodes/relation.h +++ b/src/include/nodes/relation.h @@ -642,7 +642,7 @@ typedef struct ForeignKeyOptInfo int *conkeys; /* attnums of columns in the constrained table */ int *confkeys; /* attnums of columns in the referenced table */ Oid *conpfeqop; /* OIDs of equality operators used by the FK */ - + bool possibleRef; /* confrelid is in this query */ } ForeignKeyOptInfo; /* diff --git a/src/include/optimizer/paths.h b/src/include/optimizer/paths.h index e0ac451..f3b25e2 100644 --- a/src/include/optimizer/paths.h +++ b/src/include/optimizer/paths.h @@ -76,8 +76,6 @@ extern Expr *adjust_rowcompare_for_index(RowCompareExpr *clause, int indexcol, List **indexcolnos, bool *var_on_left_p); -extern bool has_matching_fkey(RelOptInfo *rel, RelOptInfo *frel, List *clauses, - bool reverse); /* * tidpath.h