public inbox for [email protected]help / color / mirror / Atom feed
[PATCH 5/5] WIP unify handling of attributes and expressions 3+ messages / 3 participants [nested] [flat]
* [PATCH 5/5] WIP unify handling of attributes and expressions @ 2021-03-04 03:53 Tomas Vondra <[email protected]> 0 siblings, 0 replies; 3+ messages in thread From: Tomas Vondra @ 2021-03-04 03:53 UTC (permalink / raw) --- src/backend/statistics/dependencies.c | 82 ++------ src/backend/statistics/extended_stats.c | 180 +++++++++++------- src/backend/statistics/mcv.c | 89 ++------- src/backend/statistics/mvdistinct.c | 138 +++----------- .../statistics/extended_stats_internal.h | 40 ++-- 5 files changed, 183 insertions(+), 346 deletions(-) diff --git a/src/backend/statistics/dependencies.c b/src/backend/statistics/dependencies.c index 602301b724..14d6503e1a 100644 --- a/src/backend/statistics/dependencies.c +++ b/src/backend/statistics/dependencies.c @@ -70,10 +70,7 @@ static void generate_dependencies(DependencyGenerator state); static DependencyGenerator DependencyGenerator_init(int n, int k); static void DependencyGenerator_free(DependencyGenerator state); static AttrNumber *DependencyGenerator_next(DependencyGenerator state); -static double dependency_degree(int numrows, HeapTuple *rows, - ExprInfo *exprs, int k, - AttrNumber *dependency, VacAttrStats **stats, - Bitmapset *attrs); +static double dependency_degree(StatBuildData *data, int k, AttrNumber *dependency); static bool dependency_is_fully_matched(MVDependency *dependency, Bitmapset *attnums); static bool dependency_is_compatible_clause(Node *clause, Index relid, @@ -222,17 +219,13 @@ DependencyGenerator_next(DependencyGenerator state) * the last one. */ static double -dependency_degree(int numrows, HeapTuple *rows, ExprInfo *exprs, int k, - AttrNumber *dependency, VacAttrStats **stats, - Bitmapset *attrs) +dependency_degree(StatBuildData *data, int k, AttrNumber *dependency) { int i, nitems; MultiSortSupport mss; SortItem *items; - AttrNumber *attnums; AttrNumber *attnums_dep; - int numattrs; /* counters valid within a group */ int group_size = 0; @@ -247,16 +240,9 @@ dependency_degree(int numrows, HeapTuple *rows, ExprInfo *exprs, int k, /* sort info for all attributes columns */ mss = multi_sort_init(k); - /* - * Transform the attrs from bitmap to an array to make accessing the i-th - * member easier, and then construct a filtered version with only attnums - * referenced by the dependency we validate. - */ - attnums = build_attnums_array(attrs, exprs->nexprs, &numattrs); - attnums_dep = (AttrNumber *) palloc(k * sizeof(AttrNumber)); for (i = 0; i < k; i++) - attnums_dep[i] = attnums[dependency[i]]; + attnums_dep[i] = data->attnums[dependency[i]]; /* * Verify the dependency (a,b,...)->z, using a rather simple algorithm: @@ -274,7 +260,7 @@ dependency_degree(int numrows, HeapTuple *rows, ExprInfo *exprs, int k, /* prepare the sort function for the dimensions */ for (i = 0; i < k; i++) { - VacAttrStats *colstat = stats[dependency[i]]; + VacAttrStats *colstat = data->stats[dependency[i]]; TypeCacheEntry *type; type = lookup_type_cache(colstat->attrtypid, TYPECACHE_LT_OPR); @@ -293,8 +279,7 @@ dependency_degree(int numrows, HeapTuple *rows, ExprInfo *exprs, int k, * descriptor. For now that assumption holds, but it might change in the * future for example if we support statistics on multiple tables. */ - items = build_sorted_items(numrows, &nitems, rows, exprs, - stats[0]->tupDesc, mss, k, attnums_dep); + items = build_sorted_items(data, &nitems, mss, k, attnums_dep); /* * Walk through the sorted array, split it into rows according to the @@ -340,11 +325,10 @@ dependency_degree(int numrows, HeapTuple *rows, ExprInfo *exprs, int k, pfree(items); pfree(mss); - pfree(attnums); pfree(attnums_dep); /* Compute the 'degree of validity' as (supporting/total). */ - return (n_supporting_rows * 1.0 / numrows); + return (n_supporting_rows * 1.0 / data->numrows); } /* @@ -364,54 +348,15 @@ dependency_degree(int numrows, HeapTuple *rows, ExprInfo *exprs, int k, * (c) -> b */ MVDependencies * -statext_dependencies_build(int numrows, HeapTuple *rows, - ExprInfo *exprs, Bitmapset *attrs, - VacAttrStats **stats) +statext_dependencies_build(StatBuildData *data) { int i, k; - int numattrs; - AttrNumber *attnums; - int nattnums; /* result */ MVDependencies *dependencies = NULL; - /* - * 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); - - /* - * and then regular attributes - * - * XXX Maybe add this in the opposite order, just like in MCV? first - * regular attnums, then exressions. - */ - 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); + Assert(data->nattnums >= 2); /* * We'll try build functional dependencies starting from the smallest ones @@ -419,12 +364,12 @@ statext_dependencies_build(int numrows, HeapTuple *rows, * included in the statistics object. We start from the smallest ones * because we want to be able to skip already implied ones. */ - for (k = 2; k <= numattrs; k++) + for (k = 2; k <= data->nattnums; k++) { AttrNumber *dependency; /* array with k elements */ /* prepare a DependencyGenerator of variation */ - DependencyGenerator DependencyGenerator = DependencyGenerator_init(numattrs, k); + DependencyGenerator DependencyGenerator = DependencyGenerator_init(data->nattnums, k); /* generate all possible variations of k values (out of n) */ while ((dependency = DependencyGenerator_next(DependencyGenerator))) @@ -433,8 +378,7 @@ statext_dependencies_build(int numrows, HeapTuple *rows, MVDependency *d; /* compute how valid the dependency seems */ - degree = dependency_degree(numrows, rows, exprs, k, dependency, - stats, attrs); + degree = dependency_degree(data, k, dependency); /* * if the dependency seems entirely invalid, don't store it @@ -449,7 +393,7 @@ statext_dependencies_build(int numrows, HeapTuple *rows, d->degree = degree; d->nattributes = k; for (i = 0; i < k; i++) - d->attributes[i] = attnums[dependency[i]]; + d->attributes[i] = data->attnums[dependency[i]]; /* initialize the list of dependencies */ if (dependencies == NULL) @@ -477,8 +421,6 @@ statext_dependencies_build(int numrows, HeapTuple *rows, DependencyGenerator_free(DependencyGenerator); } - pfree(attrs); - return dependencies; } diff --git a/src/backend/statistics/extended_stats.c b/src/backend/statistics/extended_stats.c index 95b2cc683e..5b3fa523e9 100644 --- a/src/backend/statistics/extended_stats.c +++ b/src/backend/statistics/extended_stats.c @@ -96,8 +96,11 @@ static Datum serialize_expr_stats(AnlExprData *exprdata, int nexprs); static Datum expr_fetch_func(VacAttrStatsP stats, int rownum, bool *isNull); static AnlExprData *build_expr_data(List *exprs); static VacAttrStats *examine_expression(Node *expr); -static ExprInfo *evaluate_expressions(Relation rel, List *exprs, - int numrows, HeapTuple *rows); + +static StatBuildData *make_build_data(Relation onerel, StatExtEntry *stat, + int numrows, HeapTuple *rows, + VacAttrStats **stats); + /* * Compute requested extended stats, using the rows sampled for the plain @@ -156,7 +159,7 @@ BuildRelationExtStatistics(Relation onerel, double totalrows, VacAttrStats **stats; ListCell *lc2; int stattarget; - ExprInfo *exprs; + StatBuildData *data; /* * Check if we can build these stats based on the column analyzed. If @@ -191,7 +194,7 @@ BuildRelationExtStatistics(Relation onerel, double totalrows, continue; /* evaluate expressions (if the statistics has any) */ - exprs = evaluate_expressions(onerel, stat->exprs, numrows, rows); + data = make_build_data(onerel, stat, numrows, rows, stats); /* compute statistic of each requested type */ foreach(lc2, stat->types) @@ -199,16 +202,11 @@ BuildRelationExtStatistics(Relation onerel, double totalrows, char t = (char) lfirst_int(lc2); if (t == STATS_EXT_NDISTINCT) - ndistinct = statext_ndistinct_build(totalrows, numrows, rows, - exprs, stat->columns, - stats); + ndistinct = statext_ndistinct_build(totalrows, data); else if (t == STATS_EXT_DEPENDENCIES) - dependencies = statext_dependencies_build(numrows, rows, - exprs, stat->columns, - stats); + dependencies = statext_dependencies_build(data); else if (t == STATS_EXT_MCV) - mcv = statext_mcv_build(numrows, rows, exprs, stat->columns, - stats, totalrows, stattarget); + mcv = statext_mcv_build(data, totalrows, stattarget); else if (t == STATS_EXT_EXPRESSIONS) { AnlExprData *exprdata; @@ -236,7 +234,8 @@ BuildRelationExtStatistics(Relation onerel, double totalrows, pgstat_progress_update_param(PROGRESS_ANALYZE_EXT_STATS_COMPUTED, ++ext_cnt); - pfree(exprs); + /* free the build data (allocated as a single chunk) */ + pfree(data); } table_close(pg_stext, RowExclusiveLock); @@ -937,30 +936,31 @@ build_attnums_array(Bitmapset *attrs, int nexprs, int *numattrs) * can simply pfree the return value to release all of it. */ SortItem * -build_sorted_items(int numrows, int *nitems, HeapTuple *rows, ExprInfo *exprs, - TupleDesc tdesc, MultiSortSupport mss, +build_sorted_items(StatBuildData *data, int *nitems, + MultiSortSupport mss, int numattrs, AttrNumber *attnums) { int i, j, len, - idx; - int nvalues = numrows * numattrs; + nrows; + int nvalues = data->numrows * numattrs; SortItem *items; Datum *values; bool *isnull; char *ptr; + int *typlen; /* Compute the total amount of memory we need (both items and values). */ - len = numrows * sizeof(SortItem) + nvalues * (sizeof(Datum) + sizeof(bool)); + len = data->numrows * sizeof(SortItem) + nvalues * (sizeof(Datum) + sizeof(bool)); /* Allocate the memory and split it into the pieces. */ ptr = palloc0(len); /* items to sort */ items = (SortItem *) ptr; - ptr += numrows * sizeof(SortItem); + ptr += data->numrows * sizeof(SortItem); /* values and null flags */ values = (Datum *) ptr; @@ -973,13 +973,24 @@ build_sorted_items(int numrows, int *nitems, HeapTuple *rows, ExprInfo *exprs, Assert((ptr - (char *) items) == len); /* fix the pointers to Datum and bool arrays */ - idx = 0; - for (i = 0; i < numrows; i++) + nrows = 0; + for (i = 0; i < data->numrows; i++) { - bool toowide = false; + items[nrows].values = &values[nrows * numattrs]; + items[nrows].isnull = &isnull[nrows * numattrs]; + + nrows++; + } + + /* build a local cache of typlen for all attributes */ + typlen = (int *) palloc(sizeof(int) * data->nattnums); + for (i = 0; i < data->nattnums; i++) + typlen[i] = get_typlen(data->stats[i]->attrtypid); - items[idx].values = &values[idx * numattrs]; - items[idx].isnull = &isnull[idx * numattrs]; + nrows = 0; + for (i = 0; i < data->numrows; i++) + { + bool toowide = false; /* load the values/null flags from sample rows */ for (j = 0; j < numattrs; j++) @@ -989,22 +1000,20 @@ build_sorted_items(int numrows, int *nitems, HeapTuple *rows, ExprInfo *exprs, int attlen; AttrNumber attnum = attnums[j]; - if (AttrNumberIsForUserDefinedAttr(attnum)) + int idx; + + /* match attnum to the pre-calculated data */ + for (idx = 0; idx < data->nattnums; idx++) { - value = heap_getattr(rows[i], attnum, tdesc, &isnull); - attlen = TupleDescAttr(tdesc, attnum - 1)->attlen; + if (attnum == data->attnums[idx]) + break; } - else - { - int idx = -(attnums[j] + 1); - - Assert((idx >= 0) && (idx < exprs->nexprs)); - value = exprs->values[idx][i]; - isnull = exprs->nulls[idx][i]; + Assert(idx < data->nattnums); - attlen = get_typlen(exprs->types[idx]); - } + value = data->values[idx][i]; + isnull = data->nulls[idx][i]; + attlen = typlen[idx]; /* * If this is a varlena value, check if it's too wide and if yes @@ -1026,21 +1035,21 @@ build_sorted_items(int numrows, int *nitems, HeapTuple *rows, ExprInfo *exprs, value = PointerGetDatum(PG_DETOAST_DATUM(value)); } - items[idx].values[j] = value; - items[idx].isnull[j] = isnull; + items[nrows].values[j] = value; + items[nrows].isnull[j] = isnull; } if (toowide) continue; - idx++; + nrows++; } /* store the actual number of items (ignoring the too-wide ones) */ - *nitems = idx; + *nitems = nrows; /* all items were too wide */ - if (idx == 0) + if (nrows == 0) { /* everything is allocated as a single chunk */ pfree(items); @@ -1048,7 +1057,7 @@ build_sorted_items(int numrows, int *nitems, HeapTuple *rows, ExprInfo *exprs, } /* do the sort, using the multi-sort */ - qsort_arg((void *) items, idx, sizeof(SortItem), + qsort_arg((void *) items, nrows, sizeof(SortItem), multi_sort_compare, mss); return items; @@ -2434,59 +2443,61 @@ statext_expressions_load(Oid stxoid, int idx) * all the requested statistics types. This matters especially for * expensive expressions, of course. */ -static ExprInfo * -evaluate_expressions(Relation rel, List *exprs, int numrows, HeapTuple *rows) +static StatBuildData * +make_build_data(Relation rel, StatExtEntry *stat, int numrows, HeapTuple *rows, + VacAttrStats **stats) { /* evaluated expressions */ - ExprInfo *result; + StatBuildData *result; char *ptr; Size len; int i; + int k; int idx; TupleTableSlot *slot; EState *estate; ExprContext *econtext; List *exprstates = NIL; - int nexprs = list_length(exprs); + int nkeys = bms_num_members(stat->columns) + list_length(stat->exprs); ListCell *lc; /* allocate everything as a single chunk, so we can free it easily */ - len = MAXALIGN(sizeof(ExprInfo)); - len += MAXALIGN(sizeof(Oid) * nexprs); /* types */ - len += MAXALIGN(sizeof(Oid) * nexprs); /* collations */ + len = MAXALIGN(sizeof(StatBuildData)); + len += MAXALIGN(sizeof(AttrNumber) * nkeys); /* attnums */ + len += MAXALIGN(sizeof(VacAttrStats *) * nkeys); /* stats */ /* values */ - len += MAXALIGN(sizeof(Datum *) * nexprs); - len += nexprs * MAXALIGN(sizeof(Datum) * numrows); + len += MAXALIGN(sizeof(Datum *) * nkeys); + len += nkeys * MAXALIGN(sizeof(Datum) * numrows); /* nulls */ - len += MAXALIGN(sizeof(bool *) * nexprs); - len += nexprs * MAXALIGN(sizeof(bool) * numrows); + len += MAXALIGN(sizeof(bool *) * nkeys); + len += nkeys * MAXALIGN(sizeof(bool) * numrows); ptr = palloc(len); /* set the pointers */ - result = (ExprInfo *) ptr; - ptr += MAXALIGN(sizeof(ExprInfo)); + result = (StatBuildData *) ptr; + ptr += MAXALIGN(sizeof(StatBuildData)); - /* types */ - result->types = (Oid *) ptr; - ptr += MAXALIGN(sizeof(Oid) * nexprs); + /* attnums */ + result->attnums = (AttrNumber *) ptr; + ptr += MAXALIGN(sizeof(AttrNumber) * nkeys); - /* collations */ - result->collations = (Oid *) ptr; - ptr += MAXALIGN(sizeof(Oid) * nexprs); + /* stats */ + result->stats = (VacAttrStats **) ptr; + ptr += MAXALIGN(sizeof(VacAttrStats *) * nkeys); /* values */ result->values = (Datum **) ptr; - ptr += MAXALIGN(sizeof(Datum *) * nexprs); + ptr += MAXALIGN(sizeof(Datum *) * nkeys); /* nulls */ result->nulls = (bool **) ptr; - ptr += MAXALIGN(sizeof(bool *) * nexprs); + ptr += MAXALIGN(sizeof(bool *) * nkeys); - for (i = 0; i < nexprs; i++) + for (i = 0; i < nkeys; i++) { result->values[i] = (Datum *) ptr; ptr += MAXALIGN(sizeof(Datum) * numrows); @@ -2497,17 +2508,46 @@ evaluate_expressions(Relation rel, List *exprs, int numrows, HeapTuple *rows) Assert((ptr - (char *) result) == len); - result->nexprs = list_length(exprs); + /* we have it allocated, so let's fill the values */ + result->nattnums = nkeys; + result->numrows = numrows; + /* fill the attribute info - first attributes, then expressions */ idx = 0; - foreach (lc, exprs) + k = -1; + while ((k = bms_next_member(stat->columns, k)) >= 0) + { + result->attnums[idx] = k; + result->stats[idx] = stats[idx]; + + idx++; + } + + k = -1; + foreach (lc, stat->exprs) { Node *expr = (Node *) lfirst(lc); - result->types[idx] = exprType(expr); - result->collations[idx] = exprCollation(expr); + result->attnums[idx] = k; + result->stats[idx] = examine_expression(expr); idx++; + k--; + } + + /* first extract values for all the regular attributes */ + for (i = 0; i < numrows; i++) + { + idx = 0; + k = -1; + while ((k = bms_next_member(stat->columns, k)) >= 0) + { + result->values[idx][i] = heap_getattr(rows[i], k, + result->stats[idx]->tupDesc, + &result->nulls[idx][i]); + + idx++; + } } /* @@ -2526,7 +2566,7 @@ evaluate_expressions(Relation rel, List *exprs, int numrows, HeapTuple *rows) econtext->ecxt_scantuple = slot; /* Set up expression evaluation state */ - exprstates = ExecPrepareExprList(exprs, estate); + exprstates = ExecPrepareExprList(stat->exprs, estate); for (i = 0; i < numrows; i++) { @@ -2539,7 +2579,7 @@ evaluate_expressions(Relation rel, List *exprs, int numrows, HeapTuple *rows) /* Set up for predicate or expression evaluation */ ExecStoreHeapTuple(rows[i], slot, false); - idx = 0; + idx = bms_num_members(stat->columns); foreach (lc, exprstates) { Datum datum; diff --git a/src/backend/statistics/mcv.c b/src/backend/statistics/mcv.c index 323d476814..844ba6f71f 100644 --- a/src/backend/statistics/mcv.c +++ b/src/backend/statistics/mcv.c @@ -74,8 +74,7 @@ ((ndims) * sizeof(DimensionInfo)) + \ ((nitems) * ITEM_SIZE(ndims))) -static MultiSortSupport build_mss(VacAttrStats **stats, int numattrs, - ExprInfo *exprs); +static MultiSortSupport build_mss(StatBuildData *data); static SortItem *build_distinct_groups(int numrows, SortItem *items, MultiSortSupport mss, int *ndistinct); @@ -182,16 +181,11 @@ get_mincount_for_mcv_list(int samplerows, double totalrows) * */ MCVList * -statext_mcv_build(int numrows, HeapTuple *rows, ExprInfo *exprs, - Bitmapset *attrs, VacAttrStats **stats, - double totalrows, int stattarget) +statext_mcv_build(StatBuildData *data, double totalrows, int stattarget) { int i, - k, - numattrs, ngroups, nitems; - AttrNumber *attnums; double mincount; SortItem *items; SortItem *groups; @@ -199,38 +193,11 @@ statext_mcv_build(int numrows, HeapTuple *rows, ExprInfo *exprs, MultiSortSupport mss; /* comparator for all the columns */ - mss = build_mss(stats, bms_num_members(attrs), exprs); - - /* - * treat expressions as special attributes with high attnums - * - * XXX We do this after build_mss, because that expects the bitmapset - * to only contain simple attributes (with a matching VacAttrStats) - */ - - /* - * 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)); - + mss = build_mss(data); /* sort the rows */ - items = build_sorted_items(numrows, &nitems, rows, exprs, - stats[0]->tupDesc, mss, numattrs, attnums); + items = build_sorted_items(data, &nitems, mss, + data->nattnums, data->attnums); if (!items) return NULL; @@ -265,7 +232,7 @@ statext_mcv_build(int numrows, HeapTuple *rows, ExprInfo *exprs, * using get_mincount_for_mcv_list() and then keep all items that seem to * be more common than that. */ - mincount = get_mincount_for_mcv_list(numrows, totalrows); + mincount = get_mincount_for_mcv_list(data->numrows, totalrows); /* * Walk the groups until we find the first group with a count below the @@ -301,7 +268,7 @@ statext_mcv_build(int numrows, HeapTuple *rows, ExprInfo *exprs, + sizeof(SortSupportData)); /* compute frequencies for values in each column */ - nfreqs = (int *) palloc0(sizeof(int) * numattrs); + nfreqs = (int *) palloc0(sizeof(int) * data->nattnums); freqs = build_column_frequencies(groups, ngroups, mss, nfreqs); /* @@ -312,12 +279,12 @@ statext_mcv_build(int numrows, HeapTuple *rows, ExprInfo *exprs, mcvlist->magic = STATS_MCV_MAGIC; mcvlist->type = STATS_MCV_TYPE_BASIC; - mcvlist->ndimensions = numattrs; + mcvlist->ndimensions = data->nattnums; mcvlist->nitems = nitems; /* store info about data type OIDs */ - for (i = 0; i < numattrs; i++) - mcvlist->types[i] = stats[i]->attrtypid; + for (i = 0; i < data->nattnums; i++) + mcvlist->types[i] = data->stats[i]->attrtypid; /* Copy the first chunk of groups into the result. */ for (i = 0; i < nitems; i++) @@ -325,22 +292,22 @@ statext_mcv_build(int numrows, HeapTuple *rows, ExprInfo *exprs, /* just pointer to the proper place in the list */ MCVItem *item = &mcvlist->items[i]; - item->values = (Datum *) palloc(sizeof(Datum) * numattrs); - item->isnull = (bool *) palloc(sizeof(bool) * numattrs); + item->values = (Datum *) palloc(sizeof(Datum) * data->nattnums); + item->isnull = (bool *) palloc(sizeof(bool) * data->nattnums); /* copy values for the group */ - memcpy(item->values, groups[i].values, sizeof(Datum) * numattrs); - memcpy(item->isnull, groups[i].isnull, sizeof(bool) * numattrs); + memcpy(item->values, groups[i].values, sizeof(Datum) * data->nattnums); + memcpy(item->isnull, groups[i].isnull, sizeof(bool) * data->nattnums); /* groups should be sorted by frequency in descending order */ Assert((i == 0) || (groups[i - 1].count >= groups[i].count)); /* group frequency */ - item->frequency = (double) groups[i].count / numrows; + item->frequency = (double) groups[i].count / data->numrows; /* base frequency, if the attributes were independent */ item->base_frequency = 1.0; - for (j = 0; j < numattrs; j++) + for (j = 0; j < data->nattnums; j++) { SortItem *freq; @@ -356,7 +323,7 @@ statext_mcv_build(int numrows, HeapTuple *rows, ExprInfo *exprs, sizeof(SortItem), multi_sort_compare, tmp); - item->base_frequency *= ((double) freq->count) / numrows; + item->base_frequency *= ((double) freq->count) / data->numrows; } } @@ -375,17 +342,17 @@ statext_mcv_build(int numrows, HeapTuple *rows, ExprInfo *exprs, * build MultiSortSupport for the attributes passed in attrs */ static MultiSortSupport -build_mss(VacAttrStats **stats, int numattrs, ExprInfo *exprs) +build_mss(StatBuildData *data) { int i; /* Sort by multiple columns (using array of SortSupport) */ - MultiSortSupport mss = multi_sort_init(numattrs + exprs->nexprs); + MultiSortSupport mss = multi_sort_init(data->nattnums); /* prepare the sort functions for all the attributes */ - for (i = 0; i < numattrs; i++) + for (i = 0; i < data->nattnums; i++) { - VacAttrStats *colstat = stats[i]; + VacAttrStats *colstat = data->stats[i]; TypeCacheEntry *type; type = lookup_type_cache(colstat->attrtypid, TYPECACHE_LT_OPR); @@ -396,20 +363,6 @@ build_mss(VacAttrStats **stats, int numattrs, ExprInfo *exprs) multi_sort_add_dimension(mss, i, type->lt_opr, colstat->attrcollid); } - /* prepare the sort functions for all the expressions */ - for (i = 0; i < exprs->nexprs; i++) - { - TypeCacheEntry *type; - - type = lookup_type_cache(exprs->types[i], TYPECACHE_LT_OPR); - if (type->lt_opr == InvalidOid) /* shouldn't happen */ - elog(ERROR, "cache lookup failed for ordering operator for type %u", - exprs->types[i]); - - multi_sort_add_dimension(mss, numattrs + i, type->lt_opr, - exprs->collations[i]); - } - return mss; } diff --git a/src/backend/statistics/mvdistinct.c b/src/backend/statistics/mvdistinct.c index 5e796e7123..7ca59d9785 100644 --- a/src/backend/statistics/mvdistinct.c +++ b/src/backend/statistics/mvdistinct.c @@ -36,9 +36,7 @@ #include "utils/syscache.h" #include "utils/typcache.h" -static double ndistinct_for_combination(double totalrows, int numrows, - HeapTuple *rows, ExprInfo *exprs, - int nattrs, VacAttrStats **stats, +static double ndistinct_for_combination(double totalrows, StatBuildData *data, int k, int *combination); static double estimate_ndistinct(double totalrows, int numrows, int d, int f1); static int n_choose_k(int n, int k); @@ -88,17 +86,12 @@ static void generate_combinations(CombinationGenerator *state); * allow using Bitmapsets. */ MVNDistinct * -statext_ndistinct_build(double totalrows, int numrows, HeapTuple *rows, - ExprInfo *exprs, Bitmapset *attrs, - VacAttrStats **stats) +statext_ndistinct_build(double totalrows, StatBuildData *data) { MVNDistinct *result; - int i; int k; int itemcnt; - int numattrs = bms_num_members(attrs); - int numcombs = num_combinations(numattrs + exprs->nexprs); - Bitmapset *tmp = NULL; + int numcombs = num_combinations(data->nattnums); result = palloc(offsetof(MVNDistinct, items) + numcombs * sizeof(MVNDistinctItem)); @@ -106,38 +99,14 @@ statext_ndistinct_build(double totalrows, int numrows, HeapTuple *rows, result->type = STATS_NDISTINCT_TYPE_BASIC; result->nitems = numcombs; - /* - * 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); - itemcnt = 0; - for (k = 2; k <= bms_num_members(attrs); k++) + for (k = 2; k <= data->nattnums; k++) { int *combination; CombinationGenerator *generator; /* generate combinations of K out of N elements */ - generator = generator_init(bms_num_members(attrs), k); + generator = generator_init(data->nattnums, k); while ((combination = generator_next(generator))) { @@ -147,36 +116,16 @@ statext_ndistinct_build(double totalrows, int numrows, HeapTuple *rows, item->attributes = palloc(sizeof(AttrNumber) * k); item->nattributes = k; + /* translate the indexes to attnums */ for (j = 0; j < k; j++) { - AttrNumber attnum = InvalidAttrNumber; - - /* - * 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] >= 0); - Assert(combination[j] < exprs->nexprs); - - attnum = -(combination[j] + 1); - } - - Assert(attnum != InvalidAttrNumber); - - item->attributes[j] = attnum; + item->attributes[j] = data->attnums[combination[j]]; + + Assert(AttributeNumberIsValid(item->attributes[j])); } item->ndistinct = - ndistinct_for_combination(totalrows, numrows, rows, - exprs, numattrs, - stats, k, combination); + ndistinct_for_combination(totalrows, data, k, combination); itemcnt++; Assert(itemcnt <= result->nitems); @@ -471,9 +420,8 @@ pg_ndistinct_send(PG_FUNCTION_ARGS) * combination of multiple columns. */ static double -ndistinct_for_combination(double totalrows, int numrows, HeapTuple *rows, - ExprInfo *exprs, int nattrs, - VacAttrStats **stats, int k, int *combination) +ndistinct_for_combination(double totalrows, StatBuildData *data, + int k, int *combination) { int i, j; @@ -493,11 +441,11 @@ ndistinct_for_combination(double totalrows, int numrows, HeapTuple *rows, * using the specified column combination as dimensions. We could try to * sort in place, but it'd probably be more complex and bug-prone. */ - items = (SortItem *) palloc(numrows * sizeof(SortItem)); - values = (Datum *) palloc0(sizeof(Datum) * numrows * k); - isnull = (bool *) palloc0(sizeof(bool) * numrows * k); + items = (SortItem *) palloc(data->numrows * sizeof(SortItem)); + values = (Datum *) palloc0(sizeof(Datum) * data->numrows * k); + isnull = (bool *) palloc0(sizeof(bool) * data->numrows * k); - for (i = 0; i < numrows; i++) + for (i = 0; i < data->numrows; i++) { items[i].values = &values[i * k]; items[i].isnull = &isnull[i * k]; @@ -514,24 +462,11 @@ ndistinct_for_combination(double totalrows, int numrows, HeapTuple *rows, { Oid typid; TypeCacheEntry *type; - AttrNumber attnum = InvalidAttrNumber; - TupleDesc tdesc = NULL; Oid collid = InvalidOid; + VacAttrStats *colstat = data->stats[combination[i]]; - /* first nexprs indexes are for expressions, then regular attributes */ - if (combination[i] >= exprs->nexprs) - { - VacAttrStats *colstat = stats[combination[i] - exprs->nexprs]; - typid = colstat->attrtypid; - attnum = colstat->attr->attnum; - collid = colstat->attrcollid; - tdesc = colstat->tupDesc; - } - else - { - typid = exprs->types[combination[i]]; - collid = exprs->collations[combination[i]]; - } + typid = colstat->attrtypid; + collid = colstat->attrcollid; type = lookup_type_cache(typid, TYPECACHE_LT_OPR); if (type->lt_opr == InvalidOid) /* shouldn't happen */ @@ -542,38 +477,15 @@ ndistinct_for_combination(double totalrows, int numrows, HeapTuple *rows, multi_sort_add_dimension(mss, i, type->lt_opr, collid); /* accumulate all the data for this dimension into the arrays */ - for (j = 0; j < numrows; j++) + for (j = 0; j < data->numrows; j++) { - /* - * 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] >= exprs->nexprs) - items[j].values[i] = - heap_getattr(rows[j], - attnum, - tdesc, - &items[j].isnull[i]); - else - { - /* 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)); - - items[j].values[i] = exprs->values[idx][j]; - items[j].isnull[i] = exprs->nulls[idx][j]; - } + items[j].values[i] = data->values[combination[i]][j]; + items[j].isnull[i] = data->nulls[combination[i]][j]; } } /* We can sort the array now ... */ - qsort_arg((void *) items, numrows, sizeof(SortItem), + qsort_arg((void *) items, data->numrows, sizeof(SortItem), multi_sort_compare, mss); /* ... and count the number of distinct combinations */ @@ -581,7 +493,7 @@ ndistinct_for_combination(double totalrows, int numrows, HeapTuple *rows, f1 = 0; cnt = 1; d = 1; - for (i = 1; i < numrows; i++) + for (i = 1; i < data->numrows; i++) { if (multi_sort_compare(&items[i], &items[i - 1], mss) != 0) { @@ -598,7 +510,7 @@ ndistinct_for_combination(double totalrows, int numrows, HeapTuple *rows, if (cnt == 1) f1 += 1; - return estimate_ndistinct(totalrows, numrows, d, f1); + return estimate_ndistinct(totalrows, data->numrows, d, f1); } /* The Duj1 estimator (already used in analyze.c). */ diff --git a/src/include/statistics/extended_stats_internal.h b/src/include/statistics/extended_stats_internal.h index 1f09799deb..7acf82aa0e 100644 --- a/src/include/statistics/extended_stats_internal.h +++ b/src/include/statistics/extended_stats_internal.h @@ -57,35 +57,26 @@ typedef struct SortItem int count; } SortItem; -/* - * Used to pass pre-computed information about expressions the stats - * object is defined on. - */ -typedef struct ExprInfo -{ - int nexprs; /* number of expressions */ - Oid *collations; /* collation for each expression */ - Oid *types; /* type of each expression */ - Datum **values; /* values for each expression */ - bool **nulls; /* nulls for each expression */ -} ExprInfo; - -extern MVNDistinct *statext_ndistinct_build(double totalrows, - int numrows, HeapTuple *rows, - ExprInfo *exprs, Bitmapset *attrs, - VacAttrStats **stats); +/* a unified representation of the data the statistics is built on */ +typedef struct StatBuildData { + int numrows; + int nattnums; + AttrNumber *attnums; + VacAttrStats **stats; + Datum **values; + bool **nulls; +} StatBuildData; + + +extern MVNDistinct *statext_ndistinct_build(double totalrows, StatBuildData *data); extern bytea *statext_ndistinct_serialize(MVNDistinct *ndistinct); extern MVNDistinct *statext_ndistinct_deserialize(bytea *data); -extern MVDependencies *statext_dependencies_build(int numrows, HeapTuple *rows, - ExprInfo *exprs, Bitmapset *attrs, - VacAttrStats **stats); +extern MVDependencies *statext_dependencies_build(StatBuildData *data); extern bytea *statext_dependencies_serialize(MVDependencies *dependencies); extern MVDependencies *statext_dependencies_deserialize(bytea *data); -extern MCVList *statext_mcv_build(int numrows, HeapTuple *rows, - ExprInfo *exprs, Bitmapset *attrs, - VacAttrStats **stats, +extern MCVList *statext_mcv_build(StatBuildData *data, double totalrows, int stattarget); extern bytea *statext_mcv_serialize(MCVList *mcv, VacAttrStats **stats); extern MCVList *statext_mcv_deserialize(bytea *data); @@ -108,8 +99,7 @@ extern void *bsearch_arg(const void *key, const void *base, 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, +extern SortItem *build_sorted_items(StatBuildData *data, int *nitems, MultiSortSupport mss, int numattrs, AttrNumber *attnums); -- 2.26.2 --------------614DDB87AFFED893713AC0E9-- ^ permalink raw reply [nested|flat] 3+ messages in thread
* Partitioned tables and [un]loggedness @ 2024-04-24 07:17 Michael Paquier <[email protected]> 0 siblings, 1 reply; 3+ messages in thread From: Michael Paquier @ 2024-04-24 07:17 UTC (permalink / raw) To: Postgres hackers <[email protected]> Hi all, As a recent poke on a thread of 2019 has reminded me, the current situation of partitioned tables with unlogged is kind of weird: https://www.postgresql.org/message-id/15954-b61523bed4b110c4%40postgresql.org To sum up the situation: - ALTER TABLE .. SET LOGGED/UNLOGGED does not affect partitioned tables. - New partitions don't inherit the loggedness of the partitioned table. One of the things that could be done is to forbid the use of UNLOGGED in partitioned tables, but I'm wondering if we could be smarter than that to provide a more natural user experience. I've been experiencing with that and finished with the POC attached, that uses the following properties: - Support ALTER TABLE .. SET LOGGED/UNLOGGED for partitioned tables, where the command only works on partitioned tables so that's only a catalog switch. - CREATE TABLE PARTITION OF would make a new partition inherit the logged ness of the parent if UNLOGGED is not directly specified. There are some open questions that need attention: - How about ONLY? Would it make sense to support it so as ALTER TABLE ONLY on a partitioned table does not touch any of its partitions? Would not specifying ONLY mean that the loggedness of the partitioned table and all its partitions is changed? That would mean a burst in WAL when switching to LOGGED, which was a concern when this feature was first implemented even for a single table, so for potentially hundreds of them, that would really hurt if a DBA is not careful. - CREATE TABLE does not have a LOGGED keyword, hence it is not possible through the parser to force a partition to have a permanent persistence even if its partitioned table uses UNLOGGED. Like tablespaces or even recently access methods, the heuristics can be fun to define depending on the impact of the table rewrites. The attached has the code and regression tests to make the rewrites cheaper, which is to make new partitions inherit the loggedness of the parent while altering a parent's property leaves the partitions untouched. With the lack of a LOGGED keyword to force a partition to be permanent when its partitioned table is unlogged, that's a bit funny but feel free to comment. Thanks, -- Michael Attachments: [text/x-diff] 0001-Support-LOGGED-UNLOGGED-for-partitioned-tables.patch (15.8K, ../../[email protected]/2-0001-Support-LOGGED-UNLOGGED-for-partitioned-tables.patch) download | inline diff: From f6462a5f4b9b35d5ace52d55759fefc350e371d3 Mon Sep 17 00:00:00 2001 From: Michael Paquier <[email protected]> Date: Wed, 24 Apr 2024 16:15:29 +0900 Subject: [PATCH] Support LOGGED/UNLOGGED for partitioned tables When using ALTET TABLE SET LOGGED/UNLOGGED, indexes and sequences that are owned by the partitioned table changed need to have their relpersistence also changed. There are a few things that need to be considered here about ONLY: - If ONLY is used only on a partitioned table, switch the persistence be switched only for the partitioned table? - If ONLY is not used, should this operation recurse across all the partitions? CREATE TABLE comes with its own limitation because there is no way to detect in the parser if a new partition should be forced as logged. For now, this patch does: - If UNLOGGED is specified, the partition is unlogged. - If PERMANENT is found, the partition inherits the loggedness from the parent. - There is no way to force a partition to be permanent at query level. --- src/backend/commands/tablecmds.c | 145 ++++++++++++++++++---- src/test/regress/expected/alter_table.out | 84 +++++++++++++ src/test/regress/sql/alter_table.sql | 35 ++++++ doc/src/sgml/ref/alter_table.sgml | 6 + doc/src/sgml/ref/create_table.sgml | 5 + 5 files changed, 252 insertions(+), 23 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 3556240c8e..90b3993fc4 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -601,7 +601,9 @@ static ObjectAddress ATExecClusterOn(Relation rel, const char *indexName, static void ATExecDropCluster(Relation rel, LOCKMODE lockmode); static void ATPrepSetAccessMethod(AlteredTableInfo *tab, Relation rel, const char *amname); static void ATExecSetAccessMethodNoStorage(Relation rel, Oid newAccessMethod); -static bool ATPrepChangePersistence(Relation rel, bool toLogged); +static void ATExecSetPersistenceNoStorage(Relation rel, char newrelpersistence); +static void ATPrepSetPersistence(AlteredTableInfo *tab, Relation rel, + bool toLogged); static void ATPrepSetTableSpace(AlteredTableInfo *tab, Relation rel, const char *tablespacename, LOCKMODE lockmode); static void ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE lockmode); @@ -985,6 +987,30 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId, accessMethodId = get_table_am_oid(default_table_access_method, false); } + /* determine the persistence to use for a partition */ + if (stmt->partbound) + { + /* If UNLOGGED has been specified by the query, just use it */ + if (stmt->relation->relpersistence == RELPERSISTENCE_TEMP) + { + /* Nothing to do, should be temp */ + } + else if (stmt->relation->relpersistence == RELPERSISTENCE_UNLOGGED) + { + /* UNLOGGED has been specified by the query, just use it */ + } + else + { + /* + * Case of RELPERSISTENCE_PERMANENT, where the query specified + * nothing so inherit the persistency from the parent. + */ + Assert(list_length(inheritOids) == 1); + stmt->relation->relpersistence = + get_rel_persistence(linitial_oid(inheritOids)); + } + } + /* * Create the relation. Inherited defaults and constraints are passed in * for immediate handling --- since they don't need parsing, they can be @@ -5036,13 +5062,7 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd, ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("cannot change persistence setting twice"))); - tab->chgPersistence = ATPrepChangePersistence(rel, true); - /* force rewrite if necessary; see comment in ATRewriteTables */ - if (tab->chgPersistence) - { - tab->rewrite |= AT_REWRITE_ALTER_PERSISTENCE; - tab->newrelpersistence = RELPERSISTENCE_PERMANENT; - } + ATPrepSetPersistence(tab, rel, true); pass = AT_PASS_MISC; break; case AT_SetUnLogged: /* SET UNLOGGED */ @@ -5051,13 +5071,7 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd, ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("cannot change persistence setting twice"))); - tab->chgPersistence = ATPrepChangePersistence(rel, false); - /* force rewrite if necessary; see comment in ATRewriteTables */ - if (tab->chgPersistence) - { - tab->rewrite |= AT_REWRITE_ALTER_PERSISTENCE; - tab->newrelpersistence = RELPERSISTENCE_UNLOGGED; - } + ATPrepSetPersistence(tab, rel, false); pass = AT_PASS_MISC; break; case AT_DropOids: /* SET WITHOUT OIDS */ @@ -5427,6 +5441,14 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, break; case AT_SetLogged: /* SET LOGGED */ case AT_SetUnLogged: /* SET UNLOGGED */ + + /* + * Only do this for partitioned tables, for which this is just a + * catalog change. Tables with storage are handled by Phase 3. + */ + if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE && + tab->chgPersistence) + ATExecSetPersistenceNoStorage(rel, tab->newrelpersistence); break; case AT_DropOids: /* SET WITHOUT OIDS */ /* nothing to do here, oid columns don't exist anymore */ @@ -15518,6 +15540,79 @@ ATExecSetAccessMethodNoStorage(Relation rel, Oid newAccessMethodId) table_close(pg_class, RowExclusiveLock); } +/* + * Special handling of ALTER TABLE SET LOGGED/UNLOGGED for relations with no + * storage that have an interest in changing their persistence. + * + * Since these have no storage, setting the persistence to permanent or + * unlogged is a catalog-only operation. This needs to switch the + * persistence of all sequences and indexes related to this relation. + */ +static void +ATExecSetPersistenceNoStorage(Relation rel, char newrelpersistence) +{ + Relation pg_class; + HeapTuple tuple; + List *reloids; /* for indexes and sequences */ + ListCell *elt; + Form_pg_class rd_rel; + Oid reloid = RelationGetRelid(rel); + + /* + * Shouldn't be called on relations having storage; these are processed in + * phase 3. + */ + Assert(!RELKIND_HAS_STORAGE(rel->rd_rel->relkind)); + + /* Get a modifiable copy of the relation's pg_class row. */ + pg_class = table_open(RelationRelationId, RowExclusiveLock); + + tuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(reloid)); + if (!HeapTupleIsValid(tuple)) + elog(ERROR, "cache lookup failed for relation %u", reloid); + rd_rel = (Form_pg_class) GETSTRUCT(tuple); + + /* Leave if no update required */ + if (rd_rel->relpersistence == newrelpersistence) + { + heap_freetuple(tuple); + table_close(pg_class, RowExclusiveLock); + return; + } + + /* Update the pg_class row. */ + rd_rel->relpersistence = newrelpersistence; + CatalogTupleUpdate(pg_class, &tuple->t_self, tuple); + + InvokeObjectPostAlterHook(RelationRelationId, RelationGetRelid(rel), 0); + + heap_freetuple(tuple); + + /* Update the per-sequence and per-index relpersistence */ + reloids = getOwnedSequences(reloid); + reloids = list_union_oid(reloids, RelationGetIndexList(rel)); + foreach(elt, reloids) + { + Oid classoid = lfirst_oid(elt); + + tuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(classoid)); + if (!HeapTupleIsValid(tuple)) + elog(ERROR, "cache lookup failed for relation %u", classoid); + rd_rel = (Form_pg_class) GETSTRUCT(tuple); + + rd_rel->relpersistence = newrelpersistence; + CatalogTupleUpdate(pg_class, &tuple->t_self, tuple); + InvokeObjectPostAlterHook(RelationRelationId, classoid, 0); + + heap_freetuple(tuple); + } + + /* Make sure the persistence changes are visible */ + CommandCounterIncrement(); + + table_close(pg_class, RowExclusiveLock); +} + /* * ALTER TABLE SET TABLESPACE */ @@ -17749,12 +17844,10 @@ ATExecSetCompression(Relation rel, * This verifies that we're not trying to change a temp table. Also, * existing foreign key constraints are checked to avoid ending up with * permanent tables referencing unlogged tables. - * - * Return value is false if the operation is a no-op (in which case the - * checks are skipped), otherwise true. */ -static bool -ATPrepChangePersistence(Relation rel, bool toLogged) +static void +ATPrepSetPersistence(AlteredTableInfo *tab, Relation rel, + bool toLogged) { Relation pg_constraint; HeapTuple tuple; @@ -17778,12 +17871,12 @@ ATPrepChangePersistence(Relation rel, bool toLogged) case RELPERSISTENCE_PERMANENT: if (toLogged) /* nothing to do */ - return false; + return; break; case RELPERSISTENCE_UNLOGGED: if (!toLogged) /* nothing to do */ - return false; + return; break; } @@ -17866,7 +17959,13 @@ ATPrepChangePersistence(Relation rel, bool toLogged) table_close(pg_constraint, AccessShareLock); - return true; + /* force rewrite if necessary; see comment in ATRewriteTables */ + tab->rewrite |= AT_REWRITE_ALTER_PERSISTENCE; + if (toLogged) + tab->newrelpersistence = RELPERSISTENCE_PERMANENT; + else + tab->newrelpersistence = RELPERSISTENCE_UNLOGGED; + tab->chgPersistence = true; } /* diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out index 7666c76238..5f3caf6baf 100644 --- a/src/test/regress/expected/alter_table.out +++ b/src/test/regress/expected/alter_table.out @@ -3597,6 +3597,90 @@ ALTER TABLE logged1 SET UNLOGGED; -- silently do nothing DROP TABLE logged3; DROP TABLE logged2; DROP TABLE logged1; +-- SET LOGGED/UNLOGGED with partitioned tables +CREATE TABLE logged_part_1(f1 SERIAL PRIMARY KEY) + PARTITION BY LIST (f1); -- has sequence, index +CREATE TABLE logged_part_2(f1 SERIAL PRIMARY KEY, f2 INTEGER REFERENCES logged_part_1) + PARTITION BY LIST (f1); -- foreign key +CREATE TABLE logged_part_3(f1 SERIAL PRIMARY KEY, f2 INTEGER REFERENCES logged_part_3) + PARTITION BY LIST (f1); -- self-referencing foreign key +-- Check relpersistence of all the objects created. +SELECT relname, relpersistence FROM pg_class WHERE relname ~ '^logged_part' + ORDER BY relname; + relname | relpersistence +----------------------+---------------- + logged_part_1 | p + logged_part_1_f1_seq | p + logged_part_1_pkey | p + logged_part_2 | p + logged_part_2_f1_seq | p + logged_part_2_pkey | p + logged_part_3 | p + logged_part_3_f1_seq | p + logged_part_3_pkey | p +(9 rows) + +ALTER TABLE logged_part_1 SET UNLOGGED; -- fails as a foreign-key exists +ERROR: could not change table "logged_part_1" to unlogged because it references logged table "logged_part_2" +ALTER TABLE logged_part_2 SET UNLOGGED; +ALTER TABLE logged_part_3 SET UNLOGGED; -- skip self-referencing foreign key +-- Re-check relpersistence of all the objects created. +SELECT relname, relpersistence FROM pg_class WHERE relname ~ '^logged_part' + ORDER BY relname; + relname | relpersistence +----------------------+---------------- + logged_part_1 | p + logged_part_1_f1_seq | p + logged_part_1_pkey | p + logged_part_2 | u + logged_part_2_f1_seq | u + logged_part_2_pkey | u + logged_part_3 | u + logged_part_3_f1_seq | u + logged_part_3_pkey | u +(9 rows) + +ALTER TABLE logged_part_1 SET LOGGED; -- no-op +ALTER TABLE logged_part_2 SET LOGGED; +ALTER TABLE logged_part_3 SET LOGGED; -- skip self-referencing foreign key +SELECT relname, relpersistence FROM pg_class WHERE relname ~ '^logged_part_[2|3]' + ORDER BY relname; + relname | relpersistence +----------------------+---------------- + logged_part_2 | p + logged_part_2_f1_seq | p + logged_part_2_pkey | p + logged_part_3 | p + logged_part_3_f1_seq | p + logged_part_3_pkey | p +(6 rows) + +-- Partitions +CREATE TABLE logged_part_2_1 PARTITION OF logged_part_2 + FOR VALUES IN (1); -- permanent, inherited +CREATE UNLOGGED TABLE logged_part_2_2 PARTITION OF logged_part_2 + FOR VALUES IN (2); -- unlogged, not inherited +ALTER TABLE logged_part_2 SET UNLOGGED; +CREATE TABLE logged_part_2_3 PARTITION OF logged_part_2 + FOR VALUES IN (3); -- unlogged, inherited +SELECT relname, relpersistence FROM pg_class WHERE relname ~ '^logged_part_2' + ORDER BY relname; + relname | relpersistence +----------------------+---------------- + logged_part_2 | u + logged_part_2_1 | p + logged_part_2_1_pkey | p + logged_part_2_2 | u + logged_part_2_2_pkey | u + logged_part_2_3 | u + logged_part_2_3_pkey | u + logged_part_2_f1_seq | u + logged_part_2_pkey | u +(9 rows) + +DROP TABLE logged_part_3; +DROP TABLE logged_part_2; +DROP TABLE logged_part_1; -- test ADD COLUMN IF NOT EXISTS CREATE TABLE test_add_column(c1 integer); \d test_add_column diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql index 9df5a63bdf..903e3aa217 100644 --- a/src/test/regress/sql/alter_table.sql +++ b/src/test/regress/sql/alter_table.sql @@ -2259,6 +2259,41 @@ DROP TABLE logged3; DROP TABLE logged2; DROP TABLE logged1; +-- SET LOGGED/UNLOGGED with partitioned tables +CREATE TABLE logged_part_1(f1 SERIAL PRIMARY KEY) + PARTITION BY LIST (f1); -- has sequence, index +CREATE TABLE logged_part_2(f1 SERIAL PRIMARY KEY, f2 INTEGER REFERENCES logged_part_1) + PARTITION BY LIST (f1); -- foreign key +CREATE TABLE logged_part_3(f1 SERIAL PRIMARY KEY, f2 INTEGER REFERENCES logged_part_3) + PARTITION BY LIST (f1); -- self-referencing foreign key +-- Check relpersistence of all the objects created. +SELECT relname, relpersistence FROM pg_class WHERE relname ~ '^logged_part' + ORDER BY relname; +ALTER TABLE logged_part_1 SET UNLOGGED; -- fails as a foreign-key exists +ALTER TABLE logged_part_2 SET UNLOGGED; +ALTER TABLE logged_part_3 SET UNLOGGED; -- skip self-referencing foreign key +-- Re-check relpersistence of all the objects created. +SELECT relname, relpersistence FROM pg_class WHERE relname ~ '^logged_part' + ORDER BY relname; +ALTER TABLE logged_part_1 SET LOGGED; -- no-op +ALTER TABLE logged_part_2 SET LOGGED; +ALTER TABLE logged_part_3 SET LOGGED; -- skip self-referencing foreign key +SELECT relname, relpersistence FROM pg_class WHERE relname ~ '^logged_part_[2|3]' + ORDER BY relname; +-- Partitions +CREATE TABLE logged_part_2_1 PARTITION OF logged_part_2 + FOR VALUES IN (1); -- permanent, inherited +CREATE UNLOGGED TABLE logged_part_2_2 PARTITION OF logged_part_2 + FOR VALUES IN (2); -- unlogged, not inherited +ALTER TABLE logged_part_2 SET UNLOGGED; +CREATE TABLE logged_part_2_3 PARTITION OF logged_part_2 + FOR VALUES IN (3); -- unlogged, inherited +SELECT relname, relpersistence FROM pg_class WHERE relname ~ '^logged_part_2' + ORDER BY relname; +DROP TABLE logged_part_3; +DROP TABLE logged_part_2; +DROP TABLE logged_part_1; + -- test ADD COLUMN IF NOT EXISTS CREATE TABLE test_add_column(c1 integer); \d test_add_column diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml index fe36ff82e5..b85d8dcd15 100644 --- a/doc/src/sgml/ref/alter_table.sgml +++ b/doc/src/sgml/ref/alter_table.sgml @@ -803,6 +803,12 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM (for identity or serial columns). However, it is also possible to change the persistence of such sequences separately. </para> + + <para> + Setting this property for a partitioned table has no direct effect, + because such tables have no storage of their own, but the configured + value will be inherited by newly-created partitions. + </para> </listitem> </varlistentry> diff --git a/doc/src/sgml/ref/create_table.sgml b/doc/src/sgml/ref/create_table.sgml index 02f31d2d6f..d1ff9d9e95 100644 --- a/doc/src/sgml/ref/create_table.sgml +++ b/doc/src/sgml/ref/create_table.sgml @@ -221,6 +221,11 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM If this is specified, any sequences created together with the unlogged table (for identity or serial columns) are also created as unlogged. </para> + + <para> + When applied to a partitioned table, newly-created partitions and + their objects (sequences and indexes) will inherit this property. + </para> </listitem> </varlistentry> -- 2.43.0 [application/pgp-signature] signature.asc (833B, ../../[email protected]/3-signature.asc) download ^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: Partitioned tables and [un]loggedness @ 2024-04-24 20:26 Nathan Bossart <[email protected]> parent: Michael Paquier <[email protected]> 0 siblings, 0 replies; 3+ messages in thread From: Nathan Bossart @ 2024-04-24 20:26 UTC (permalink / raw) To: Michael Paquier <[email protected]>; +Cc: Postgres hackers <[email protected]> On Wed, Apr 24, 2024 at 04:17:44PM +0900, Michael Paquier wrote: > - Support ALTER TABLE .. SET LOGGED/UNLOGGED for partitioned tables, > where the command only works on partitioned tables so that's only a > catalog switch. I'm not following what this means. Does SET [UN]LOGGED on a partitioned table recurse to its partitions? Does this mean that you cannot changed whether a single partition is [UN]LOGGED? How does this work with sub-partitioning? > - CREATE TABLE PARTITION OF would make a new partition inherit the > logged ness of the parent if UNLOGGED is not directly specified. This one seems generally reasonable to me, provided it's properly noted in the docs. > - How about ONLY? Would it make sense to support it so as ALTER TABLE > ONLY on a partitioned table does not touch any of its partitions? > Would not specifying ONLY mean that the loggedness of the partitioned > table and all its partitions is changed? That would mean a burst in > WAL when switching to LOGGED, which was a concern when this feature > was first implemented even for a single table, so for potentially > hundreds of them, that would really hurt if a DBA is not careful. I guess ONLY could be a way of changing the default for new partitions without changing whether existing ones were logged. I'm not tremendously concerned about the burst-of-WAL problem. Or, at least, I'm not any more concerned about it for partitioned tables than I am for regular tables. I do wonder if we can improve the performance of setting tables LOGGED, but that's for a separate thread... > - CREATE TABLE does not have a LOGGED keyword, hence it is not > possible through the parser to force a partition to have a permanent > persistence even if its partitioned table uses UNLOGGED. Could we add LOGGED for CREATE TABLE? -- Nathan Bossart Amazon Web Services: https://aws.amazon.com ^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2024-04-24 20:26 UTC | newest] Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2021-03-04 03:53 [PATCH 5/5] WIP unify handling of attributes and expressions Tomas Vondra <[email protected]> 2024-04-24 07:17 Partitioned tables and [un]loggedness Michael Paquier <[email protected]> 2024-04-24 20:26 ` Re: Partitioned tables and [un]loggedness Nathan Bossart <[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