From a1af0362f1215d5e6f2027888ff47ff7c35e9bac Mon Sep 17 00:00:00 2001 From: shihao zhong Date: Mon, 9 Nov 2026 17:04:25 -0500 Subject: [PATCH v3 3/3] Share the pg_statistic tuple building code with expression stats update_attstats() and serialize_expr_stats() had two copies of the code that turns a VacAttrStats into pg_statistic column values. Bug #19849 came from changing only one of them. Move that code into a new function, fill_attstats_values(), and call it from both places. No behavior change. Discussion: https://postgr.es/m/19849-38b8bd2fea807154@postgresql.org --- src/backend/commands/analyze.c | 153 +++++++++++++----------- src/backend/statistics/extended_stats.c | 78 +----------- src/include/commands/vacuum.h | 2 + 3 files changed, 88 insertions(+), 145 deletions(-) diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c index ab3848443a7..d5a3b2e9cef 100644 --- a/src/backend/commands/analyze.c +++ b/src/backend/commands/analyze.c @@ -1713,6 +1713,87 @@ acquire_inherited_sample_rows(Relation onerel, int elevel, } +/* + * fill_attstats_values() -- fill in pg_statistic columns from a VacAttrStats + * + * values[] and nulls[] must have room for Natts_pg_statistic entries. All + * columns are filled in except starelid, staattnum and stainherit, which + * the caller must set. This is used both for regular column statistics and + * for statistics on the expressions of extended statistics objects. + */ +void +fill_attstats_values(VacAttrStats *stats, Datum *values, bool *nulls) +{ + int i, + k; + + for (i = 0; i < Natts_pg_statistic; ++i) + { + nulls[i] = false; + values[i] = (Datum) 0; + } + + values[Anum_pg_statistic_stanullfrac - 1] = Float4GetDatum(stats->stanullfrac); + values[Anum_pg_statistic_stawidth - 1] = Int32GetDatum(stats->stawidth); + values[Anum_pg_statistic_stadistinct - 1] = Float4GetDatum(stats->stadistinct); + i = Anum_pg_statistic_stakind1 - 1; + for (k = 0; k < STATISTIC_NUM_SLOTS; k++) + { + values[i++] = Int16GetDatum(stats->stakind[k]); /* stakindN */ + } + i = Anum_pg_statistic_staop1 - 1; + for (k = 0; k < STATISTIC_NUM_SLOTS; k++) + { + values[i++] = ObjectIdGetDatum(stats->staop[k]); /* staopN */ + } + i = Anum_pg_statistic_stacoll1 - 1; + for (k = 0; k < STATISTIC_NUM_SLOTS; k++) + { + values[i++] = ObjectIdGetDatum(stats->stacoll[k]); /* stacollN */ + } + i = Anum_pg_statistic_stanumbers1 - 1; + for (k = 0; k < STATISTIC_NUM_SLOTS; k++) + { + if (stats->stanumbers[k] != NULL) + { + int nnum = stats->numnumbers[k]; + Datum *numdatums = palloc_array(Datum, nnum); + ArrayType *arry; + + for (int n = 0; n < nnum; n++) + numdatums[n] = Float4GetDatum(stats->stanumbers[k][n]); + arry = construct_array_builtin(numdatums, nnum, FLOAT4OID); + values[i++] = PointerGetDatum(arry); /* stanumbersN */ + } + else + { + nulls[i] = true; + values[i++] = (Datum) 0; + } + } + i = Anum_pg_statistic_stavalues1 - 1; + for (k = 0; k < STATISTIC_NUM_SLOTS; k++) + { + if (stats->stavalues[k] != NULL) + { + ArrayType *arry; + + arry = construct_array(stats->stavalues[k], + stats->numvalues[k], + stats->statypid[k], + stats->statyplen[k], + stats->statypbyval[k], + stats->statypalign[k]); + values[i++] = PointerGetDatum(arry); /* stavaluesN */ + } + else + { + nulls[i] = true; + values[i++] = (Datum) 0; + } + } +} + /* * update_attstats() -- update attribute statistics for one relation * @@ -1752,9 +1833,7 @@ update_attstats(Oid relid, bool inh, int natts, VacAttrStats **vacattrstats) VacAttrStats *stats = vacattrstats[attno]; HeapTuple stup, oldtup; - int i, - k, - n; + int i; Datum values[Natts_pg_statistic]; bool nulls[Natts_pg_statistic]; bool replaces[Natts_pg_statistic]; @@ -1764,79 +1843,15 @@ update_attstats(Oid relid, bool inh, int natts, VacAttrStats **vacattrstats) continue; /* - * Construct a new pg_statistic tuple. - * - * serialize_expr_stats() in extended_stats.c builds pg_statistic - * tuples the same way, so keep the two in sync. + * Construct a new pg_statistic tuple */ for (i = 0; i < Natts_pg_statistic; ++i) - { - nulls[i] = false; replaces[i] = true; - } + fill_attstats_values(stats, values, nulls); values[Anum_pg_statistic_starelid - 1] = ObjectIdGetDatum(relid); values[Anum_pg_statistic_staattnum - 1] = Int16GetDatum(stats->tupattnum); values[Anum_pg_statistic_stainherit - 1] = BoolGetDatum(inh); - values[Anum_pg_statistic_stanullfrac - 1] = Float4GetDatum(stats->stanullfrac); - values[Anum_pg_statistic_stawidth - 1] = Int32GetDatum(stats->stawidth); - values[Anum_pg_statistic_stadistinct - 1] = Float4GetDatum(stats->stadistinct); - i = Anum_pg_statistic_stakind1 - 1; - for (k = 0; k < STATISTIC_NUM_SLOTS; k++) - { - values[i++] = Int16GetDatum(stats->stakind[k]); /* stakindN */ - } - i = Anum_pg_statistic_staop1 - 1; - for (k = 0; k < STATISTIC_NUM_SLOTS; k++) - { - values[i++] = ObjectIdGetDatum(stats->staop[k]); /* staopN */ - } - i = Anum_pg_statistic_stacoll1 - 1; - for (k = 0; k < STATISTIC_NUM_SLOTS; k++) - { - values[i++] = ObjectIdGetDatum(stats->stacoll[k]); /* stacollN */ - } - i = Anum_pg_statistic_stanumbers1 - 1; - for (k = 0; k < STATISTIC_NUM_SLOTS; k++) - { - if (stats->stanumbers[k] != NULL) - { - int nnum = stats->numnumbers[k]; - Datum *numdatums = palloc_array(Datum, nnum); - ArrayType *arry; - - for (n = 0; n < nnum; n++) - numdatums[n] = Float4GetDatum(stats->stanumbers[k][n]); - arry = construct_array_builtin(numdatums, nnum, FLOAT4OID); - values[i++] = PointerGetDatum(arry); /* stanumbersN */ - } - else - { - nulls[i] = true; - values[i++] = (Datum) 0; - } - } - i = Anum_pg_statistic_stavalues1 - 1; - for (k = 0; k < STATISTIC_NUM_SLOTS; k++) - { - if (stats->stavalues[k] != NULL) - { - ArrayType *arry; - - arry = construct_array(stats->stavalues[k], - stats->numvalues[k], - stats->statypid[k], - stats->statyplen[k], - stats->statypbyval[k], - stats->statypalign[k]); - values[i++] = PointerGetDatum(arry); /* stavaluesN */ - } - else - { - nulls[i] = true; - values[i++] = (Datum) 0; - } - } /* Is there already a pg_statistic tuple for this attribute? */ oldtup = SearchSysCache3(STATRELATTINH, diff --git a/src/backend/statistics/extended_stats.c b/src/backend/statistics/extended_stats.c index 14652a36331..6211b23d729 100644 --- a/src/backend/statistics/extended_stats.c +++ b/src/backend/statistics/extended_stats.c @@ -2333,12 +2333,7 @@ build_expr_data(List *exprs, int stattarget) return exprdata; } -/* - * Form an array of pg_statistic rows, one per expression. - * - * Each row is built the same way update_attstats() in analyze.c builds - * it, so keep the two in sync. - */ +/* form an array of pg_statistic rows, one per expression */ static Datum serialize_expr_stats(AnlExprData *exprdata, int nexprs) { @@ -2360,8 +2355,6 @@ serialize_expr_stats(AnlExprData *exprdata, int nexprs) for (exprno = 0; exprno < nexprs; exprno++) { - int i, - k; VacAttrStats *stats = exprdata[exprno].vacattrstat; Datum values[Natts_pg_statistic]; @@ -2378,77 +2371,10 @@ serialize_expr_stats(AnlExprData *exprdata, int nexprs) continue; } - /* - * Construct a new pg_statistic tuple - */ - for (i = 0; i < Natts_pg_statistic; ++i) - { - nulls[i] = false; - } - + fill_attstats_values(stats, values, nulls); values[Anum_pg_statistic_starelid - 1] = ObjectIdGetDatum(InvalidOid); values[Anum_pg_statistic_staattnum - 1] = Int16GetDatum(InvalidAttrNumber); values[Anum_pg_statistic_stainherit - 1] = BoolGetDatum(false); - values[Anum_pg_statistic_stanullfrac - 1] = Float4GetDatum(stats->stanullfrac); - values[Anum_pg_statistic_stawidth - 1] = Int32GetDatum(stats->stawidth); - values[Anum_pg_statistic_stadistinct - 1] = Float4GetDatum(stats->stadistinct); - i = Anum_pg_statistic_stakind1 - 1; - for (k = 0; k < STATISTIC_NUM_SLOTS; k++) - { - values[i++] = Int16GetDatum(stats->stakind[k]); /* stakindN */ - } - i = Anum_pg_statistic_staop1 - 1; - for (k = 0; k < STATISTIC_NUM_SLOTS; k++) - { - values[i++] = ObjectIdGetDatum(stats->staop[k]); /* staopN */ - } - i = Anum_pg_statistic_stacoll1 - 1; - for (k = 0; k < STATISTIC_NUM_SLOTS; k++) - { - values[i++] = ObjectIdGetDatum(stats->stacoll[k]); /* stacollN */ - } - i = Anum_pg_statistic_stanumbers1 - 1; - for (k = 0; k < STATISTIC_NUM_SLOTS; k++) - { - if (stats->stanumbers[k] != NULL) - { - int nnum = stats->numnumbers[k]; - int n; - Datum *numdatums = palloc_array(Datum, nnum); - ArrayType *arry; - - for (n = 0; n < nnum; n++) - numdatums[n] = Float4GetDatum(stats->stanumbers[k][n]); - arry = construct_array_builtin(numdatums, nnum, FLOAT4OID); - values[i++] = PointerGetDatum(arry); /* stanumbersN */ - } - else - { - nulls[i] = true; - values[i++] = (Datum) 0; - } - } - i = Anum_pg_statistic_stavalues1 - 1; - for (k = 0; k < STATISTIC_NUM_SLOTS; k++) - { - if (stats->stavalues[k] != NULL) - { - ArrayType *arry; - - arry = construct_array(stats->stavalues[k], - stats->numvalues[k], - stats->statypid[k], - stats->statyplen[k], - stats->statypbyval[k], - stats->statypalign[k]); - values[i++] = PointerGetDatum(arry); /* stavaluesN */ - } - else - { - nulls[i] = true; - values[i++] = (Datum) 0; - } - } stup = heap_form_tuple(RelationGetDescr(sd), values, nulls); diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h index 89fa1fcd261..333d6451c2f 100644 --- a/src/include/commands/vacuum.h +++ b/src/include/commands/vacuum.h @@ -442,6 +442,8 @@ extern void analyze_rel(Oid relid, RangeVar *relation, extern bool attribute_is_analyzable(Relation onerel, int attnum, Form_pg_attribute attr, int *p_attstattarget); extern bool std_typanalyze(VacAttrStats *stats); +extern void fill_attstats_values(VacAttrStats *stats, Datum *values, + bool *nulls); /* in utils/misc/sampling.c --- duplicate of declarations in utils/sampling.h */ extern double anl_random_fract(void); -- 2.37.1 (Apple Git-137.1)