From 7021bf02a373779c2792d14fcaf3af258ad05342 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Mon, 19 Jan 2026 15:48:01 +0900 Subject: [PATCH v28 3/3] Several fixes and adjustments --- .../statistics/extended_stats_internal.h | 12 ++- src/backend/statistics/extended_stats_funcs.c | 93 ++++++++++++------- src/backend/statistics/mcv.c | 74 ++++++++------- src/bin/pg_dump/pg_dump.c | 8 +- src/test/regress/expected/stats_import.out | 2 +- doc/src/sgml/func/func-admin.sgml | 2 +- 6 files changed, 112 insertions(+), 79 deletions(-) diff --git a/src/include/statistics/extended_stats_internal.h b/src/include/statistics/extended_stats_internal.h index f7cc1f65e0df..7bf0bf3b008a 100644 --- a/src/include/statistics/extended_stats_internal.h +++ b/src/include/statistics/extended_stats_internal.h @@ -89,6 +89,13 @@ extern MCVList *statext_mcv_build(StatsBuildData *data, double totalrows, int stattarget); extern bytea *statext_mcv_serialize(MCVList *mcvlist, VacAttrStats **stats); extern MCVList *statext_mcv_deserialize(bytea *data); +extern void statext_mcv_free(MCVList *mcvlist); +extern Datum statext_mcv_import(HeapTuple tup, int elevel, + int numattrs, Oid *atttypids, + int32 *atttypmods, Oid *atttypcolls, + int nitems, Datum *mcv_elems, + bool *mcv_nulls, bool *mcv_elem_nulls, + float8 *freqs, float8 *base_freqs); extern MultiSortSupport multi_sort_init(int ndims); extern void multi_sort_add_dimension(MultiSortSupport mss, int sortdim, @@ -135,9 +142,4 @@ extern Selectivity mcv_clause_selectivity_or(PlannerInfo *root, Selectivity *overlap_basesel, Selectivity *totalsel); -extern Datum import_mcvlist(HeapTuple tup, int elevel, int numattrs, - Oid *atttypids, int32 *atttypmods, Oid *atttypcolls, - int nitems, Datum *mcv_elems, bool *mcv_nulls, - bool *mcv_elem_nulls, float8 *freqs, float8 *base_freqs); - #endif /* EXTENDED_STATS_INTERNAL_H */ diff --git a/src/backend/statistics/extended_stats_funcs.c b/src/backend/statistics/extended_stats_funcs.c index 3622e5729252..d867445d85c7 100644 --- a/src/backend/statistics/extended_stats_funcs.c +++ b/src/backend/statistics/extended_stats_funcs.c @@ -252,14 +252,24 @@ expand_stxkind(HeapTuple tup, StakindFlags *enabled) for (int i = 0; i < ARR_DIMS(arr)[0]; i++) { - if (kinds[i] == STATS_EXT_NDISTINCT) - enabled->ndistinct = true; - else if (kinds[i] == STATS_EXT_DEPENDENCIES) - enabled->dependencies = true; - else if (kinds[i] == STATS_EXT_MCV) - enabled->mcv = true; - else if (kinds[i] == STATS_EXT_EXPRESSIONS) - enabled->expressions = true; + switch (kinds[i]) + { + case STATS_EXT_NDISTINCT: + enabled->ndistinct = true; + break; + case STATS_EXT_DEPENDENCIES: + enabled->dependencies = true; + break; + case STATS_EXT_MCV: + enabled->mcv = true; + break; + case STATS_EXT_EXPRESSIONS: + enabled->expressions = true; + break; + default: + elog(ERROR, "incorrect stxkind %c found", kinds[i]); + break; + } } } @@ -304,12 +314,13 @@ upsert_pg_statistic_ext_data(const Datum *values, const bool *nulls, } /* - * Insert or Update Extended Statistics + * Insert or update an extended statistics object. * - * Major errors, such as the table not existing, the statistics object not - * existing, or a permissions failure are always reported at ERROR. Other - * errors, such as a conversion failure on one statistic kind, are reported - * as WARNINGs, and other statistic kinds may still be updated. + * Major errors, such as the table not existing or permission errors, are + * reported as ERRORs. There are a couple of paths that generate a WARNING, + * like when the statistics object or its schema do not exist, a conversion + * failure on one statistic kind, or when other statistic kinds may still + * be updated. */ static bool extended_statistics_update(FunctionCallInfo fcinfo) @@ -365,7 +376,7 @@ extended_statistics_update(FunctionCallInfo fcinfo) errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), errmsg("recovery is in progress"), errhint("Statistics cannot be modified during recovery.")); - PG_RETURN_BOOL(false); + return false; } /* relation arguments */ @@ -397,8 +408,8 @@ extended_statistics_update(FunctionCallInfo fcinfo) { ereport(WARNING, errcode(ERRCODE_UNDEFINED_OBJECT), - errmsg("could not find schema \"%s\"", stxname)); - PG_RETURN_BOOL(false); + errmsg("could not find schema \"%s\"", nspname)); + return false; } pg_stext = table_open(StatisticExtRelationId, RowExclusiveLock); @@ -411,7 +422,7 @@ extended_statistics_update(FunctionCallInfo fcinfo) errcode(ERRCODE_UNDEFINED_OBJECT), errmsg("could not find extended statistics object \"%s\".\"%s\"", get_namespace_name(nspoid), stxname)); - PG_RETURN_BOOL(false); + return false; } stxform = (Form_pg_statistic_ext) GETSTRUCT(tup); @@ -428,7 +439,7 @@ extended_statistics_update(FunctionCallInfo fcinfo) errmsg("could not restore extended statistics object \"%s\".\"%s\": incorrect relation \"%s\".\"%s\" specified", get_namespace_name(nspoid), stxname, relnspname, relname)); - PG_RETURN_BOOL(false); + return false; } expand_stxkind(tup, &enabled); @@ -665,7 +676,7 @@ extended_statistics_update(FunctionCallInfo fcinfo) errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("could not use parameter \"%s\": expected text array of 2 dimensions", extarginfo[MOST_COMMON_VALS_ARG].argname)); - return (Datum) 0; + return false; } nitems = ARR_DIMS(mcv_arr)[0]; @@ -677,20 +688,19 @@ extended_statistics_update(FunctionCallInfo fcinfo) 1, nitems) || !check_mcvlist_array(base_freqs_arr, MOST_COMMON_BASE_FREQS_ARG, 1, nitems)) - return (Datum) 0; - + return false; deconstruct_array_builtin(mcv_arr, TEXTOID, &mcv_elems, &mcv_nulls, &check_nummcv); Assert(check_nummcv == (nitems * numattrs)); - datum = import_mcvlist(tup, WARNING, numattrs, - atttypids, atttypmods, atttypcolls, - nitems, mcv_elems, mcv_nulls, - (bool *) ARR_DATA_PTR(nulls_arr), - (float8 *) ARR_DATA_PTR(freqs_arr), - (float8 *) ARR_DATA_PTR(base_freqs_arr)); + datum = statext_mcv_import(tup, WARNING, numattrs, + atttypids, atttypmods, atttypcolls, + nitems, mcv_elems, mcv_nulls, + (bool *) ARR_DATA_PTR(nulls_arr), + (float8 *) ARR_DATA_PTR(freqs_arr), + (float8 *) ARR_DATA_PTR(base_freqs_arr)); values[Anum_pg_statistic_ext_data_stxdmcv - 1] = datum; replaces[Anum_pg_statistic_ext_data_stxdmcv - 1] = true; @@ -780,9 +790,9 @@ check_mcvlist_array(ArrayType *arr, int argindex, int required_ndims, } /* - * Warn of type mismatch. Common pattern. + * Warn of type mismatch. */ -static Datum +static void warn_type_mismatch(Datum d, const char *argname) { char *s = TextDatumGetCString(d); @@ -791,7 +801,6 @@ warn_type_mismatch(Datum d, const char *argname) errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("could not match expression %s element \"%s\" with input type", argname, s)); - return (Datum) 0; } /* @@ -912,8 +921,11 @@ import_expressions(Relation pgsd, int numexprs, &values[Anum_pg_statistic_stanullfrac - 1]); if (!ok) - return warn_type_mismatch(exprs_elems[null_frac_idx], - extexprarginfo[NULL_FRAC_ELEM].argname); + { + warn_type_mismatch(exprs_elems[null_frac_idx], + extexprarginfo[NULL_FRAC_ELEM].argname); + return (Datum) 0; + } } if (!exprs_nulls[avg_width_idx]) @@ -922,8 +934,11 @@ import_expressions(Relation pgsd, int numexprs, &values[Anum_pg_statistic_stawidth - 1]); if (!ok) - return warn_type_mismatch(exprs_elems[avg_width_idx], - extexprarginfo[AVG_WIDTH_ELEM].argname); + { + warn_type_mismatch(exprs_elems[avg_width_idx], + extexprarginfo[AVG_WIDTH_ELEM].argname); + return (Datum) 0; + } } if (!exprs_nulls[n_distinct_idx]) @@ -932,8 +947,11 @@ import_expressions(Relation pgsd, int numexprs, &values[Anum_pg_statistic_stadistinct - 1]); if (!ok) - return warn_type_mismatch(exprs_elems[n_distinct_idx], - extexprarginfo[N_DISTINCT_ELEM].argname); + { + warn_type_mismatch(exprs_elems[n_distinct_idx], + extexprarginfo[N_DISTINCT_ELEM].argname); + return (Datum) 0; + } } /* @@ -1025,6 +1043,7 @@ import_expressions(Relation pgsd, int numexprs, errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("could not match expression %s of element \"%s\" with input type", extexprarginfo[CORRELATION_ELEM].argname, s)); + pfree(s); return (Datum) 0; } @@ -1164,6 +1183,8 @@ delete_pg_statistic_ext_data(Oid stxoid, bool inherited) /* * Restore (insert or replace) statistics for the given statistics object. + * + * TODO add a bunch of comments here.. */ Datum pg_restore_extended_stats(PG_FUNCTION_ARGS) diff --git a/src/backend/statistics/mcv.c b/src/backend/statistics/mcv.c index f4f6836149a5..3f17c4903ce3 100644 --- a/src/backend/statistics/mcv.c +++ b/src/backend/statistics/mcv.c @@ -2173,21 +2173,39 @@ mcv_clause_selectivity_or(PlannerInfo *root, StatisticExtInfo *stat, } /* - * The MCV is an array of records, but this is expected as 4 separate arrays. - * It is not possible to have a generic input function for pg_mcv_list - * because the most_common_values is a composite type with element types - * defined by the specific statistics object. - */ + * Free allocations of a MCVList. + */ +void +statext_mcv_free(MCVList *mcvlist) +{ + for (int i = 0; i < mcvlist->nitems; i++) + { + MCVItem *item = &mcvlist->items[i]; + + pfree(item->values); + pfree(item->isnull); + } + pfree(mcvlist); +} + +/* + * The MCV is an array of records, but this is expected as 4 separate arrays. + * It is not possible to have a generic input function for pg_mcv_list + * because the most_common_values is a composite type with element types + * defined by the specific statistics object. + * + * TODO: We ought to document this function, its purpose, and from where + * the inputs come from as well as what they mean. + */ Datum -import_mcvlist(HeapTuple tup, int elevel, int numattrs, Oid *atttypids, - int32 *atttypmods, Oid *atttypcolls, int nitems, - Datum *mcv_elems, bool *mcv_nulls, - bool *mcv_elem_nulls, float8 *freqs, float8 *base_freqs) +statext_mcv_import(HeapTuple tup, int elevel, int numattrs, + Oid *atttypids, int32 *atttypmods, Oid *atttypcolls, + int nitems, Datum *mcv_elems, bool *mcv_nulls, + bool *mcv_elem_nulls, float8 *freqs, + float8 *base_freqs) { MCVList *mcvlist; bytea *bytes; - - HeapTuple *vatuples; VacAttrStats **vastats; /* @@ -2246,8 +2264,9 @@ import_mcvlist(HeapTuple tup, int elevel, int numattrs, Oid *atttypids, { ereport(elevel, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("MCV elemement \"%s\" does not match expected input type.", s))); - return (Datum) 0; + errmsg("could not parse MCV element \"%s\": incorrect value", s))); + pfree(s); + goto error; } pfree(s); @@ -2263,7 +2282,6 @@ import_mcvlist(HeapTuple tup, int elevel, int numattrs, Oid *atttypids, * to be filled out. */ vastats = (VacAttrStats **) palloc0_array(VacAttrStats *, numattrs); - vatuples = (HeapTuple *) palloc0_array(HeapTuple, numattrs); for (int i = 0; i < numattrs; i++) { @@ -2275,8 +2293,6 @@ import_mcvlist(HeapTuple tup, int elevel, int numattrs, Oid *atttypids, if (!HeapTupleIsValid(typtuple)) elog(ERROR, "cache lookup failed for type %u", typid); - vatuples[i] = typtuple; - vastats[i] = palloc0_object(VacAttrStats); vastats[i]->attrtype = (Form_pg_type) GETSTRUCT(typtuple); @@ -2288,30 +2304,24 @@ import_mcvlist(HeapTuple tup, int elevel, int numattrs, Oid *atttypids, for (int i = 0; i < numattrs; i++) { - pfree(vatuples[i]); pfree(vastats[i]); } - pfree((void *) vatuples); - pfree((void *) vastats); + pfree(vastats); + + pfree(mcv_elems); + pfree(mcv_nulls); if (bytes == NULL) { ereport(elevel, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("Unable to import mcv list"))); - return (Datum) 0; + errmsg("could not import MCV list"))); + goto error; } - for (int i = 0; i < nitems; i++) - { - MCVItem *item = &mcvlist->items[i]; - - pfree(item->values); - pfree(item->isnull); - } - pfree(mcvlist); - pfree(mcv_elems); - pfree(mcv_nulls); - return PointerGetDatum(bytes); + +error: + statext_mcv_free(mcvlist); + return (Datum) 0; } diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index f2e48fc691f9..88b917b29f4b 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -18555,7 +18555,7 @@ dumpStatisticsExtStats(Archive *fout, const StatsExtInfo *statsextinfo) PQExpBuffer pq = createPQExpBuffer(); /* - * Set up query for constraint-specific details. + * Set up query for details about extended statistics objects. * * 19+: query pg_stats_ext and pg_stats_ext_exprs as-is 15-18: query * pg_stats_ext translating the ndistinct and dependencies, 14: @@ -18579,7 +18579,7 @@ dumpStatisticsExtStats(Archive *fout, const StatsExtInfo *statsextinfo) appendPQExpBufferStr(pq, "false AS inherited, "); /* - * The ndistinnct and depdendencies formats changed in v19, so + * The ndistinct and dependencies formats changed in v19, so * everything before that needs to be translated. * * The ndistinct translation converts this: @@ -18678,11 +18678,11 @@ dumpStatisticsExtStats(Archive *fout, const StatsExtInfo *statsextinfo) "FROM ( " "SELECT s.stxndistinct AS n_distinct, " " s.stxdependencies AS dependencies " - "FROM pg_catalog.pg_statistics_ext AS s " + "FROM pg_catalog.pg_statistic_ext AS s " "JOIN pg_catalog.pg_namespace AS n " "ON n.oid = s.stxnamespace " "WHERE n.nspname = $1 " - "AND e.stxname = $2 " + "AND s.stxname = $2 " ") AS e "); /* we always have an inherited column, but it may be a constant */ diff --git a/src/test/regress/expected/stats_import.out b/src/test/regress/expected/stats_import.out index e3a993f9fcf7..63e84a099487 100644 --- a/src/test/regress/expected/stats_import.out +++ b/src/test/regress/expected/stats_import.out @@ -1622,7 +1622,7 @@ SELECT pg_catalog.pg_restore_extended_stats( 'statistics_schemaname', 'schema_not_exist', 'statistics_name', 'test_stat_clone', 'inherited', false); -WARNING: could not find schema "test_stat_clone" +WARNING: could not find schema "schema_not_exist" pg_restore_extended_stats --------------------------- f diff --git a/doc/src/sgml/func/func-admin.sgml b/doc/src/sgml/func/func-admin.sgml index da3719e72b45..b9c1b7362f69 100644 --- a/doc/src/sgml/func/func-admin.sgml +++ b/doc/src/sgml/func/func-admin.sgml @@ -2205,7 +2205,7 @@ SELECT pg_restore_attribute_stats( 'statistics_name', 'stats_name'::name, 'inherited', false, 'n_distinct', '{"2, 3": 4, "2, -1": 4, "2, -2": 4, "3, -1": 4, "3, -2": 4}'::pg_ndistinct, - 'dependencies', '{"2 => 1": 1.000000, "2 => -1": 1.000000, "2 => -2": 1.000000}'::pg_dependencies + 'dependencies', '{"2 => 1": 1.000000, "2 => -1": 1.000000, "2 => -2": 1.000000}'::pg_dependencies, 'exprs', '{{0,4,-0.75,"{1}","{0.5}","{-1,0}",-0.6,NULL,NULL,NULL},{0.25,4,-0.5,"{2}","{0.5}",NULL,1,NULL,NULL,NULL}}'::text[]); -- 2.51.0