public inbox for [email protected]help / color / mirror / Atom feed
[PATCH 4/5] WIP rework tracking of expressions 2+ messages / 2 participants [nested] [flat]
* [PATCH 4/5] WIP rework tracking of expressions @ 2021-02-17 00:02 Tomas Vondra <[email protected]> 0 siblings, 0 replies; 2+ messages in thread From: Tomas Vondra @ 2021-02-17 00:02 UTC (permalink / raw) --- src/backend/statistics/dependencies.c | 136 ++++++++++++++---- src/backend/statistics/extended_stats.c | 73 ++++------ src/backend/statistics/mcv.c | 26 +++- src/backend/statistics/mvdistinct.c | 120 +++++++++------- src/backend/utils/adt/selfuncs.c | 29 +++- .../statistics/extended_stats_internal.h | 11 +- src/include/statistics/statistics.h | 3 +- 7 files changed, 255 insertions(+), 143 deletions(-) diff --git a/src/backend/statistics/dependencies.c b/src/backend/statistics/dependencies.c index 6bf3127bcc..602301b724 100644 --- a/src/backend/statistics/dependencies.c +++ b/src/backend/statistics/dependencies.c @@ -252,7 +252,7 @@ dependency_degree(int numrows, HeapTuple *rows, ExprInfo *exprs, int k, * member easier, and then construct a filtered version with only attnums * referenced by the dependency we validate. */ - attnums = build_attnums_array(attrs, &numattrs); + attnums = build_attnums_array(attrs, exprs->nexprs, &numattrs); attnums_dep = (AttrNumber *) palloc(k * sizeof(AttrNumber)); for (i = 0; i < k; i++) @@ -372,19 +372,46 @@ statext_dependencies_build(int numrows, HeapTuple *rows, k; int numattrs; AttrNumber *attnums; + int nattnums; /* result */ MVDependencies *dependencies = NULL; - /* treat expressions as special attributes with high attnums */ - attrs = add_expressions_to_attributes(attrs, exprs->nexprs); + /* + * Transform the bms into an array of attnums, to make accessing i-th + * member easier. We add the expressions first, represented by negative + * attnums (this is OK, we don't allow statistics on system attributes), + * and then regular attributes. + */ + nattnums = bms_num_members(attrs) + exprs->nexprs; + attnums = (AttrNumber *) palloc(sizeof(AttrNumber) * nattnums); + + numattrs = 0; + + /* treat expressions as attributes with negative attnums */ + for (i = 0; i < exprs->nexprs; i++) + attnums[numattrs++] = -(i+1); /* - * Transform the bms into an array, to make accessing i-th member easier. + * and then regular attributes + * + * XXX Maybe add this in the opposite order, just like in MCV? first + * regular attnums, then exressions. */ - attnums = build_attnums_array(attrs, &numattrs); + k = -1; + while ((k = bms_next_member(attrs, k)) >= 0) + attnums[numattrs++] = k; Assert(numattrs >= 2); + Assert(numattrs == nattnums); + + /* + * Build a new bitmapset of attnums, offset by number of expressions (this + * is needed, because bitmaps can store only non-negative values). + */ + attrs = NULL; + for (i = 0; i < numattrs; i++) + attrs = bms_add_member(attrs, attnums[i] + exprs->nexprs); /* * We'll try build functional dependencies starting from the smallest ones @@ -1374,8 +1401,10 @@ dependencies_clauselist_selectivity(PlannerInfo *root, * We also skip clauses that we already estimated using different types of * statistics (we treat them as incompatible). * - * For expressions, we generate attnums higher than MaxHeapAttributeNumber - * so that we can work with attnums only. + * To handle expressions, we assign them negative attnums, as if it was a + * system attribute (this is fine, as we only allow extended stats on user + * attributes). And then we offset everything by the number of expressions, + * so that we can store the values in a bitmapset. */ listidx = 0; foreach(l, clauses) @@ -1391,13 +1420,12 @@ dependencies_clauselist_selectivity(PlannerInfo *root, { /* * If it's a simple column refrence, just extract the attnum. If - * it's an expression, make sure it's not a duplicate and assign - * a special attnum to it (higher than any regular value). + * it's an expression, assign a negative attnum as if it was a + * system attribute. */ if (dependency_is_compatible_clause(clause, rel->relid, &attnum)) { list_attnums[listidx] = attnum; - clauses_attnums = bms_add_member(clauses_attnums, attnum); } else if (dependency_is_compatible_expression(clause, rel->relid, rel->statlist, @@ -1413,7 +1441,8 @@ dependencies_clauselist_selectivity(PlannerInfo *root, { if (equal(unique_exprs[i], expr)) { - attnum = EXPRESSION_ATTNUM(i); + /* negative attribute number to expression */ + attnum = -(i + 1); break; } } @@ -1421,14 +1450,10 @@ dependencies_clauselist_selectivity(PlannerInfo *root, /* not found in the list, so add it */ if (attnum == InvalidAttrNumber) { - attnum = EXPRESSION_ATTNUM(unique_exprs_cnt); unique_exprs[unique_exprs_cnt++] = expr; - /* shouldn't have seen this attnum yet */ - Assert(!bms_is_member(attnum, clauses_attnums)); - - /* we may add the attnum repeatedly to clauses_attnums */ - clauses_attnums = bms_add_member(clauses_attnums, attnum); + /* after incrementing the value, to get -1, -2, ... */ + attnum = -unique_exprs_cnt; } /* remember which attnum was assigned to this clause */ @@ -1439,6 +1464,37 @@ dependencies_clauselist_selectivity(PlannerInfo *root, listidx++; } + Assert(listidx == list_length(clauses)); + + /* + * Now that we know how many expressions there are, we can offset the + * values just enough to build the bitmapset. + */ + for (i = 0; i < list_length(clauses); i++) + { + AttrNumber attnum; + + /* ignore incompatible or already estimated clauses */ + if (list_attnums[i] == InvalidAttrNumber) + continue; + + /* make sure the attnum is in the expected range */ + Assert(list_attnums[i] >= (-unique_exprs_cnt)); + Assert(list_attnums[i] <= MaxHeapAttributeNumber); + + /* make sure the attnum is not negative */ + attnum = list_attnums[i] + unique_exprs_cnt; + + /* + * Expressions are unique, and so we must not have seen this attnum + * before. + */ + Assert(AttrNumberIsForUserDefinedAttr(list_attnums[i]) || + !bms_is_member(attnum, clauses_attnums)); + + clauses_attnums = bms_add_member(clauses_attnums, attnum); + } + /* * If there's not at least two distinct attnums and expressions, then * reject the whole list of clauses. We must return 1.0 so the calling @@ -1470,19 +1526,37 @@ dependencies_clauselist_selectivity(PlannerInfo *root, foreach(l, rel->statlist) { StatisticExtInfo *stat = (StatisticExtInfo *) lfirst(l); - Bitmapset *matched; int nmatched; int nexprs; + int k; MVDependencies *deps; /* skip statistics that are not of the correct type */ if (stat->kind != STATS_EXT_DEPENDENCIES) continue; - /* count matching simple clauses */ - matched = bms_intersect(clauses_attnums, stat->keys); - nmatched = bms_num_members(matched); - bms_free(matched); + /* + * Count matching attributes - we have to undo two attnum offsets. + * First, the dependency is offset using the number of expressions + * for that statistics, and then (if it's a plain attribute) we + * need to apply the same offset as above, by unique_exprs_cnt. + */ + nmatched = 0; + k = -1; + while ((k = bms_next_member(stat->keys, k)) >= 0) + { + AttrNumber attnum = (AttrNumber) k; + + /* skip expressions */ + if (!AttrNumberIsForUserDefinedAttr(attnum)) + continue; + + /* apply the same offset as above */ + attnum += unique_exprs_cnt; + + if (bms_is_member(attnum, clauses_attnums)) + nmatched++; + } /* count matching expressions */ nexprs = 0; @@ -1537,13 +1611,23 @@ dependencies_clauselist_selectivity(PlannerInfo *root, Node *expr; int k; AttrNumber unique_attnum = InvalidAttrNumber; + AttrNumber attnum; - /* regular attribute, no need to remap */ - if (dep->attributes[j] <= MaxHeapAttributeNumber) + /* undo the per-statistics offset */ + attnum = dep->attributes[j]; + + /* regular attribute, simply offset by number of expressions */ + if (AttrNumberIsForUserDefinedAttr(attnum)) + { + dep->attributes[j] = attnum + unique_exprs_cnt; continue; + } + + /* the attnum should be a valid system attnum (-1, -2, ...) */ + Assert(AttributeNumberIsValid(attnum)); /* index of the expression */ - idx = EXPRESSION_INDEX(dep->attributes[j]); + idx = (1 - attnum); /* make sure the expression index is valid */ Assert((idx >= 0) && (idx < list_length(stat->exprs))); @@ -1559,7 +1643,7 @@ dependencies_clauselist_selectivity(PlannerInfo *root, */ if (equal(unique_exprs[k], expr)) { - unique_attnum = EXPRESSION_ATTNUM(k); + unique_attnum = -(k + 1) + unique_exprs_cnt; break; } } diff --git a/src/backend/statistics/extended_stats.c b/src/backend/statistics/extended_stats.c index 6ed938d6ab..95b2cc683e 100644 --- a/src/backend/statistics/extended_stats.c +++ b/src/backend/statistics/extended_stats.c @@ -892,7 +892,7 @@ bsearch_arg(const void *key, const void *base, size_t nmemb, size_t size, * is not necessary here (and when querying the bitmap). */ AttrNumber * -build_attnums_array(Bitmapset *attrs, int *numattrs) +build_attnums_array(Bitmapset *attrs, int nexprs, int *numattrs) { int i, j; @@ -908,16 +908,19 @@ build_attnums_array(Bitmapset *attrs, int *numattrs) j = -1; while ((j = bms_next_member(attrs, j)) >= 0) { + AttrNumber attnum = (j - nexprs); + /* * Make sure the bitmap contains only user-defined attributes. As * bitmaps can't contain negative values, this can be violated in two * ways. Firstly, the bitmap might contain 0 as a member, and secondly * the integer value might be larger than MaxAttrNumber. */ - Assert(AttrNumberIsForUserDefinedAttr(j)); - Assert(j <= MaxAttrNumber); + Assert(AttributeNumberIsValid(attnum)); + Assert(attnum <= MaxAttrNumber); + Assert(attnum >= (-nexprs)); - attnums[i++] = (AttrNumber) j; + attnums[i++] = (AttrNumber) attnum; /* protect against overflows */ Assert(i <= num); @@ -984,15 +987,16 @@ build_sorted_items(int numrows, int *nitems, HeapTuple *rows, ExprInfo *exprs, Datum value; bool isnull; int attlen; + AttrNumber attnum = attnums[j]; - if (attnums[j] <= MaxHeapAttributeNumber) + if (AttrNumberIsForUserDefinedAttr(attnum)) { - value = heap_getattr(rows[i], attnums[j], tdesc, &isnull); - attlen = TupleDescAttr(tdesc, attnums[j] - 1)->attlen; + value = heap_getattr(rows[i], attnum, tdesc, &isnull); + attlen = TupleDescAttr(tdesc, attnum - 1)->attlen; } else { - int idx = EXPRESSION_INDEX(attnums[j]); + int idx = -(attnums[j] + 1); Assert((idx >= 0) && (idx < exprs->nexprs)); @@ -1097,6 +1101,21 @@ stat_find_expression(StatisticExtInfo *stat, Node *expr) return -1; } +static bool +stat_covers_attributes(StatisticExtInfo *stat, Bitmapset *attnums) +{ + int k; + + k = -1; + while ((k = bms_next_member(attnums, k)) >= 0) + { + if (!bms_is_member(k, stat->keys)) + return false; + } + + return true; +} + /* * stat_covers_expressions * Test whether a statistics object covers all expressions in a list. @@ -1181,7 +1200,7 @@ choose_best_statistics(List *stats, char requiredkind, continue; /* ignore clauses that are not covered by this object */ - if (!bms_is_subset(clause_attnums[i], info->keys) || + if (!stat_covers_attributes(info, clause_attnums[i]) || !stat_covers_expressions(info, clause_exprs[i], &expr_idxs)) continue; @@ -1685,7 +1704,7 @@ statext_mcv_clauselist_selectivity(PlannerInfo *root, List *clauses, int varReli * estimate. */ if (!bms_is_member(listidx, *estimatedclauses) && - bms_is_subset(list_attnums[listidx], stat->keys) && + stat_covers_attributes(stat, list_attnums[listidx]) && stat_covers_expressions(stat, list_exprs[listidx], NULL)) { /* record simple clauses (single column or expression) */ @@ -2555,37 +2574,3 @@ evaluate_expressions(Relation rel, List *exprs, int numrows, HeapTuple *rows) return result; } - -/* - * add_expressions_to_attributes - * add expressions as attributes with high attnums - * - * Treat the expressions as attributes with attnums above the regular - * attnum range. This will allow us to handle everything in the same - * way, and identify expressions in the dependencies. - * - * XXX This always creates a copy of the bitmap. We might optimize this - * by only creating the copy with (nexprs > 0) but then we'd have to track - * this in order to free it (if we want to). Does not seem worth it. - */ -Bitmapset * -add_expressions_to_attributes(Bitmapset *attrs, int nexprs) -{ - int i; - - /* - * Copy the bitmapset and add fake attnums representing expressions, - * starting above MaxHeapAttributeNumber. - */ - attrs = bms_copy(attrs); - - /* start with (MaxHeapAttributeNumber + 1) */ - for (i = 0; i < nexprs; i++) - { - Assert(EXPRESSION_ATTNUM(i) > MaxHeapAttributeNumber); - - attrs = bms_add_member(attrs, EXPRESSION_ATTNUM(i)); - } - - return attrs; -} diff --git a/src/backend/statistics/mcv.c b/src/backend/statistics/mcv.c index 3bb6fa733d..323d476814 100644 --- a/src/backend/statistics/mcv.c +++ b/src/backend/statistics/mcv.c @@ -187,6 +187,7 @@ statext_mcv_build(int numrows, HeapTuple *rows, ExprInfo *exprs, double totalrows, int stattarget) { int i, + k, numattrs, ngroups, nitems; @@ -206,10 +207,26 @@ statext_mcv_build(int numrows, HeapTuple *rows, ExprInfo *exprs, * XXX We do this after build_mss, because that expects the bitmapset * to only contain simple attributes (with a matching VacAttrStats) */ - attrs = add_expressions_to_attributes(attrs, exprs->nexprs); - /* now build the array, with the special expression attnums */ - attnums = build_attnums_array(attrs, &numattrs); + /* + * Transform the bms into an array, to make accessing i-th member easier. + */ + attnums = (AttrNumber *) palloc(sizeof(AttrNumber) * (bms_num_members(attrs) + exprs->nexprs)); + + numattrs = 0; + + /* regular attributes */ + k = -1; + while ((k = bms_next_member(attrs, k)) >= 0) + attnums[numattrs++] = k; + + /* treat expressions as attributes with negative attnums */ + for (i = 0; i < exprs->nexprs; i++) + attnums[numattrs++] = -(i+1); + + Assert(numattrs >= 2); + Assert(numattrs == (bms_num_members(attrs) + exprs->nexprs)); + /* sort the rows */ items = build_sorted_items(numrows, &nitems, rows, exprs, @@ -349,7 +366,6 @@ statext_mcv_build(int numrows, HeapTuple *rows, ExprInfo *exprs, pfree(items); pfree(groups); - pfree(attrs); return mcvlist; } @@ -1692,6 +1708,8 @@ mcv_get_match_bitmap(PlannerInfo *root, List *clauses, bool match = true; MCVItem *item = &mcvlist->items[i]; + Assert(idx >= 0); + /* * When the MCV item or the Const value is NULL we can * treat this as a mismatch. We must not call the operator diff --git a/src/backend/statistics/mvdistinct.c b/src/backend/statistics/mvdistinct.c index 55d3fa0e1f..5e796e7123 100644 --- a/src/backend/statistics/mvdistinct.c +++ b/src/backend/statistics/mvdistinct.c @@ -83,9 +83,9 @@ static void generate_combinations(CombinationGenerator *state); * This computes the ndistinct estimate using the same estimator used * in analyze.c and then computes the coefficient. * - * To handle expressions easily, we treat them as special attributes with - * attnums above MaxHeapAttributeNumber, and we assume the expressions are - * placed after all simple attributes. + * To handle expressions easily, we treat them as system attributes with + * negative attnums, and offset everything by number of expressions to + * allow using Bitmapsets. */ MVNDistinct * statext_ndistinct_build(double totalrows, int numrows, HeapTuple *rows, @@ -93,10 +93,12 @@ statext_ndistinct_build(double totalrows, int numrows, HeapTuple *rows, VacAttrStats **stats) { MVNDistinct *result; + int i; int k; int itemcnt; int numattrs = bms_num_members(attrs); int numcombs = num_combinations(numattrs + exprs->nexprs); + Bitmapset *tmp = NULL; result = palloc(offsetof(MVNDistinct, items) + numcombs * sizeof(MVNDistinctItem)); @@ -104,8 +106,26 @@ statext_ndistinct_build(double totalrows, int numrows, HeapTuple *rows, result->type = STATS_NDISTINCT_TYPE_BASIC; result->nitems = numcombs; - /* treat expressions as special attributes with high attnums */ - attrs = add_expressions_to_attributes(attrs, exprs->nexprs); + /* + * Treat expressions as system attributes with negative attnums, + * but offset everything by number of expressions. + */ + for (i = 0; i < exprs->nexprs; i++) + { + AttrNumber attnum = -(i + 1); + tmp = bms_add_member(tmp, attnum + exprs->nexprs); + } + + /* regular attributes */ + k = -1; + while ((k = bms_next_member(attrs, k)) >= 0) + { + AttrNumber attnum = k; + tmp = bms_add_member(tmp, attnum + exprs->nexprs); + } + + /* use the newly built bitmapset */ + attrs = tmp; /* make sure there were no clashes */ Assert(bms_num_members(attrs) == numattrs + exprs->nexprs); @@ -124,29 +144,33 @@ statext_ndistinct_build(double totalrows, int numrows, HeapTuple *rows, MVNDistinctItem *item = &result->items[itemcnt]; int j; - item->attrs = NULL; + item->attributes = palloc(sizeof(AttrNumber) * k); + item->nattributes = k; + for (j = 0; j < k; j++) { AttrNumber attnum = InvalidAttrNumber; /* - * The simple attributes are before expressions, so have - * indexes below numattrs. - * */ - if (combination[j] < numattrs) - attnum = stats[combination[j]]->attr->attnum; + * The expressions have negative attnums, so even with the + * offset are before regular attributes. So the first chunk + * of indexes are for expressions. + */ + if (combination[j] >= exprs->nexprs) + attnum + = stats[combination[j] - exprs->nexprs]->attr->attnum; else { /* make sure the expression index is valid */ - Assert((combination[j] - numattrs) >= 0); - Assert((combination[j] - numattrs) < exprs->nexprs); + Assert(combination[j] >= 0); + Assert(combination[j] < exprs->nexprs); - attnum = EXPRESSION_ATTNUM(combination[j] - numattrs); + attnum = -(combination[j] + 1); } Assert(attnum != InvalidAttrNumber); - item->attrs = bms_add_member(item->attrs, attnum); + item->attributes[j] = attnum; } item->ndistinct = @@ -223,7 +247,7 @@ statext_ndistinct_serialize(MVNDistinct *ndistinct) { int nmembers; - nmembers = bms_num_members(ndistinct->items[i].attrs); + nmembers = ndistinct->items[i].nattributes; Assert(nmembers >= 2); len += SizeOfItem(nmembers); @@ -248,22 +272,15 @@ statext_ndistinct_serialize(MVNDistinct *ndistinct) for (i = 0; i < ndistinct->nitems; i++) { MVNDistinctItem item = ndistinct->items[i]; - int nmembers = bms_num_members(item.attrs); - int x; + int nmembers = item.nattributes; memcpy(tmp, &item.ndistinct, sizeof(double)); tmp += sizeof(double); memcpy(tmp, &nmembers, sizeof(int)); tmp += sizeof(int); - x = -1; - while ((x = bms_next_member(item.attrs, x)) >= 0) - { - AttrNumber value = (AttrNumber) x; - - memcpy(tmp, &value, sizeof(AttrNumber)); - tmp += sizeof(AttrNumber); - } + memcpy(tmp, item.attributes, sizeof(AttrNumber) * nmembers); + tmp += nmembers * sizeof(AttrNumber); /* protect against overflows */ Assert(tmp <= ((char *) output + len)); @@ -335,27 +352,21 @@ statext_ndistinct_deserialize(bytea *data) for (i = 0; i < ndistinct->nitems; i++) { MVNDistinctItem *item = &ndistinct->items[i]; - int nelems; - - item->attrs = NULL; /* ndistinct value */ memcpy(&item->ndistinct, tmp, sizeof(double)); tmp += sizeof(double); /* number of attributes */ - memcpy(&nelems, tmp, sizeof(int)); + memcpy(&item->nattributes, tmp, sizeof(int)); tmp += sizeof(int); - Assert((nelems >= 2) && (nelems <= STATS_MAX_DIMENSIONS)); + Assert((item->nattributes >= 2) && (item->nattributes <= STATS_MAX_DIMENSIONS)); - while (nelems-- > 0) - { - AttrNumber attno; + item->attributes + = (AttrNumber *) palloc(item->nattributes * sizeof(AttrNumber)); - memcpy(&attno, tmp, sizeof(AttrNumber)); - tmp += sizeof(AttrNumber); - item->attrs = bms_add_member(item->attrs, attno); - } + memcpy(item->attributes, tmp, sizeof(AttrNumber) * item->nattributes); + tmp += sizeof(AttrNumber) * item->nattributes; /* still within the bytea */ Assert(tmp <= ((char *) data + VARSIZE_ANY(data))); @@ -403,17 +414,16 @@ pg_ndistinct_out(PG_FUNCTION_ARGS) for (i = 0; i < ndist->nitems; i++) { - MVNDistinctItem item = ndist->items[i]; - int x = -1; - bool first = true; + int j; + MVNDistinctItem item = ndist->items[i]; if (i > 0) appendStringInfoString(&str, ", "); - while ((x = bms_next_member(item.attrs, x)) >= 0) + for (j = 0; j < item.nattributes; j++) { - appendStringInfo(&str, "%s%d", first ? "\"" : ", ", x); - first = false; + AttrNumber attnum = item.attributes[j]; + appendStringInfo(&str, "%s%d", (j == 0) ? "\"" : ", ", attnum); } appendStringInfo(&str, "\": %d", (int) item.ndistinct); } @@ -508,9 +518,10 @@ ndistinct_for_combination(double totalrows, int numrows, HeapTuple *rows, TupleDesc tdesc = NULL; Oid collid = InvalidOid; - if (combination[i] < nattrs) + /* first nexprs indexes are for expressions, then regular attributes */ + if (combination[i] >= exprs->nexprs) { - VacAttrStats *colstat = stats[combination[i]]; + VacAttrStats *colstat = stats[combination[i] - exprs->nexprs]; typid = colstat->attrtypid; attnum = colstat->attr->attnum; collid = colstat->attrcollid; @@ -518,8 +529,8 @@ ndistinct_for_combination(double totalrows, int numrows, HeapTuple *rows, } else { - typid = exprs->types[combination[i] - nattrs]; - collid = exprs->collations[combination[i] - nattrs]; + typid = exprs->types[combination[i]]; + collid = exprs->collations[combination[i]]; } type = lookup_type_cache(typid, TYPECACHE_LT_OPR); @@ -534,10 +545,13 @@ ndistinct_for_combination(double totalrows, int numrows, HeapTuple *rows, for (j = 0; j < numrows; j++) { /* - * The first nattrs indexes identify simple attributes, higher - * indexes are expressions. + * The first exprs indexes identify expressions, higher indexes + * are for plain attributes. + * + * XXX This seems a bit strange that we don't offset the (i) + * in any way? */ - if (combination[i] < nattrs) + if (combination[i] >= exprs->nexprs) items[j].values[i] = heap_getattr(rows[j], attnum, @@ -545,7 +559,9 @@ ndistinct_for_combination(double totalrows, int numrows, HeapTuple *rows, &items[j].isnull[i]); else { - int idx = (combination[i] - nattrs); + /* we know the first nexprs expressions are expressions, + * and the value is directly the expression index */ + int idx = combination[i]; /* make sure the expression index is valid */ Assert((idx >= 0) && (idx < exprs->nexprs)); diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c index a7edcaeaff..26e1912940 100644 --- a/src/backend/utils/adt/selfuncs.c +++ b/src/backend/utils/adt/selfuncs.c @@ -4144,7 +4144,8 @@ estimate_multivariate_ndistinct(PlannerInfo *root, RelOptInfo *rel, if (equal(exprinfo->expr, expr)) { - matched = bms_add_member(matched, MaxHeapAttributeNumber + idx); + AttrNumber attnum = -(idx + 1); + matched = bms_add_member(matched, attnum + list_length(matched_info->exprs)); found = true; break; } @@ -4165,10 +4166,10 @@ estimate_multivariate_ndistinct(PlannerInfo *root, RelOptInfo *rel, if (!AttrNumberIsForUserDefinedAttr(attnum)) continue; - if (!bms_is_member(attnum, matched_info->keys)) + if (!bms_is_member(attnum + list_length(matched_info->exprs), matched_info->keys)) continue; - matched = bms_add_member(matched, attnum); + matched = bms_add_member(matched, attnum + list_length(matched_info->exprs)); } } } @@ -4176,13 +4177,29 @@ estimate_multivariate_ndistinct(PlannerInfo *root, RelOptInfo *rel, /* Find the specific item that exactly matches the combination */ for (i = 0; i < stats->nitems; i++) { + int j; MVNDistinctItem *tmpitem = &stats->items[i]; - if (bms_subset_compare(tmpitem->attrs, matched) == BMS_EQUAL) + if (tmpitem->nattributes != bms_num_members(matched)) + continue; + + /* assume it's the right item */ + item = tmpitem; + + for (j = 0; j < tmpitem->nattributes; j++) { - item = tmpitem; - break; + AttrNumber attnum = tmpitem->attributes[j]; + + if (!bms_is_member(attnum, matched)) + { + /* nah, it's not this item */ + item = NULL; + break; + } } + + if (item) + break; } /* make sure we found an item */ diff --git a/src/include/statistics/extended_stats_internal.h b/src/include/statistics/extended_stats_internal.h index b2e59f9bc5..1f09799deb 100644 --- a/src/include/statistics/extended_stats_internal.h +++ b/src/include/statistics/extended_stats_internal.h @@ -106,7 +106,7 @@ extern void *bsearch_arg(const void *key, const void *base, int (*compar) (const void *, const void *, void *), void *arg); -extern AttrNumber *build_attnums_array(Bitmapset *attrs, int *numattrs); +extern AttrNumber *build_attnums_array(Bitmapset *attrs, int nexprs, int *numattrs); extern SortItem *build_sorted_items(int numrows, int *nitems, HeapTuple *rows, ExprInfo *exprs, TupleDesc tdesc, @@ -141,13 +141,4 @@ extern Selectivity mcv_clause_selectivity_or(PlannerInfo *root, Selectivity *overlap_basesel, Selectivity *totalsel); -extern Bitmapset *add_expressions_to_attributes(Bitmapset *attrs, int nexprs); - -/* translate 0-based expression index to attnum and back */ -#define EXPRESSION_ATTNUM(index) \ - (MaxHeapAttributeNumber + (index) + 1) - -#define EXPRESSION_INDEX(attnum) \ - ((attnum) - MaxHeapAttributeNumber - 1) - #endif /* EXTENDED_STATS_INTERNAL_H */ diff --git a/src/include/statistics/statistics.h b/src/include/statistics/statistics.h index 006d578e0c..326cf26fea 100644 --- a/src/include/statistics/statistics.h +++ b/src/include/statistics/statistics.h @@ -26,7 +26,8 @@ typedef struct MVNDistinctItem { double ndistinct; /* ndistinct value for this combination */ - Bitmapset *attrs; /* attr numbers of items */ + int nattributes; /* number of attributes */ + AttrNumber *attributes; /* attribute numbers */ } MVNDistinctItem; /* A MVNDistinct object, comprising all possible combinations of columns */ -- 2.26.2 --------------614DDB87AFFED893713AC0E9 Content-Type: text/x-patch; charset=UTF-8; name="0005-WIP-unify-handling-of-attributes-and-expres-20210304.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="0005-WIP-unify-handling-of-attributes-and-expres-20210304.pa"; filename*1="tch" ^ permalink raw reply [nested|flat] 2+ messages in thread
* Some shared memory chunks are allocated even if related processes won't start @ 2024-03-04 05:26 Hayato Kuroda (Fujitsu) <[email protected]> 0 siblings, 0 replies; 2+ messages in thread From: Hayato Kuroda (Fujitsu) @ 2024-03-04 05:26 UTC (permalink / raw) To: '[email protected]' <[email protected]> Dear hackers, While reading codes, I found that ApplyLauncherShmemInit() and AutoVacuumShmemInit() are always called even if they would not be launched. It may be able to reduce the start time to avoid the unnecessary allocation. However, I know this improvement would be quite small because the allocated chunks are quite small. Anyway, there are several ways to fix: 1) Skip calling ShmemInitStruct() if the related process would not be launched. I think this approach is the easiest way. E.g., ``` --- a/src/backend/replication/logical/launcher.c +++ b/src/backend/replication/logical/launcher.c @@ -962,6 +962,9 @@ ApplyLauncherShmemInit(void) { bool found; + if (max_logical_replication_workers == 0 || IsBinaryUpgrade) + return; + ``` 2) Dynamically allocate the shared memory. This was allowed by recent commit [1]. I made a small PoC only for logical launcher to show what I meant. PSA diff file. Since some processes (backend, apply worker, parallel apply worker, and tablesync worker) refers the chunk, codes for attachment must be added on the several places. If you agree it should be fixed, I will create a patch. Thought? [1]: https://github.com/postgres/postgres/commit/8b2bcf3f287c79eaebf724cba57e5ff664b01e06 Best Regards, Hayato Kuroda FUJITSU LIMITED https://www.fujitsu.com/ Attachments: [application/octet-stream] dynamic_allocation.diff (84B, ../../TYCPR01MB12077BFDFBC142086D424FDFEF5232@TYCPR01MB12077.jpnprd01.prod.outlook.com/2-dynamic_allocation.diff) download | inline diff: 53 22 src/backend/replication/logical/launcher.c 0 1 src/backend/storage/ipc/ipci.c ^ permalink raw reply [nested|flat] 2+ messages in thread
end of thread, other threads:[~2024-03-04 05:26 UTC | newest] Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2021-02-17 00:02 [PATCH 4/5] WIP rework tracking of expressions Tomas Vondra <[email protected]> 2024-03-04 05:26 Some shared memory chunks are allocated even if related processes won't start Hayato Kuroda (Fujitsu) <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox