From 26da69fca825d041485dd8ae85ccbd5e55ad2c33 Mon Sep 17 00:00:00 2001 From: Tomas Vondra Date: Mon, 17 Jun 2024 15:41:28 +0200 Subject: [PATCH v20240617 17/56] a branch of updates around JoinPairInfo 1. rename rels to relids while the "rels" may reference to list of RelOptInfo or Relids. but the later one reference to Relids all the time. 2. Store RestrictInfo to JoinPairInfo.clauses so that we can reuse the left_relids, right_relids which will save us from calling pull_varnos. 3. create bms_nth_member function in bitmapset.c and use it extract_relation_info, the function name is self-documented. 4. pfree the JoinPairInfo array when we are done with that. --- src/backend/nodes/bitmapset.c | 18 +++++++++++ src/backend/statistics/extended_stats.c | 43 ++++++++++++------------- src/backend/statistics/mcv.c | 35 ++++++-------------- src/include/nodes/bitmapset.h | 1 + 4 files changed, 48 insertions(+), 49 deletions(-) diff --git a/src/backend/nodes/bitmapset.c b/src/backend/nodes/bitmapset.c index cd05c642b04..7c1291ae641 100644 --- a/src/backend/nodes/bitmapset.c +++ b/src/backend/nodes/bitmapset.c @@ -772,6 +772,24 @@ bms_num_members(const Bitmapset *a) return result; } +/* + * bms_nth_member - return the nth member, index starts with 0. + */ +int +bms_nth_member(const Bitmapset *a, int i) +{ + int idx, res = -1; + + for (idx = 0; idx <= i; idx++) + { + res = bms_next_member(a, res); + + if (res < 0) + elog(ERROR, "no enough members for %d", i); + } + return res; +} + /* * bms_membership - does a set have zero, one, or multiple members? * diff --git a/src/backend/statistics/extended_stats.c b/src/backend/statistics/extended_stats.c index 3e7a133c047..13042dd63c0 100644 --- a/src/backend/statistics/extended_stats.c +++ b/src/backend/statistics/extended_stats.c @@ -3004,11 +3004,11 @@ statext_try_join_estimates(PlannerInfo *root, List *clauses, int varRelid, } /* - * Information about two joined relations, along with the join clauses between. + * Information about two joined relations, group by clauses by relids. */ typedef struct JoinPairInfo { - Bitmapset *rels; + Bitmapset *relids; List *clauses; } JoinPairInfo; @@ -3071,9 +3071,9 @@ statext_build_join_pairs(PlannerInfo *root, List *clauses, found = false; for (i = 0; i < cnt; i++) { - if (bms_is_subset(rinfo->clause_relids, info[i].rels)) + if (bms_is_subset(rinfo->clause_relids, info[i].relids)) { - info[i].clauses = lappend(info[i].clauses, clause); + info[i].clauses = lappend(info[i].clauses, rinfo); found = true; break; } @@ -3081,14 +3081,17 @@ statext_build_join_pairs(PlannerInfo *root, List *clauses, if (!found) { - info[cnt].rels = rinfo->clause_relids; - info[cnt].clauses = lappend(info[cnt].clauses, clause); + info[cnt].relids = rinfo->clause_relids; + info[cnt].clauses = lappend(info[cnt].clauses, rinfo); cnt++; } } if (cnt == 0) + { + pfree(info); return NULL; + } *npairs = cnt; return info; @@ -3112,7 +3115,6 @@ static RelOptInfo * extract_relation_info(PlannerInfo *root, JoinPairInfo *info, int index, StatisticExtInfo **stat) { - int k; int relid; RelOptInfo *rel; ListCell *lc; @@ -3122,16 +3124,7 @@ extract_relation_info(PlannerInfo *root, JoinPairInfo *info, int index, Assert((index >= 0) && (index <= 1)); - k = -1; - while (index >= 0) - { - k = bms_next_member(info->rels, k); - if (k < 0) - elog(ERROR, "failed to extract relid"); - - relid = k; - index--; - } + relid = bms_nth_member(info->relids, index); rel = find_base_rel(root, relid); @@ -3142,9 +3135,10 @@ extract_relation_info(PlannerInfo *root, JoinPairInfo *info, int index, */ foreach(lc, info->clauses) { - ListCell *lc2; - Node *clause = (Node *) lfirst(lc); - OpExpr *opclause = (OpExpr *) clause; + ListCell *lc2; + RestrictInfo *rinfo = (RestrictInfo *) lfirst(lc); + Node *clause = (Node *) rinfo->clause; + OpExpr *opclause = (OpExpr *) clause; /* only opclauses supported for now */ Assert(is_opclause(clause)); @@ -3185,7 +3179,8 @@ extract_relation_info(PlannerInfo *root, JoinPairInfo *info, int index, * relid and maybe keep it as a whole. It should be compatible * because we already checked it when building the join pairs. */ - varnos = pull_varnos(root, arg); + varnos = list_cell_number(opclause->args, lc2) == 0 ? + rinfo->left_relids : rinfo->right_relids; if (relid == bms_singleton_member(varnos)) exprs = lappend(exprs, arg); @@ -3438,8 +3433,9 @@ statext_clauselist_join_selectivity(PlannerInfo *root, List *clauses, */ foreach(lc, info->clauses) { - Node *clause = (Node *) lfirst(lc); - ListCell *lc2; + RestrictInfo *rinfo = (RestrictInfo *) lfirst(lc); + Node *clause = (Node *) rinfo->clause; + ListCell *lc2; listidx = -1; foreach(lc2, clauses) @@ -3461,5 +3457,6 @@ statext_clauselist_join_selectivity(PlannerInfo *root, List *clauses, } } + pfree(info); return s; } diff --git a/src/backend/statistics/mcv.c b/src/backend/statistics/mcv.c index 3169022cd6d..8b09bf16662 100644 --- a/src/backend/statistics/mcv.c +++ b/src/backend/statistics/mcv.c @@ -2214,8 +2214,7 @@ mcv_combine_extended(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2, MCVList *mcv1, *mcv2; - int idx, - i, + int i, j; Selectivity s = 0; @@ -2306,25 +2305,15 @@ mcv_combine_extended(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2, indexes2 = (int *) palloc(sizeof(int) * list_length(clauses)); reverse = (bool *) palloc(sizeof(bool) * list_length(clauses)); - idx = 0; - foreach(lc, clauses) + foreach (lc, clauses) { - Node *clause = (Node *) lfirst(lc); + RestrictInfo *rinfo = (RestrictInfo *) lfirst(lc); + Node *clause = (Node *) rinfo->clause; OpExpr *opexpr; Node *expr1, *expr2; - Bitmapset *relids1, - *relids2; - /* - * Strip the RestrictInfo node, get the actual clause. - * - * XXX Not sure if we need to care about removing other node types too - * (e.g. RelabelType etc.). statext_is_supported_join_clause matches - * this, but maybe we need to relax it? - */ - if (IsA(clause, RestrictInfo)) - clause = (Node *) ((RestrictInfo *) clause)->clause; + int idx = list_cell_number(clauses, lc); opexpr = (OpExpr *) clause; @@ -2338,12 +2327,8 @@ mcv_combine_extended(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2, expr1 = linitial(opexpr->args); expr2 = lsecond(opexpr->args); - /* determine order of clauses (rel1 op rel2) or (rel2 op rel1) */ - relids1 = pull_varnos(root, expr1); - relids2 = pull_varnos(root, expr2); - - if ((bms_singleton_member(relids1) == rel1->relid) && - (bms_singleton_member(relids2) == rel2->relid)) + if ((bms_singleton_member(rinfo->left_relids) == rel1->relid) && + (bms_singleton_member(rinfo->right_relids) == rel2->relid)) { Oid collid; @@ -2358,8 +2343,8 @@ mcv_combine_extended(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2, exprs1 = lappend(exprs1, expr1); exprs2 = lappend(exprs2, expr2); } - else if ((bms_singleton_member(relids2) == rel1->relid) && - (bms_singleton_member(relids1) == rel2->relid)) + else if ((bms_singleton_member(rinfo->right_relids) == rel1->relid) && + (bms_singleton_member(rinfo->left_relids) == rel2->relid)) { Oid collid; @@ -2383,8 +2368,6 @@ mcv_combine_extended(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2, Assert((indexes2[idx] >= 0) && (indexes2[idx] < bms_num_members(stat2->keys) + list_length(stat2->exprs))); - - idx++; } /* diff --git a/src/include/nodes/bitmapset.h b/src/include/nodes/bitmapset.h index 283bea5ea96..8d32e7a2447 100644 --- a/src/include/nodes/bitmapset.h +++ b/src/include/nodes/bitmapset.h @@ -110,6 +110,7 @@ extern bool bms_nonempty_difference(const Bitmapset *a, const Bitmapset *b); extern int bms_singleton_member(const Bitmapset *a); extern bool bms_get_singleton_member(const Bitmapset *a, int *member); extern int bms_num_members(const Bitmapset *a); +extern int bms_nth_member(const Bitmapset *a, int i); /* optimized tests when we don't need to know exact membership count: */ extern BMS_Membership bms_membership(const Bitmapset *a); -- 2.45.2