On Tue, Aug 25, 2026 at 03:18:25PM -0400, Corey Huinker wrote: > The big changes from the previous effort are: > > 1. Instead of having separate per-stat parameters, or a shorted stat-only > array of NullableDatum (thus requiring a separate index of datums, or some > offset), instead we're just going to accept that the _internal function > will ignore the first few elements of the NullableDatum array, specifically > the ones concerning the object identification parameters that have already > been checked/used/resolved. This removes the need for a second enum to > index the shorter array, which in turn removes the need to rename the enum > values.
Glop.
> 2. The wrapper functions around the recovery check and locking were not
> implemented, and so the *_update() functions were not removed, and so the
> the renaming of the *_update_internal() functions to the name of the
> recently vacated *_update() function is not done either. I think this
> leaves the _update() functions rather "thin" in what they do, but that
> gives us a chance to review how much further we want to go. The function
> attribute_statistics_update() is called in only by
> pg_restore_attribute_stats, and therefore could be entirely folded into
> that function. However, removing relation_statistics_update() would result
> in that code being duplicated in pg_restore_relation_stats() and
> pg_clear_relation_stats(), so that's less of a win without the stat_util
> wrapper function.
Less duplication sounds better to me for attribute_statistics_update().
> 0001-0003 phase out the use of FunctionCallInfo in any place where a simple
> NullableDatum array would suffice. It's still in 3 parts to make each
> change easier to see.
So, you have split the change so as one could see the changes across
the various API layers, with the most internal parts touched first:
first the stats_check_*() functions, second the stats update
functions, and third the places where we used the fake fcinfos
previously. For review, that's fine. I'd rather merge all three
changes together in the final result, but it's not the end of the
story for me... See below.
The incorrect comment in attribute_statistics_update_internal() was a
nice catch, incorrect since ce207d2a7901. Fixed that separately.
Traces related to LOCAL_FCINFO and InitFunctionCallInfoData() are gone
now with v2, which is nice.
Now, the real deal:
+ NullableDatum unused = {.isnull = true, .value = (Datum) 0};
[...]
+ args[ATTRELSCHEMA_ARG] = unused;
+ args[ATTRELNAME_ARG] = unused;
+ args[ATTNAME_ARG] = unused;
+ args[ATTNUM_ARG] = unused;
+ args[INHERITED_ARG] = unused;
I'm finding this part of the patch not acceptable, because it is
dictated by the fact that import_attribute_statistics() does not care
about these five unused parameters for postgres_fdw, these arguments
being required for the restore functions of the relation and attribute
stats.
To me, this points to a design defect of the postgres_fdw code,
because we pass to the import function pointers for each value from a
fcinfo then rebuild one. That's wasteful, and it complicates the
interfaces. Instead of a positional array, I think that we should use
two dedicated structures with named fields instead (one for
pg_class/rels, one for atts/pg_statistic), for relations and
attributes to avoid the guesses with the elements that may or may not
be used (aka the hardcoded unused pieces are not welcome here). That
would give for the attributes something among the lines of:
typedef struct AttStatsValues
{
NullableDatum null_frac;
NullableDatum avg_width;
/* And the rest, should be around a dozen in total */
} AttStatsValues;
With that, we should be able to bypass the positional issues, as well
as the fact that some of the parameters are not used, while cleaning
up the FDW-side import functions and all their arguments. The point
is where to make the cut due to the pairing of the arguments from the
fcinfos in the restore functions, but that's doable.
> 0004-0005 are the removal of "version" as a special parameter. They are
> strictly speaking outside the scope of $SUBJECT, so its fine if they don't
> get addressed in this thread.
Let's focus on the core proposal of the thread. I am not sure that
these are strongly necessary, TBH, this is just moving the check
of the version parameter from one place to another place.
With all that said, I have put my hands on my own idea of the problem,
using two structures shared by the fdw code and the stats restore code
to fill in the values, removing the need for a positional logic, and
finish with the attached, also leading to a negative in terms of code
lines:
8 files changed, 298 insertions(+), 333 deletions(-)
First I was wondering about the stats_check_*() functions being a
barrier, but it's easy enough to go through them knowing that we want
the argument names in the reports. The gain comes from
import_attribute_statistics() and import_relation_statistics(), that
do not need a zillion number of arguments. The fcinfos are of course
gone. In terms of the stats restore, the cut comes in
relation_statistics_update_internal() and
attribute_statistics_update_internal() which are the places where the
values in the structures are filled. We still need the array of
NullableDatums due to the pairing in stats_fill_args_from_arg_pairs()
during the stats restore that feeds from the original fcinfos. This
cut feels OK done this way, after watching the result this leads to on
the FDW side, which is much more palatable.
0001 is a merge of your original proposal, kept separated to show the
amount of changes I have done on top of it. 0002 is my refactoring
piece with the two structures for relation and attribute stats. Both
ought to be merged in a single commit, because they touch the same
places. HEAD-only cleanup; there is no way I would touch v19 at this
stage of the release cycle for a change that invasive.
So, what do you think of this v2?
--
Michael
From 41988f12359d7ac145bd773e73e047208b938a5e Mon Sep 17 00:00:00 2001 From: Michael Paquier <[email protected]> Date: Tue, 1 Sep 2026 17:05:23 +0900 Subject: [PATCH v2 1/2] Stop using FunctionCallInfo in the statistics import code All traces of LOCAL_FCINFO and InitFunctionCallInfoData() with fake fcinfos are removed from these code paths, replacing them with nullable datum arrays. Note that this is a merge of all commits from Corey, for because that was simpler for me. Author: Corey Huinker <[email protected]> --- src/include/statistics/stat_utils.h | 12 +- src/backend/statistics/attribute_stats.c | 165 ++++++++---------- src/backend/statistics/extended_stats_funcs.c | 81 +++++---- src/backend/statistics/relation_stats.c | 101 +++++------ src/backend/statistics/stat_utils.c | 44 ++--- 5 files changed, 188 insertions(+), 215 deletions(-) diff --git a/src/include/statistics/stat_utils.h b/src/include/statistics/stat_utils.h index 15e962dbb7c8..0560826fcfe5 100644 --- a/src/include/statistics/stat_utils.h +++ b/src/include/statistics/stat_utils.h @@ -25,21 +25,21 @@ struct StatsArgInfo Oid argtype; }; -extern void stats_check_required_arg(FunctionCallInfo fcinfo, +extern void stats_check_required_arg(const NullableDatum *args, struct StatsArgInfo *arginfo, int argnum); -extern bool stats_check_arg_array(FunctionCallInfo fcinfo, +extern bool stats_check_arg_array(const NullableDatum *args, struct StatsArgInfo *arginfo, int argnum); -extern bool stats_check_arg_pair(FunctionCallInfo fcinfo, +extern bool stats_check_arg_pair(const NullableDatum *args, struct StatsArgInfo *arginfo, int argnum1, int argnum2); extern void RangeVarCallbackForStats(const RangeVar *relation, Oid relId, Oid oldRelId, void *arg); -extern bool stats_fill_fcinfo_from_arg_pairs(FunctionCallInfo pairs_fcinfo, - FunctionCallInfo positional_fcinfo, - struct StatsArgInfo *arginfo); +extern bool stats_fill_args_from_arg_pairs(FunctionCallInfo pairs_fcinfo, + NullableDatum *positional_args, + struct StatsArgInfo *arginfo); extern void statatt_get_type(Oid reloid, AttrNumber attnum, Oid *atttypid, int32 *atttypmod, diff --git a/src/backend/statistics/attribute_stats.c b/src/backend/statistics/attribute_stats.c index c35892ce6d0b..5bb33e4283ae 100644 --- a/src/backend/statistics/attribute_stats.c +++ b/src/backend/statistics/attribute_stats.c @@ -104,12 +104,12 @@ static struct StatsArgInfo cleararginfo[] = [C_NUM_ATTRIBUTE_STATS_ARGS] = {0} }; -static bool attribute_statistics_update(FunctionCallInfo fcinfo); +static bool attribute_statistics_update(const NullableDatum *args); static bool attribute_statistics_update_internal(Oid reloid, const char *attname, AttrNumber attnum, bool inherited, - FunctionCallInfo fcinfo); + const NullableDatum *args); static void upsert_pg_statistic(Relation starel, HeapTuple oldtup, const Datum *values, const bool *nulls, const bool *replaces); static bool delete_pg_statistic(Oid reloid, AttrNumber attnum, bool stainherit); @@ -131,7 +131,7 @@ static bool delete_pg_statistic(Oid reloid, AttrNumber attnum, bool stainherit); * and other statistic kinds may still be updated. */ static bool -attribute_statistics_update(FunctionCallInfo fcinfo) +attribute_statistics_update(const NullableDatum *args) { char *nspname; char *relname; @@ -141,11 +141,11 @@ attribute_statistics_update(FunctionCallInfo fcinfo) bool inherited; Oid locked_table = InvalidOid; - stats_check_required_arg(fcinfo, attarginfo, ATTRELSCHEMA_ARG); - stats_check_required_arg(fcinfo, attarginfo, ATTRELNAME_ARG); + stats_check_required_arg(args, attarginfo, ATTRELSCHEMA_ARG); + stats_check_required_arg(args, attarginfo, ATTRELNAME_ARG); - nspname = TextDatumGetCString(PG_GETARG_DATUM(ATTRELSCHEMA_ARG)); - relname = TextDatumGetCString(PG_GETARG_DATUM(ATTRELNAME_ARG)); + nspname = TextDatumGetCString(args[ATTRELSCHEMA_ARG].value); + relname = TextDatumGetCString(args[ATTRELNAME_ARG].value); if (RecoveryInProgress()) ereport(ERROR, @@ -159,13 +159,13 @@ attribute_statistics_update(FunctionCallInfo fcinfo) RangeVarCallbackForStats, &locked_table); /* user can specify either attname or attnum, but not both */ - if (!PG_ARGISNULL(ATTNAME_ARG)) + if (!args[ATTNAME_ARG].isnull) { - if (!PG_ARGISNULL(ATTNUM_ARG)) + if (!args[ATTNUM_ARG].isnull) ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("cannot specify both \"%s\" and \"%s\"", "attname", "attnum"))); - attname = TextDatumGetCString(PG_GETARG_DATUM(ATTNAME_ARG)); + attname = TextDatumGetCString(args[ATTNAME_ARG].value); attnum = get_attnum(reloid, attname); /* note that this test covers attisdropped cases too: */ if (attnum == InvalidAttrNumber) @@ -174,9 +174,9 @@ attribute_statistics_update(FunctionCallInfo fcinfo) errmsg("column \"%s\" of relation \"%s\" does not exist", attname, relname))); } - else if (!PG_ARGISNULL(ATTNUM_ARG)) + else if (!args[ATTNUM_ARG].isnull) { - attnum = PG_GETARG_INT16(ATTNUM_ARG); + attnum = DatumGetInt16(args[ATTNUM_ARG].value); attname = get_attname(reloid, attnum, true); /* annoyingly, get_attname doesn't check attisdropped */ if (attname == NULL || @@ -201,11 +201,11 @@ attribute_statistics_update(FunctionCallInfo fcinfo) errmsg("cannot modify statistics on system column \"%s\"", attname))); - stats_check_required_arg(fcinfo, attarginfo, INHERITED_ARG); - inherited = PG_GETARG_BOOL(INHERITED_ARG); + stats_check_required_arg(args, attarginfo, INHERITED_ARG); + inherited = DatumGetBool(args[INHERITED_ARG].value); return attribute_statistics_update_internal(reloid, attname, attnum, - inherited, fcinfo); + inherited, args); } /* @@ -214,7 +214,7 @@ attribute_statistics_update(FunctionCallInfo fcinfo) static bool attribute_statistics_update_internal(Oid reloid, const char *attname, AttrNumber attnum, - bool inherited, FunctionCallInfo fcinfo) + bool inherited, const NullableDatum *args) { Relation starel; HeapTuple statup; @@ -231,16 +231,16 @@ attribute_statistics_update_internal(Oid reloid, FmgrInfo array_in_fn; - bool do_mcv = !PG_ARGISNULL(MOST_COMMON_FREQS_ARG) && - !PG_ARGISNULL(MOST_COMMON_VALS_ARG); - bool do_histogram = !PG_ARGISNULL(HISTOGRAM_BOUNDS_ARG); - bool do_correlation = !PG_ARGISNULL(CORRELATION_ARG); - bool do_mcelem = !PG_ARGISNULL(MOST_COMMON_ELEMS_ARG) && - !PG_ARGISNULL(MOST_COMMON_ELEM_FREQS_ARG); - bool do_dechist = !PG_ARGISNULL(ELEM_COUNT_HISTOGRAM_ARG); - bool do_bounds_histogram = !PG_ARGISNULL(RANGE_BOUNDS_HISTOGRAM_ARG); - bool do_range_length_histogram = !PG_ARGISNULL(RANGE_LENGTH_HISTOGRAM_ARG) && - !PG_ARGISNULL(RANGE_EMPTY_FRAC_ARG); + bool do_mcv = !args[MOST_COMMON_FREQS_ARG].isnull && + !args[MOST_COMMON_VALS_ARG].isnull; + bool do_histogram = !args[HISTOGRAM_BOUNDS_ARG].isnull; + bool do_correlation = !args[CORRELATION_ARG].isnull; + bool do_mcelem = !args[MOST_COMMON_ELEMS_ARG].isnull && + !args[MOST_COMMON_ELEM_FREQS_ARG].isnull; + bool do_dechist = !args[ELEM_COUNT_HISTOGRAM_ARG].isnull; + bool do_bounds_histogram = !args[RANGE_BOUNDS_HISTOGRAM_ARG].isnull; + bool do_range_length_histogram = !args[RANGE_LENGTH_HISTOGRAM_ARG].isnull && + !args[RANGE_EMPTY_FRAC_ARG].isnull; Datum values[Natts_pg_statistic] = {0}; bool nulls[Natts_pg_statistic] = {0}; @@ -253,31 +253,31 @@ attribute_statistics_update_internal(Oid reloid, * and skip the corresponding statistics kind, reporting back a failure. */ - if (!stats_check_arg_array(fcinfo, attarginfo, MOST_COMMON_FREQS_ARG)) + if (!stats_check_arg_array(args, attarginfo, MOST_COMMON_FREQS_ARG)) { do_mcv = false; result = false; } - if (!stats_check_arg_array(fcinfo, attarginfo, MOST_COMMON_ELEM_FREQS_ARG)) + if (!stats_check_arg_array(args, attarginfo, MOST_COMMON_ELEM_FREQS_ARG)) { do_mcelem = false; result = false; } - if (!stats_check_arg_array(fcinfo, attarginfo, ELEM_COUNT_HISTOGRAM_ARG)) + if (!stats_check_arg_array(args, attarginfo, ELEM_COUNT_HISTOGRAM_ARG)) { do_dechist = false; result = false; } - if (!stats_check_arg_pair(fcinfo, attarginfo, + if (!stats_check_arg_pair(args, attarginfo, MOST_COMMON_VALS_ARG, MOST_COMMON_FREQS_ARG)) { do_mcv = false; result = false; } - if (!stats_check_arg_pair(fcinfo, attarginfo, + if (!stats_check_arg_pair(args, attarginfo, MOST_COMMON_ELEMS_ARG, MOST_COMMON_ELEM_FREQS_ARG)) { @@ -285,7 +285,7 @@ attribute_statistics_update_internal(Oid reloid, result = false; } - if (!stats_check_arg_pair(fcinfo, attarginfo, + if (!stats_check_arg_pair(args, attarginfo, RANGE_LENGTH_HISTOGRAM_ARG, RANGE_EMPTY_FRAC_ARG)) { @@ -361,19 +361,19 @@ attribute_statistics_update_internal(Oid reloid, replaces); /* if specified, set to argument values */ - if (!PG_ARGISNULL(NULL_FRAC_ARG)) + if (!args[NULL_FRAC_ARG].isnull) { - values[Anum_pg_statistic_stanullfrac - 1] = PG_GETARG_DATUM(NULL_FRAC_ARG); + values[Anum_pg_statistic_stanullfrac - 1] = args[NULL_FRAC_ARG].value; replaces[Anum_pg_statistic_stanullfrac - 1] = true; } - if (!PG_ARGISNULL(AVG_WIDTH_ARG)) + if (!args[AVG_WIDTH_ARG].isnull) { - values[Anum_pg_statistic_stawidth - 1] = PG_GETARG_DATUM(AVG_WIDTH_ARG); + values[Anum_pg_statistic_stawidth - 1] = args[AVG_WIDTH_ARG].value; replaces[Anum_pg_statistic_stawidth - 1] = true; } - if (!PG_ARGISNULL(N_DISTINCT_ARG)) + if (!args[N_DISTINCT_ARG].isnull) { - values[Anum_pg_statistic_stadistinct - 1] = PG_GETARG_DATUM(N_DISTINCT_ARG); + values[Anum_pg_statistic_stadistinct - 1] = args[N_DISTINCT_ARG].value; replaces[Anum_pg_statistic_stadistinct - 1] = true; } @@ -381,10 +381,10 @@ attribute_statistics_update_internal(Oid reloid, if (do_mcv) { bool converted; - Datum stanumbers = PG_GETARG_DATUM(MOST_COMMON_FREQS_ARG); + Datum stanumbers = args[MOST_COMMON_FREQS_ARG].value; Datum stavalues = statatt_build_stavalues("most_common_vals", &array_in_fn, - PG_GETARG_DATUM(MOST_COMMON_VALS_ARG), + args[MOST_COMMON_VALS_ARG].value, atttypid, atttypmod, &converted); @@ -424,7 +424,7 @@ attribute_statistics_update_internal(Oid reloid, stavalues = statatt_build_stavalues("histogram_bounds", &array_in_fn, - PG_GETARG_DATUM(HISTOGRAM_BOUNDS_ARG), + args[HISTOGRAM_BOUNDS_ARG].value, atttypid, atttypmod, &converted); @@ -442,7 +442,7 @@ attribute_statistics_update_internal(Oid reloid, /* STATISTIC_KIND_CORRELATION */ if (do_correlation) { - Datum elems[] = {PG_GETARG_DATUM(CORRELATION_ARG)}; + Datum elems[] = {args[CORRELATION_ARG].value}; ArrayType *arry = construct_array_builtin(elems, 1, FLOAT4OID); Datum stanumbers = PointerGetDatum(arry); @@ -455,13 +455,13 @@ attribute_statistics_update_internal(Oid reloid, /* STATISTIC_KIND_MCELEM */ if (do_mcelem) { - Datum stanumbers = PG_GETARG_DATUM(MOST_COMMON_ELEM_FREQS_ARG); + Datum stanumbers = args[MOST_COMMON_ELEM_FREQS_ARG].value; bool converted = false; Datum stavalues; stavalues = statatt_build_stavalues("most_common_elems", &array_in_fn, - PG_GETARG_DATUM(MOST_COMMON_ELEMS_ARG), + args[MOST_COMMON_ELEMS_ARG].value, elemtypid, atttypmod, &converted); @@ -479,7 +479,7 @@ attribute_statistics_update_internal(Oid reloid, /* STATISTIC_KIND_DECHIST */ if (do_dechist) { - Datum stanumbers = PG_GETARG_DATUM(ELEM_COUNT_HISTOGRAM_ARG); + Datum stanumbers = args[ELEM_COUNT_HISTOGRAM_ARG].value; statatt_set_slot(values, nulls, replaces, STATISTIC_KIND_DECHIST, @@ -509,7 +509,7 @@ attribute_statistics_update_internal(Oid reloid, stavalues = statatt_build_stavalues("range_bounds_histogram", &array_in_fn, - PG_GETARG_DATUM(RANGE_BOUNDS_HISTOGRAM_ARG), + args[RANGE_BOUNDS_HISTOGRAM_ARG].value, bounds_typid, atttypmod, &converted); @@ -529,7 +529,7 @@ attribute_statistics_update_internal(Oid reloid, if (do_range_length_histogram) { /* The anyarray is always a float8[] for this stakind */ - Datum elems[] = {PG_GETARG_DATUM(RANGE_EMPTY_FRAC_ARG)}; + Datum elems[] = {args[RANGE_EMPTY_FRAC_ARG].value}; ArrayType *arry = construct_array_builtin(elems, 1, FLOAT4OID); Datum stanumbers = PointerGetDatum(arry); @@ -538,7 +538,7 @@ attribute_statistics_update_internal(Oid reloid, stavalues = statatt_build_stavalues("range_length_histogram", &array_in_fn, - PG_GETARG_DATUM(RANGE_LENGTH_HISTOGRAM_ARG), + args[RANGE_LENGTH_HISTOGRAM_ARG].value, FLOAT8OID, 0, &converted); if (converted) @@ -631,10 +631,10 @@ pg_clear_attribute_stats(PG_FUNCTION_ARGS) bool inherited; Oid locked_table = InvalidOid; - stats_check_required_arg(fcinfo, cleararginfo, C_ATTRELSCHEMA_ARG); - stats_check_required_arg(fcinfo, cleararginfo, C_ATTRELNAME_ARG); - stats_check_required_arg(fcinfo, cleararginfo, C_ATTNAME_ARG); - stats_check_required_arg(fcinfo, cleararginfo, C_INHERITED_ARG); + stats_check_required_arg(fcinfo->args, cleararginfo, C_ATTRELSCHEMA_ARG); + stats_check_required_arg(fcinfo->args, cleararginfo, C_ATTRELNAME_ARG); + stats_check_required_arg(fcinfo->args, cleararginfo, C_ATTNAME_ARG); + stats_check_required_arg(fcinfo->args, cleararginfo, C_INHERITED_ARG); nspname = TextDatumGetCString(PG_GETARG_DATUM(C_ATTRELSCHEMA_ARG)); relname = TextDatumGetCString(PG_GETARG_DATUM(C_ATTRELNAME_ARG)); @@ -699,17 +699,14 @@ pg_clear_attribute_stats(PG_FUNCTION_ARGS) Datum pg_restore_attribute_stats(PG_FUNCTION_ARGS) { - LOCAL_FCINFO(positional_fcinfo, NUM_ATTRIBUTE_STATS_ARGS); + NullableDatum positional_args[NUM_ATTRIBUTE_STATS_ARGS]; bool result = true; - InitFunctionCallInfoData(*positional_fcinfo, NULL, NUM_ATTRIBUTE_STATS_ARGS, - InvalidOid, NULL, NULL); - - if (!stats_fill_fcinfo_from_arg_pairs(fcinfo, positional_fcinfo, - attarginfo)) + if (!stats_fill_args_from_arg_pairs(fcinfo, positional_args, + attarginfo)) result = false; - if (!attribute_statistics_update(positional_fcinfo)) + if (!attribute_statistics_update(positional_args)) result = false; PG_RETURN_BOOL(result); @@ -739,10 +736,11 @@ import_attribute_statistics(Relation rel, AttrNumber attnum, bool inherited, const NullableDatum *range_empty_frac, const NullableDatum *range_bounds_histogram) { - LOCAL_FCINFO(newfcinfo, NUM_ATTRIBUTE_STATS_ARGS); + NullableDatum args[NUM_ATTRIBUTE_STATS_ARGS]; Oid reloid = RelationGetRelid(rel); char *relname = RelationGetRelationName(rel); char *attname = get_attname(reloid, attnum, true); + NullableDatum unused = {.isnull = true, .value = (Datum) 0}; Assert(null_frac); Assert(avg_width); @@ -766,37 +764,28 @@ import_attribute_statistics(Relation rel, AttrNumber attnum, bool inherited, errmsg("column %d of relation \"%s\" does not exist", attnum, relname))); - InitFunctionCallInfoData(*newfcinfo, NULL, NUM_ATTRIBUTE_STATS_ARGS, - InvalidOid, NULL, NULL); + args[ATTRELSCHEMA_ARG] = unused; + args[ATTRELNAME_ARG] = unused; + args[ATTNAME_ARG] = unused; + args[ATTNUM_ARG] = unused; + args[INHERITED_ARG] = unused; - newfcinfo->args[ATTRELSCHEMA_ARG].value = - CStringGetTextDatum(get_namespace_name(RelationGetNamespace(rel))); - newfcinfo->args[ATTRELSCHEMA_ARG].isnull = false; - newfcinfo->args[ATTRELNAME_ARG].value = CStringGetTextDatum(relname); - newfcinfo->args[ATTRELNAME_ARG].isnull = false; - newfcinfo->args[ATTNAME_ARG].value = CStringGetTextDatum(attname); - newfcinfo->args[ATTNAME_ARG].isnull = false; - newfcinfo->args[ATTNUM_ARG].value = Int16GetDatum(attnum); - newfcinfo->args[ATTNUM_ARG].isnull = false; - newfcinfo->args[INHERITED_ARG].value = BoolGetDatum(inherited); - newfcinfo->args[INHERITED_ARG].isnull = false; - - newfcinfo->args[NULL_FRAC_ARG] = *null_frac; - newfcinfo->args[AVG_WIDTH_ARG] = *avg_width; - newfcinfo->args[N_DISTINCT_ARG] = *n_distinct; - newfcinfo->args[MOST_COMMON_VALS_ARG] = *most_common_vals; - newfcinfo->args[MOST_COMMON_FREQS_ARG] = *most_common_freqs; - newfcinfo->args[HISTOGRAM_BOUNDS_ARG] = *histogram_bounds; - newfcinfo->args[CORRELATION_ARG] = *correlation; - newfcinfo->args[MOST_COMMON_ELEMS_ARG] = *most_common_elems; - newfcinfo->args[MOST_COMMON_ELEM_FREQS_ARG] = *most_common_elem_freqs; - newfcinfo->args[ELEM_COUNT_HISTOGRAM_ARG] = *elem_count_histogram; - newfcinfo->args[RANGE_LENGTH_HISTOGRAM_ARG] = *range_length_histogram; - newfcinfo->args[RANGE_EMPTY_FRAC_ARG] = *range_empty_frac; - newfcinfo->args[RANGE_BOUNDS_HISTOGRAM_ARG] = *range_bounds_histogram; + args[NULL_FRAC_ARG] = *null_frac; + args[AVG_WIDTH_ARG] = *avg_width; + args[N_DISTINCT_ARG] = *n_distinct; + args[MOST_COMMON_VALS_ARG] = *most_common_vals; + args[MOST_COMMON_FREQS_ARG] = *most_common_freqs; + args[HISTOGRAM_BOUNDS_ARG] = *histogram_bounds; + args[CORRELATION_ARG] = *correlation; + args[MOST_COMMON_ELEMS_ARG] = *most_common_elems; + args[MOST_COMMON_ELEM_FREQS_ARG] = *most_common_elem_freqs; + args[ELEM_COUNT_HISTOGRAM_ARG] = *elem_count_histogram; + args[RANGE_LENGTH_HISTOGRAM_ARG] = *range_length_histogram; + args[RANGE_EMPTY_FRAC_ARG] = *range_empty_frac; + args[RANGE_BOUNDS_HISTOGRAM_ARG] = *range_bounds_histogram; return attribute_statistics_update_internal(reloid, attname, attnum, - inherited, newfcinfo); + inherited, args); } /* diff --git a/src/backend/statistics/extended_stats_funcs.c b/src/backend/statistics/extended_stats_funcs.c index 988f81c6be42..96591e79c723 100644 --- a/src/backend/statistics/extended_stats_funcs.c +++ b/src/backend/statistics/extended_stats_funcs.c @@ -121,7 +121,7 @@ static const char *extexprargname[NUM_ATTRIBUTE_STATS_ELEMS] = "range_bounds_histogram" }; -static bool extended_statistics_update(FunctionCallInfo fcinfo); +static bool extended_statistics_update(const NullableDatum *args); static HeapTuple get_pg_statistic_ext(Relation pg_stext, Oid nspoid, const char *stxname); @@ -311,7 +311,7 @@ upsert_pg_statistic_ext_data(const Datum *values, const bool *nulls, * be updated. */ static bool -extended_statistics_update(FunctionCallInfo fcinfo) +extended_statistics_update(const NullableDatum *args) { char *relnspname; char *relname; @@ -356,12 +356,12 @@ extended_statistics_update(FunctionCallInfo fcinfo) * Therefore, none of the three array values is meaningful unless the * other two are also present and in sync in terms of array length. */ - has.mcv = (!PG_ARGISNULL(MOST_COMMON_VALS_ARG) && - !PG_ARGISNULL(MOST_COMMON_FREQS_ARG) && - !PG_ARGISNULL(MOST_COMMON_BASE_FREQS_ARG)); - has.ndistinct = !PG_ARGISNULL(NDISTINCT_ARG); - has.dependencies = !PG_ARGISNULL(DEPENDENCIES_ARG); - has.expressions = !PG_ARGISNULL(EXPRESSIONS_ARG); + has.mcv = (!args[MOST_COMMON_VALS_ARG].isnull && + !args[MOST_COMMON_FREQS_ARG].isnull && + !args[MOST_COMMON_BASE_FREQS_ARG].isnull); + has.ndistinct = !args[NDISTINCT_ARG].isnull; + has.dependencies = !args[DEPENDENCIES_ARG].isnull; + has.expressions = !args[EXPRESSIONS_ARG].isnull; if (RecoveryInProgress()) { @@ -373,18 +373,18 @@ extended_statistics_update(FunctionCallInfo fcinfo) } /* relation arguments */ - stats_check_required_arg(fcinfo, extarginfo, RELSCHEMA_ARG); - relnspname = TextDatumGetCString(PG_GETARG_DATUM(RELSCHEMA_ARG)); - stats_check_required_arg(fcinfo, extarginfo, RELNAME_ARG); - relname = TextDatumGetCString(PG_GETARG_DATUM(RELNAME_ARG)); + stats_check_required_arg(args, extarginfo, RELSCHEMA_ARG); + relnspname = TextDatumGetCString(args[RELSCHEMA_ARG].value); + stats_check_required_arg(args, extarginfo, RELNAME_ARG); + relname = TextDatumGetCString(args[RELNAME_ARG].value); /* extended statistics arguments */ - stats_check_required_arg(fcinfo, extarginfo, STATSCHEMA_ARG); - nspname = TextDatumGetCString(PG_GETARG_DATUM(STATSCHEMA_ARG)); - stats_check_required_arg(fcinfo, extarginfo, STATNAME_ARG); - stxname = TextDatumGetCString(PG_GETARG_DATUM(STATNAME_ARG)); - stats_check_required_arg(fcinfo, extarginfo, INHERITED_ARG); - inherited = PG_GETARG_BOOL(INHERITED_ARG); + stats_check_required_arg(args, extarginfo, STATSCHEMA_ARG); + nspname = TextDatumGetCString(args[STATSCHEMA_ARG].value); + stats_check_required_arg(args, extarginfo, STATNAME_ARG); + stxname = TextDatumGetCString(args[STATNAME_ARG].value); + stats_check_required_arg(args, extarginfo, INHERITED_ARG); + inherited = DatumGetBool(args[INHERITED_ARG].value); /* * First open the relation where we expect to find the statistics. This @@ -514,9 +514,9 @@ extended_statistics_update(FunctionCallInfo fcinfo) */ if (!enabled.mcv) { - if (!PG_ARGISNULL(MOST_COMMON_VALS_ARG) || - !PG_ARGISNULL(MOST_COMMON_FREQS_ARG) || - !PG_ARGISNULL(MOST_COMMON_BASE_FREQS_ARG)) + if (!args[MOST_COMMON_VALS_ARG].isnull || + !args[MOST_COMMON_FREQS_ARG].isnull || + !args[MOST_COMMON_BASE_FREQS_ARG].isnull) { ereport(WARNING, errcode(ERRCODE_INVALID_PARAMETER_VALUE), @@ -538,9 +538,9 @@ extended_statistics_update(FunctionCallInfo fcinfo) * statistics object expects something, something is wrong. This * issues a WARNING if a partial input has been provided. */ - if (!PG_ARGISNULL(MOST_COMMON_VALS_ARG) || - !PG_ARGISNULL(MOST_COMMON_FREQS_ARG) || - !PG_ARGISNULL(MOST_COMMON_BASE_FREQS_ARG)) + if (!args[MOST_COMMON_VALS_ARG].isnull || + !args[MOST_COMMON_FREQS_ARG].isnull || + !args[MOST_COMMON_BASE_FREQS_ARG].isnull) { ereport(WARNING, errcode(ERRCODE_INVALID_PARAMETER_VALUE), @@ -655,7 +655,7 @@ extended_statistics_update(FunctionCallInfo fcinfo) if (has.ndistinct) { - Datum ndistinct_datum = PG_GETARG_DATUM(NDISTINCT_ARG); + Datum ndistinct_datum = args[NDISTINCT_ARG].value; bytea *data = DatumGetByteaPP(ndistinct_datum); MVNDistinct *ndistinct = statext_ndistinct_deserialize(data); @@ -674,7 +674,7 @@ extended_statistics_update(FunctionCallInfo fcinfo) if (has.dependencies) { - Datum dependencies_datum = PG_GETARG_DATUM(DEPENDENCIES_ARG); + Datum dependencies_datum = args[DEPENDENCIES_ARG].value; bytea *data = DatumGetByteaPP(dependencies_datum); MVDependencies *dependencies = statext_dependencies_deserialize(data); @@ -696,9 +696,9 @@ extended_statistics_update(FunctionCallInfo fcinfo) Datum datum; bool val_ok = false; - datum = import_mcv(PG_GETARG_ARRAYTYPE_P(MOST_COMMON_VALS_ARG), - PG_GETARG_ARRAYTYPE_P(MOST_COMMON_FREQS_ARG), - PG_GETARG_ARRAYTYPE_P(MOST_COMMON_BASE_FREQS_ARG), + datum = import_mcv(DatumGetArrayTypeP(args[MOST_COMMON_VALS_ARG].value), + DatumGetArrayTypeP(args[MOST_COMMON_FREQS_ARG].value), + DatumGetArrayTypeP(args[MOST_COMMON_BASE_FREQS_ARG].value), atttypids, atttypmods, atttypcolls, numattrs, &val_ok); @@ -733,7 +733,7 @@ extended_statistics_update(FunctionCallInfo fcinfo) &atttypids[numattnums], &atttypmods[numattnums], &atttypcolls[numattnums], - PG_GETARG_JSONB_P(EXPRESSIONS_ARG), + DatumGetJsonbP(args[EXPRESSIONS_ARG].value), &ok); table_close(pgsd, RowExclusiveLock); @@ -1718,22 +1718,19 @@ delete_pg_statistic_ext_data(Oid stxoid, bool inherited) * Restore (insert or replace) statistics for the given statistics object. * * This function accepts variadic arguments in key-value pairs, which are - * given to stats_fill_fcinfo_from_arg_pairs to be mapped into positional + * given to stats_fill_args_from_arg_pairs to be mapped into positional * arguments. */ Datum pg_restore_extended_stats(PG_FUNCTION_ARGS) { - LOCAL_FCINFO(positional_fcinfo, NUM_EXTENDED_STATS_ARGS); + NullableDatum positional_args[NUM_EXTENDED_STATS_ARGS]; bool result = true; - InitFunctionCallInfoData(*positional_fcinfo, NULL, NUM_EXTENDED_STATS_ARGS, - InvalidOid, NULL, NULL); - - if (!stats_fill_fcinfo_from_arg_pairs(fcinfo, positional_fcinfo, extarginfo)) + if (!stats_fill_args_from_arg_pairs(fcinfo, positional_args, extarginfo)) result = false; - if (!extended_statistics_update(positional_fcinfo)) + if (!extended_statistics_update(positional_args)) result = false; PG_RETURN_BOOL(result); @@ -1758,17 +1755,17 @@ pg_clear_extended_stats(PG_FUNCTION_ARGS) Oid locked_table = InvalidOid; /* relation arguments */ - stats_check_required_arg(fcinfo, extarginfo, RELSCHEMA_ARG); + stats_check_required_arg(fcinfo->args, extarginfo, RELSCHEMA_ARG); relnspname = TextDatumGetCString(PG_GETARG_DATUM(RELSCHEMA_ARG)); - stats_check_required_arg(fcinfo, extarginfo, RELNAME_ARG); + stats_check_required_arg(fcinfo->args, extarginfo, RELNAME_ARG); relname = TextDatumGetCString(PG_GETARG_DATUM(RELNAME_ARG)); /* extended statistics arguments */ - stats_check_required_arg(fcinfo, extarginfo, STATSCHEMA_ARG); + stats_check_required_arg(fcinfo->args, extarginfo, STATSCHEMA_ARG); nspname = TextDatumGetCString(PG_GETARG_DATUM(STATSCHEMA_ARG)); - stats_check_required_arg(fcinfo, extarginfo, STATNAME_ARG); + stats_check_required_arg(fcinfo->args, extarginfo, STATNAME_ARG); stxname = TextDatumGetCString(PG_GETARG_DATUM(STATNAME_ARG)); - stats_check_required_arg(fcinfo, extarginfo, INHERITED_ARG); + stats_check_required_arg(fcinfo->args, extarginfo, INHERITED_ARG); inherited = PG_GETARG_BOOL(INHERITED_ARG); if (RecoveryInProgress()) diff --git a/src/backend/statistics/relation_stats.c b/src/backend/statistics/relation_stats.c index f2743c00c589..392aeed5d253 100644 --- a/src/backend/statistics/relation_stats.c +++ b/src/backend/statistics/relation_stats.c @@ -59,26 +59,26 @@ static struct StatsArgInfo relarginfo[] = [NUM_RELATION_STATS_ARGS] = {0} }; -static bool relation_statistics_update(FunctionCallInfo fcinfo); +static bool relation_statistics_update(const NullableDatum *args); static bool relation_statistics_update_internal(Oid reloid, - FunctionCallInfo fcinfo); + const NullableDatum *args); /* * Internal function for modifying statistics for a relation. */ static bool -relation_statistics_update(FunctionCallInfo fcinfo) +relation_statistics_update(const NullableDatum *args) { char *nspname; char *relname; Oid reloid; Oid locked_table = InvalidOid; - stats_check_required_arg(fcinfo, relarginfo, RELSCHEMA_ARG); - stats_check_required_arg(fcinfo, relarginfo, RELNAME_ARG); + stats_check_required_arg(args, relarginfo, RELSCHEMA_ARG); + stats_check_required_arg(args, relarginfo, RELNAME_ARG); - nspname = TextDatumGetCString(PG_GETARG_DATUM(RELSCHEMA_ARG)); - relname = TextDatumGetCString(PG_GETARG_DATUM(RELNAME_ARG)); + nspname = TextDatumGetCString(args[RELSCHEMA_ARG].value); + relname = TextDatumGetCString(args[RELNAME_ARG].value); if (RecoveryInProgress()) ereport(ERROR, @@ -90,14 +90,14 @@ relation_statistics_update(FunctionCallInfo fcinfo) ShareUpdateExclusiveLock, 0, RangeVarCallbackForStats, &locked_table); - return relation_statistics_update_internal(reloid, fcinfo); + return relation_statistics_update_internal(reloid, args); } /* * Workhorse function for relation_statistics_update. */ static bool -relation_statistics_update_internal(Oid reloid, FunctionCallInfo fcinfo) +relation_statistics_update_internal(Oid reloid, const NullableDatum *args) { int32 relpages = 0; bool update_relpages = false; @@ -116,15 +116,15 @@ relation_statistics_update_internal(Oid reloid, FunctionCallInfo fcinfo) int nreplaces = 0; bool result = true; - if (!PG_ARGISNULL(RELPAGES_ARG)) + if (!args[RELPAGES_ARG].isnull) { - relpages = PG_GETARG_INT32(RELPAGES_ARG); + relpages = DatumGetInt32(args[RELPAGES_ARG].value); update_relpages = true; } - if (!PG_ARGISNULL(RELTUPLES_ARG)) + if (!args[RELTUPLES_ARG].isnull) { - reltuples = PG_GETARG_FLOAT4(RELTUPLES_ARG); + reltuples = DatumGetFloat4(args[RELTUPLES_ARG].value); if (isnan(reltuples) || isinf(reltuples)) { ereport(WARNING, @@ -143,15 +143,15 @@ relation_statistics_update_internal(Oid reloid, FunctionCallInfo fcinfo) update_reltuples = true; } - if (!PG_ARGISNULL(RELALLVISIBLE_ARG)) + if (!args[RELALLVISIBLE_ARG].isnull) { - relallvisible = PG_GETARG_INT32(RELALLVISIBLE_ARG); + relallvisible = DatumGetInt32(args[RELALLVISIBLE_ARG].value); update_relallvisible = true; } - if (!PG_ARGISNULL(RELALLFROZEN_ARG)) + if (!args[RELALLFROZEN_ARG].isnull) { - relallfrozen = PG_GETARG_INT32(RELALLFROZEN_ARG); + relallfrozen = DatumGetInt32(args[RELALLFROZEN_ARG].value); update_relallfrozen = true; } @@ -223,42 +223,36 @@ relation_statistics_update_internal(Oid reloid, FunctionCallInfo fcinfo) Datum pg_clear_relation_stats(PG_FUNCTION_ARGS) { - LOCAL_FCINFO(newfcinfo, 6); + NullableDatum args[NUM_RELATION_STATS_ARGS]; - InitFunctionCallInfoData(*newfcinfo, NULL, 6, InvalidOid, NULL, NULL); + args[RELSCHEMA_ARG].value = PG_GETARG_DATUM(0); + args[RELSCHEMA_ARG].isnull = PG_ARGISNULL(0); + args[RELNAME_ARG].value = PG_GETARG_DATUM(1); + args[RELNAME_ARG].isnull = PG_ARGISNULL(1); + args[RELPAGES_ARG].value = Int32GetDatum(0); + args[RELPAGES_ARG].isnull = false; + args[RELTUPLES_ARG].value = Float4GetDatum(-1.0); + args[RELTUPLES_ARG].isnull = false; + args[RELALLVISIBLE_ARG].value = Int32GetDatum(0); + args[RELALLVISIBLE_ARG].isnull = false; + args[RELALLFROZEN_ARG].value = Int32GetDatum(0); + args[RELALLFROZEN_ARG].isnull = false; - newfcinfo->args[0].value = PG_GETARG_DATUM(0); - newfcinfo->args[0].isnull = PG_ARGISNULL(0); - newfcinfo->args[1].value = PG_GETARG_DATUM(1); - newfcinfo->args[1].isnull = PG_ARGISNULL(1); - newfcinfo->args[2].value = Int32GetDatum(0); - newfcinfo->args[2].isnull = false; - newfcinfo->args[3].value = Float4GetDatum(-1.0); - newfcinfo->args[3].isnull = false; - newfcinfo->args[4].value = Int32GetDatum(0); - newfcinfo->args[4].isnull = false; - newfcinfo->args[5].value = Int32GetDatum(0); - newfcinfo->args[5].isnull = false; - - relation_statistics_update(newfcinfo); + relation_statistics_update(args); PG_RETURN_VOID(); } Datum pg_restore_relation_stats(PG_FUNCTION_ARGS) { - LOCAL_FCINFO(positional_fcinfo, NUM_RELATION_STATS_ARGS); + NullableDatum positional_args[NUM_RELATION_STATS_ARGS]; bool result = true; - InitFunctionCallInfoData(*positional_fcinfo, NULL, - NUM_RELATION_STATS_ARGS, - InvalidOid, NULL, NULL); - - if (!stats_fill_fcinfo_from_arg_pairs(fcinfo, positional_fcinfo, - relarginfo)) + if (!stats_fill_args_from_arg_pairs(fcinfo, positional_args, + relarginfo)) result = false; - if (!relation_statistics_update(positional_fcinfo)) + if (!relation_statistics_update(positional_args)) result = false; PG_RETURN_BOOL(result); @@ -279,28 +273,21 @@ import_relation_statistics(Relation rel, const NullableDatum *relallvisible, const NullableDatum *relallfrozen) { - LOCAL_FCINFO(newfcinfo, NUM_RELATION_STATS_ARGS); + NullableDatum args[NUM_RELATION_STATS_ARGS]; + NullableDatum unused = {.isnull = true, .value = (Datum) 0}; Assert(relpages); Assert(reltuples); Assert(relallvisible); Assert(relallfrozen); - InitFunctionCallInfoData(*newfcinfo, NULL, NUM_RELATION_STATS_ARGS, - InvalidOid, NULL, NULL); + args[RELSCHEMA_ARG] = unused; + args[RELNAME_ARG] = unused; - newfcinfo->args[RELSCHEMA_ARG].value = - CStringGetTextDatum(get_namespace_name(RelationGetNamespace(rel))); - newfcinfo->args[RELSCHEMA_ARG].isnull = false; - newfcinfo->args[RELNAME_ARG].value = - CStringGetTextDatum(RelationGetRelationName(rel)); - newfcinfo->args[RELNAME_ARG].isnull = false; + args[RELPAGES_ARG] = *relpages; + args[RELTUPLES_ARG] = *reltuples; + args[RELALLVISIBLE_ARG] = *relallvisible; + args[RELALLFROZEN_ARG] = *relallfrozen; - newfcinfo->args[RELPAGES_ARG] = *relpages; - newfcinfo->args[RELTUPLES_ARG] = *reltuples; - newfcinfo->args[RELALLVISIBLE_ARG] = *relallvisible; - newfcinfo->args[RELALLFROZEN_ARG] = *relallfrozen; - - return relation_statistics_update_internal(RelationGetRelid(rel), - newfcinfo); + return relation_statistics_update_internal(RelationGetRelid(rel), args); } diff --git a/src/backend/statistics/stat_utils.c b/src/backend/statistics/stat_utils.c index f4ff9ab9b20c..e12678311f46 100644 --- a/src/backend/statistics/stat_utils.c +++ b/src/backend/statistics/stat_utils.c @@ -51,11 +51,11 @@ static Node *statatt_get_index_expr(Relation rel, int attnum); * Ensure that a given argument is not null. */ void -stats_check_required_arg(FunctionCallInfo fcinfo, +stats_check_required_arg(const NullableDatum *args, struct StatsArgInfo *arginfo, int argnum) { - if (PG_ARGISNULL(argnum)) + if (args[argnum].isnull) ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("argument \"%s\" must not be null", @@ -70,16 +70,16 @@ stats_check_required_arg(FunctionCallInfo fcinfo, * true. */ bool -stats_check_arg_array(FunctionCallInfo fcinfo, +stats_check_arg_array(const NullableDatum *args, struct StatsArgInfo *arginfo, int argnum) { ArrayType *arr; - if (PG_ARGISNULL(argnum)) + if (args[argnum].isnull) return true; - arr = DatumGetArrayTypeP(PG_GETARG_DATUM(argnum)); + arr = DatumGetArrayTypeP(args[argnum].value); if (ARR_NDIM(arr) != 1) { @@ -111,17 +111,17 @@ stats_check_arg_array(FunctionCallInfo fcinfo, * true. */ bool -stats_check_arg_pair(FunctionCallInfo fcinfo, +stats_check_arg_pair(const NullableDatum *args, struct StatsArgInfo *arginfo, int argnum1, int argnum2) { - if (PG_ARGISNULL(argnum1) && PG_ARGISNULL(argnum2)) + if (args[argnum1].isnull && args[argnum2].isnull) return true; - if (PG_ARGISNULL(argnum1) || PG_ARGISNULL(argnum2)) + if (args[argnum1].isnull || args[argnum2].isnull) { - int nullarg = PG_ARGISNULL(argnum1) ? argnum1 : argnum2; - int otherarg = PG_ARGISNULL(argnum1) ? argnum2 : argnum1; + int nullarg = args[argnum1].isnull ? argnum1 : argnum2; + int otherarg = args[argnum1].isnull ? argnum2 : argnum1; ereport(WARNING, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), @@ -340,17 +340,17 @@ statatt_get_index_expr(Relation rel, int attnum) /* * Translate variadic argument pairs from 'pairs_fcinfo' into a - * 'positional_fcinfo' appropriate for calling relation_statistics_update() or - * attribute_statistics_update() with positional arguments. + * NullableDatum[] appropriate for calling the internal statistics update + * functions for relation stats, attribute stats, or extended stats. * - * Caller should have already initialized positional_fcinfo with a size - * appropriate for calling the intended positional function, and arginfo - * should also match the intended positional function. + * Caller should have already initialized args with a size appropriate for + * calling the intended function, and arginfo should also match the intended + * function. */ bool -stats_fill_fcinfo_from_arg_pairs(FunctionCallInfo pairs_fcinfo, - FunctionCallInfo positional_fcinfo, - struct StatsArgInfo *arginfo) +stats_fill_args_from_arg_pairs(FunctionCallInfo pairs_fcinfo, + NullableDatum *positional_args, + struct StatsArgInfo *arginfo) { Datum *args; bool *argnulls; @@ -361,8 +361,8 @@ stats_fill_fcinfo_from_arg_pairs(FunctionCallInfo pairs_fcinfo, /* clear positional args */ for (int i = 0; arginfo[i].argname != NULL; i++) { - positional_fcinfo->args[i].value = (Datum) 0; - positional_fcinfo->args[i].isnull = true; + positional_args[i].value = (Datum) 0; + positional_args[i].isnull = true; } nargs = extract_variadic_args(pairs_fcinfo, 0, true, @@ -420,8 +420,8 @@ stats_fill_fcinfo_from_arg_pairs(FunctionCallInfo pairs_fcinfo, continue; } - positional_fcinfo->args[argnum].value = args[i + 1]; - positional_fcinfo->args[argnum].isnull = false; + positional_args[argnum].value = args[i + 1]; + positional_args[argnum].isnull = false; } return result; -- 2.55.0
From a1433a72b43430b86b9209c134dcfb9cf0d6ec9b Mon Sep 17 00:00:00 2001 From: Michael Paquier <[email protected]> Date: Tue, 1 Sep 2026 17:07:05 +0900 Subject: [PATCH v2 2/2] Use named structs for the statistics values to import This simplifies the interface of postgres_fdw for the import of the stats, removing the need of a round-trip in terms of a NullableDatum array built on the FDW side before rebuilding it in the stats update path. The stats import functions also become deeply simplified, passing down one pointer to a structure with all the values assigned. Author: Michael Paquier <[email protected]> --- src/include/statistics/stat_utils.h | 11 +- src/include/statistics/statistics.h | 66 ++++++--- src/backend/statistics/attribute_stats.c | 162 ++++++++++------------- src/backend/statistics/relation_stats.c | 64 ++++----- src/backend/statistics/stat_utils.c | 28 ++-- contrib/postgres_fdw/postgres_fdw.c | 69 +++++----- src/tools/pgindent/typedefs.list | 2 + 7 files changed, 197 insertions(+), 205 deletions(-) diff --git a/src/include/statistics/stat_utils.h b/src/include/statistics/stat_utils.h index 0560826fcfe5..8544c064bbb1 100644 --- a/src/include/statistics/stat_utils.h +++ b/src/include/statistics/stat_utils.h @@ -28,11 +28,12 @@ struct StatsArgInfo extern void stats_check_required_arg(const NullableDatum *args, struct StatsArgInfo *arginfo, int argnum); -extern bool stats_check_arg_array(const NullableDatum *args, - struct StatsArgInfo *arginfo, int argnum); -extern bool stats_check_arg_pair(const NullableDatum *args, - struct StatsArgInfo *arginfo, - int argnum1, int argnum2); +extern bool stats_check_arg_array(const NullableDatum *arg, + const char *argname); +extern bool stats_check_arg_pair(const NullableDatum *arg1, + const NullableDatum *arg2, + const char *argname1, + const char *argname2); extern void RangeVarCallbackForStats(const RangeVar *relation, Oid relId, Oid oldRelId, void *arg); diff --git a/src/include/statistics/statistics.h b/src/include/statistics/statistics.h index 0b163103a729..fa0430b94774 100644 --- a/src/include/statistics/statistics.h +++ b/src/include/statistics/statistics.h @@ -128,28 +128,56 @@ extern StatisticExtInfo *choose_best_statistics(List *stats, char requiredkind, int nclauses); extern HeapTuple statext_expressions_load(Oid stxoid, bool inh, int idx); +/* + * Statistics values applied to pg_class during stats import or restore + * + * A field with isnull set to true leaves the corresponding pg_class column + * untouched. The caller must initialize every field. + * + * The "version" field is currently ignored. In the future it can be used to + * interpret the format of older statistics. + */ +typedef struct RelationStatsValues +{ + NullableDatum version; + NullableDatum relpages; + NullableDatum reltuples; + NullableDatum relallvisible; + NullableDatum relallfrozen; +} RelationStatsValues; + +/* + * Statistics values applied to pg_statistic during stats import or restore. + * + * A field with isnull set to true leaves the corresponding statistics kind + * unset. The caller must initialize every field. + * + * The "version" field is currently ignored. In the future, it can be used to + * interpret the format of older statistics. + */ +typedef struct AttributeStatsValues +{ + NullableDatum version; + NullableDatum null_frac; + NullableDatum avg_width; + NullableDatum n_distinct; + NullableDatum most_common_vals; + NullableDatum most_common_freqs; + NullableDatum histogram_bounds; + NullableDatum correlation; + NullableDatum most_common_elems; + NullableDatum most_common_elem_freqs; + NullableDatum elem_count_histogram; + NullableDatum range_length_histogram; + NullableDatum range_empty_frac; + NullableDatum range_bounds_histogram; +} AttributeStatsValues; + extern bool import_relation_statistics(Relation rel, - const NullableDatum *version, - const NullableDatum *relpages, - const NullableDatum *reltuples, - const NullableDatum *relallvisible, - const NullableDatum *relallfrozen); + const RelationStatsValues *statvalues); extern bool import_attribute_statistics(Relation rel, AttrNumber attnum, bool inherited, - const NullableDatum *version, - const NullableDatum *null_frac, - const NullableDatum *avg_width, - const NullableDatum *n_distinct, - const NullableDatum *most_common_vals, - const NullableDatum *most_common_freqs, - const NullableDatum *histogram_bounds, - const NullableDatum *correlation, - const NullableDatum *most_common_elems, - const NullableDatum *most_common_elem_freqs, - const NullableDatum *elem_count_histogram, - const NullableDatum *range_length_histogram, - const NullableDatum *range_empty_frac, - const NullableDatum *range_bounds_histogram); + const AttributeStatsValues *statvalues); extern bool delete_attribute_statistics(Relation rel, AttrNumber attnum, bool inherited); diff --git a/src/backend/statistics/attribute_stats.c b/src/backend/statistics/attribute_stats.c index 5bb33e4283ae..1d19827dc45e 100644 --- a/src/backend/statistics/attribute_stats.c +++ b/src/backend/statistics/attribute_stats.c @@ -109,7 +109,7 @@ static bool attribute_statistics_update_internal(Oid reloid, const char *attname, AttrNumber attnum, bool inherited, - const NullableDatum *args); + const AttributeStatsValues *statvalues); static void upsert_pg_statistic(Relation starel, HeapTuple oldtup, const Datum *values, const bool *nulls, const bool *replaces); static bool delete_pg_statistic(Oid reloid, AttrNumber attnum, bool stainherit); @@ -140,6 +140,7 @@ attribute_statistics_update(const NullableDatum *args) AttrNumber attnum; bool inherited; Oid locked_table = InvalidOid; + AttributeStatsValues values; stats_check_required_arg(args, attarginfo, ATTRELSCHEMA_ARG); stats_check_required_arg(args, attarginfo, ATTRELNAME_ARG); @@ -204,8 +205,25 @@ attribute_statistics_update(const NullableDatum *args) stats_check_required_arg(args, attarginfo, INHERITED_ARG); inherited = DatumGetBool(args[INHERITED_ARG].value); + /* Collect the values to apply */ + values.version.value = (Datum) 0; + values.version.isnull = true; + values.null_frac = args[NULL_FRAC_ARG]; + values.avg_width = args[AVG_WIDTH_ARG]; + values.n_distinct = args[N_DISTINCT_ARG]; + values.most_common_vals = args[MOST_COMMON_VALS_ARG]; + values.most_common_freqs = args[MOST_COMMON_FREQS_ARG]; + values.histogram_bounds = args[HISTOGRAM_BOUNDS_ARG]; + values.correlation = args[CORRELATION_ARG]; + values.most_common_elems = args[MOST_COMMON_ELEMS_ARG]; + values.most_common_elem_freqs = args[MOST_COMMON_ELEM_FREQS_ARG]; + values.elem_count_histogram = args[ELEM_COUNT_HISTOGRAM_ARG]; + values.range_length_histogram = args[RANGE_LENGTH_HISTOGRAM_ARG]; + values.range_empty_frac = args[RANGE_EMPTY_FRAC_ARG]; + values.range_bounds_histogram = args[RANGE_BOUNDS_HISTOGRAM_ARG]; + return attribute_statistics_update_internal(reloid, attname, attnum, - inherited, args); + inherited, &values); } /* @@ -214,7 +232,8 @@ attribute_statistics_update(const NullableDatum *args) static bool attribute_statistics_update_internal(Oid reloid, const char *attname, AttrNumber attnum, - bool inherited, const NullableDatum *args) + bool inherited, + const AttributeStatsValues *statvalues) { Relation starel; HeapTuple statup; @@ -231,16 +250,16 @@ attribute_statistics_update_internal(Oid reloid, FmgrInfo array_in_fn; - bool do_mcv = !args[MOST_COMMON_FREQS_ARG].isnull && - !args[MOST_COMMON_VALS_ARG].isnull; - bool do_histogram = !args[HISTOGRAM_BOUNDS_ARG].isnull; - bool do_correlation = !args[CORRELATION_ARG].isnull; - bool do_mcelem = !args[MOST_COMMON_ELEMS_ARG].isnull && - !args[MOST_COMMON_ELEM_FREQS_ARG].isnull; - bool do_dechist = !args[ELEM_COUNT_HISTOGRAM_ARG].isnull; - bool do_bounds_histogram = !args[RANGE_BOUNDS_HISTOGRAM_ARG].isnull; - bool do_range_length_histogram = !args[RANGE_LENGTH_HISTOGRAM_ARG].isnull && - !args[RANGE_EMPTY_FRAC_ARG].isnull; + bool do_mcv = !statvalues->most_common_freqs.isnull && + !statvalues->most_common_vals.isnull; + bool do_histogram = !statvalues->histogram_bounds.isnull; + bool do_correlation = !statvalues->correlation.isnull; + bool do_mcelem = !statvalues->most_common_elems.isnull && + !statvalues->most_common_elem_freqs.isnull; + bool do_dechist = !statvalues->elem_count_histogram.isnull; + bool do_bounds_histogram = !statvalues->range_bounds_histogram.isnull; + bool do_range_length_histogram = !statvalues->range_length_histogram.isnull && + !statvalues->range_empty_frac.isnull; Datum values[Natts_pg_statistic] = {0}; bool nulls[Natts_pg_statistic] = {0}; @@ -253,41 +272,45 @@ attribute_statistics_update_internal(Oid reloid, * and skip the corresponding statistics kind, reporting back a failure. */ - if (!stats_check_arg_array(args, attarginfo, MOST_COMMON_FREQS_ARG)) + if (!stats_check_arg_array(&statvalues->most_common_freqs, + "most_common_freqs")) { do_mcv = false; result = false; } - if (!stats_check_arg_array(args, attarginfo, MOST_COMMON_ELEM_FREQS_ARG)) + if (!stats_check_arg_array(&statvalues->most_common_elem_freqs, + "most_common_elem_freqs")) { do_mcelem = false; result = false; } - if (!stats_check_arg_array(args, attarginfo, ELEM_COUNT_HISTOGRAM_ARG)) + if (!stats_check_arg_array(&statvalues->elem_count_histogram, + "elem_count_histogram")) { do_dechist = false; result = false; } - if (!stats_check_arg_pair(args, attarginfo, - MOST_COMMON_VALS_ARG, MOST_COMMON_FREQS_ARG)) + if (!stats_check_arg_pair(&statvalues->most_common_vals, + &statvalues->most_common_freqs, + "most_common_vals", "most_common_freqs")) { do_mcv = false; result = false; } - if (!stats_check_arg_pair(args, attarginfo, - MOST_COMMON_ELEMS_ARG, - MOST_COMMON_ELEM_FREQS_ARG)) + if (!stats_check_arg_pair(&statvalues->most_common_elems, + &statvalues->most_common_elem_freqs, + "most_common_elems", "most_common_elem_freqs")) { do_mcelem = false; result = false; } - if (!stats_check_arg_pair(args, attarginfo, - RANGE_LENGTH_HISTOGRAM_ARG, - RANGE_EMPTY_FRAC_ARG)) + if (!stats_check_arg_pair(&statvalues->range_length_histogram, + &statvalues->range_empty_frac, + "range_length_histogram", "range_empty_frac")) { do_range_length_histogram = false; result = false; @@ -361,19 +384,19 @@ attribute_statistics_update_internal(Oid reloid, replaces); /* if specified, set to argument values */ - if (!args[NULL_FRAC_ARG].isnull) + if (!statvalues->null_frac.isnull) { - values[Anum_pg_statistic_stanullfrac - 1] = args[NULL_FRAC_ARG].value; + values[Anum_pg_statistic_stanullfrac - 1] = statvalues->null_frac.value; replaces[Anum_pg_statistic_stanullfrac - 1] = true; } - if (!args[AVG_WIDTH_ARG].isnull) + if (!statvalues->avg_width.isnull) { - values[Anum_pg_statistic_stawidth - 1] = args[AVG_WIDTH_ARG].value; + values[Anum_pg_statistic_stawidth - 1] = statvalues->avg_width.value; replaces[Anum_pg_statistic_stawidth - 1] = true; } - if (!args[N_DISTINCT_ARG].isnull) + if (!statvalues->n_distinct.isnull) { - values[Anum_pg_statistic_stadistinct - 1] = args[N_DISTINCT_ARG].value; + values[Anum_pg_statistic_stadistinct - 1] = statvalues->n_distinct.value; replaces[Anum_pg_statistic_stadistinct - 1] = true; } @@ -381,10 +404,10 @@ attribute_statistics_update_internal(Oid reloid, if (do_mcv) { bool converted; - Datum stanumbers = args[MOST_COMMON_FREQS_ARG].value; + Datum stanumbers = statvalues->most_common_freqs.value; Datum stavalues = statatt_build_stavalues("most_common_vals", &array_in_fn, - args[MOST_COMMON_VALS_ARG].value, + statvalues->most_common_vals.value, atttypid, atttypmod, &converted); @@ -424,7 +447,7 @@ attribute_statistics_update_internal(Oid reloid, stavalues = statatt_build_stavalues("histogram_bounds", &array_in_fn, - args[HISTOGRAM_BOUNDS_ARG].value, + statvalues->histogram_bounds.value, atttypid, atttypmod, &converted); @@ -442,7 +465,7 @@ attribute_statistics_update_internal(Oid reloid, /* STATISTIC_KIND_CORRELATION */ if (do_correlation) { - Datum elems[] = {args[CORRELATION_ARG].value}; + Datum elems[] = {statvalues->correlation.value}; ArrayType *arry = construct_array_builtin(elems, 1, FLOAT4OID); Datum stanumbers = PointerGetDatum(arry); @@ -455,13 +478,13 @@ attribute_statistics_update_internal(Oid reloid, /* STATISTIC_KIND_MCELEM */ if (do_mcelem) { - Datum stanumbers = args[MOST_COMMON_ELEM_FREQS_ARG].value; + Datum stanumbers = statvalues->most_common_elem_freqs.value; bool converted = false; Datum stavalues; stavalues = statatt_build_stavalues("most_common_elems", &array_in_fn, - args[MOST_COMMON_ELEMS_ARG].value, + statvalues->most_common_elems.value, elemtypid, atttypmod, &converted); @@ -479,7 +502,7 @@ attribute_statistics_update_internal(Oid reloid, /* STATISTIC_KIND_DECHIST */ if (do_dechist) { - Datum stanumbers = args[ELEM_COUNT_HISTOGRAM_ARG].value; + Datum stanumbers = statvalues->elem_count_histogram.value; statatt_set_slot(values, nulls, replaces, STATISTIC_KIND_DECHIST, @@ -509,7 +532,7 @@ attribute_statistics_update_internal(Oid reloid, stavalues = statatt_build_stavalues("range_bounds_histogram", &array_in_fn, - args[RANGE_BOUNDS_HISTOGRAM_ARG].value, + statvalues->range_bounds_histogram.value, bounds_typid, atttypmod, &converted); @@ -529,7 +552,7 @@ attribute_statistics_update_internal(Oid reloid, if (do_range_length_histogram) { /* The anyarray is always a float8[] for this stakind */ - Datum elems[] = {args[RANGE_EMPTY_FRAC_ARG].value}; + Datum elems[] = {statvalues->range_empty_frac.value}; ArrayType *arry = construct_array_builtin(elems, 1, FLOAT4OID); Datum stanumbers = PointerGetDatum(arry); @@ -538,7 +561,7 @@ attribute_statistics_update_internal(Oid reloid, stavalues = statatt_build_stavalues("range_length_histogram", &array_in_fn, - args[RANGE_LENGTH_HISTOGRAM_ARG].value, + statvalues->range_length_histogram.value, FLOAT8OID, 0, &converted); if (converted) @@ -713,48 +736,19 @@ pg_restore_attribute_stats(PG_FUNCTION_ARGS) } /* - * Import attribute statistics from NullableDatum inputs for all statistical - * values. + * Import attribute statistics for a relation. * - * For now, the 'version' argument is ignored. In the future it can be used - * to interpret older statistics properly. + * See AttributeStatsValues for the values to provide. */ bool import_attribute_statistics(Relation rel, AttrNumber attnum, bool inherited, - const NullableDatum *version, - const NullableDatum *null_frac, - const NullableDatum *avg_width, - const NullableDatum *n_distinct, - const NullableDatum *most_common_vals, - const NullableDatum *most_common_freqs, - const NullableDatum *histogram_bounds, - const NullableDatum *correlation, - const NullableDatum *most_common_elems, - const NullableDatum *most_common_elem_freqs, - const NullableDatum *elem_count_histogram, - const NullableDatum *range_length_histogram, - const NullableDatum *range_empty_frac, - const NullableDatum *range_bounds_histogram) + const AttributeStatsValues *statvalues) { - NullableDatum args[NUM_ATTRIBUTE_STATS_ARGS]; Oid reloid = RelationGetRelid(rel); char *relname = RelationGetRelationName(rel); char *attname = get_attname(reloid, attnum, true); - NullableDatum unused = {.isnull = true, .value = (Datum) 0}; - Assert(null_frac); - Assert(avg_width); - Assert(n_distinct); - Assert(most_common_vals); - Assert(most_common_freqs); - Assert(histogram_bounds); - Assert(correlation); - Assert(most_common_elems); - Assert(most_common_elem_freqs); - Assert(elem_count_histogram); - Assert(range_length_histogram); - Assert(range_empty_frac); - Assert(range_bounds_histogram); + Assert(statvalues); /* annoyingly, get_attname doesn't check attisdropped */ if (attname == NULL || @@ -764,28 +758,8 @@ import_attribute_statistics(Relation rel, AttrNumber attnum, bool inherited, errmsg("column %d of relation \"%s\" does not exist", attnum, relname))); - args[ATTRELSCHEMA_ARG] = unused; - args[ATTRELNAME_ARG] = unused; - args[ATTNAME_ARG] = unused; - args[ATTNUM_ARG] = unused; - args[INHERITED_ARG] = unused; - - args[NULL_FRAC_ARG] = *null_frac; - args[AVG_WIDTH_ARG] = *avg_width; - args[N_DISTINCT_ARG] = *n_distinct; - args[MOST_COMMON_VALS_ARG] = *most_common_vals; - args[MOST_COMMON_FREQS_ARG] = *most_common_freqs; - args[HISTOGRAM_BOUNDS_ARG] = *histogram_bounds; - args[CORRELATION_ARG] = *correlation; - args[MOST_COMMON_ELEMS_ARG] = *most_common_elems; - args[MOST_COMMON_ELEM_FREQS_ARG] = *most_common_elem_freqs; - args[ELEM_COUNT_HISTOGRAM_ARG] = *elem_count_histogram; - args[RANGE_LENGTH_HISTOGRAM_ARG] = *range_length_histogram; - args[RANGE_EMPTY_FRAC_ARG] = *range_empty_frac; - args[RANGE_BOUNDS_HISTOGRAM_ARG] = *range_bounds_histogram; - return attribute_statistics_update_internal(reloid, attname, attnum, - inherited, args); + inherited, statvalues); } /* diff --git a/src/backend/statistics/relation_stats.c b/src/backend/statistics/relation_stats.c index 392aeed5d253..28d87fa6f77d 100644 --- a/src/backend/statistics/relation_stats.c +++ b/src/backend/statistics/relation_stats.c @@ -61,7 +61,7 @@ static struct StatsArgInfo relarginfo[] = static bool relation_statistics_update(const NullableDatum *args); static bool relation_statistics_update_internal(Oid reloid, - const NullableDatum *args); + const RelationStatsValues *statvalues); /* * Internal function for modifying statistics for a relation. @@ -73,6 +73,7 @@ relation_statistics_update(const NullableDatum *args) char *relname; Oid reloid; Oid locked_table = InvalidOid; + RelationStatsValues values; stats_check_required_arg(args, relarginfo, RELSCHEMA_ARG); stats_check_required_arg(args, relarginfo, RELNAME_ARG); @@ -90,14 +91,23 @@ relation_statistics_update(const NullableDatum *args) ShareUpdateExclusiveLock, 0, RangeVarCallbackForStats, &locked_table); - return relation_statistics_update_internal(reloid, args); + /* Collect the values to apply. */ + values.version.value = (Datum) 0; + values.version.isnull = true; + values.relpages = args[RELPAGES_ARG]; + values.reltuples = args[RELTUPLES_ARG]; + values.relallvisible = args[RELALLVISIBLE_ARG]; + values.relallfrozen = args[RELALLFROZEN_ARG]; + + return relation_statistics_update_internal(reloid, &values); } /* * Workhorse function for relation_statistics_update. */ static bool -relation_statistics_update_internal(Oid reloid, const NullableDatum *args) +relation_statistics_update_internal(Oid reloid, + const RelationStatsValues *statvalues) { int32 relpages = 0; bool update_relpages = false; @@ -116,15 +126,15 @@ relation_statistics_update_internal(Oid reloid, const NullableDatum *args) int nreplaces = 0; bool result = true; - if (!args[RELPAGES_ARG].isnull) + if (!statvalues->relpages.isnull) { - relpages = DatumGetInt32(args[RELPAGES_ARG].value); + relpages = DatumGetInt32(statvalues->relpages.value); update_relpages = true; } - if (!args[RELTUPLES_ARG].isnull) + if (!statvalues->reltuples.isnull) { - reltuples = DatumGetFloat4(args[RELTUPLES_ARG].value); + reltuples = DatumGetFloat4(statvalues->reltuples.value); if (isnan(reltuples) || isinf(reltuples)) { ereport(WARNING, @@ -143,15 +153,15 @@ relation_statistics_update_internal(Oid reloid, const NullableDatum *args) update_reltuples = true; } - if (!args[RELALLVISIBLE_ARG].isnull) + if (!statvalues->relallvisible.isnull) { - relallvisible = DatumGetInt32(args[RELALLVISIBLE_ARG].value); + relallvisible = DatumGetInt32(statvalues->relallvisible.value); update_relallvisible = true; } - if (!args[RELALLFROZEN_ARG].isnull) + if (!statvalues->relallfrozen.isnull) { - relallfrozen = DatumGetInt32(args[RELALLFROZEN_ARG].value); + relallfrozen = DatumGetInt32(statvalues->relallfrozen.value); update_relallfrozen = true; } @@ -259,35 +269,15 @@ pg_restore_relation_stats(PG_FUNCTION_ARGS) } /* - * Import relation statistics from NullableDatum inputs for all statistical - * values. + * Import relation statistics. * - * For now, the 'version' argument is ignored. In the future it can be used - * to interpret older statistics properly. + * See RelationStatsValues for the values to provide. */ bool -import_relation_statistics(Relation rel, - const NullableDatum *version, - const NullableDatum *relpages, - const NullableDatum *reltuples, - const NullableDatum *relallvisible, - const NullableDatum *relallfrozen) +import_relation_statistics(Relation rel, const RelationStatsValues *statvalues) { - NullableDatum args[NUM_RELATION_STATS_ARGS]; - NullableDatum unused = {.isnull = true, .value = (Datum) 0}; + Assert(statvalues); - Assert(relpages); - Assert(reltuples); - Assert(relallvisible); - Assert(relallfrozen); - - args[RELSCHEMA_ARG] = unused; - args[RELNAME_ARG] = unused; - - args[RELPAGES_ARG] = *relpages; - args[RELTUPLES_ARG] = *reltuples; - args[RELALLVISIBLE_ARG] = *relallvisible; - args[RELALLFROZEN_ARG] = *relallfrozen; - - return relation_statistics_update_internal(RelationGetRelid(rel), args); + return relation_statistics_update_internal(RelationGetRelid(rel), + statvalues); } diff --git a/src/backend/statistics/stat_utils.c b/src/backend/statistics/stat_utils.c index e12678311f46..1cd00959b5ca 100644 --- a/src/backend/statistics/stat_utils.c +++ b/src/backend/statistics/stat_utils.c @@ -70,23 +70,21 @@ stats_check_required_arg(const NullableDatum *args, * true. */ bool -stats_check_arg_array(const NullableDatum *args, - struct StatsArgInfo *arginfo, - int argnum) +stats_check_arg_array(const NullableDatum *arg, const char *argname) { ArrayType *arr; - if (args[argnum].isnull) + if (arg->isnull) return true; - arr = DatumGetArrayTypeP(args[argnum].value); + arr = DatumGetArrayTypeP(arg->value); if (ARR_NDIM(arr) != 1) { ereport(WARNING, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("argument \"%s\" must not be a multidimensional array", - arginfo[argnum].argname))); + argname))); return false; } @@ -95,7 +93,7 @@ stats_check_arg_array(const NullableDatum *args, ereport(WARNING, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("argument \"%s\" array must not contain null values", - arginfo[argnum].argname))); + argname))); return false; } @@ -111,23 +109,21 @@ stats_check_arg_array(const NullableDatum *args, * true. */ bool -stats_check_arg_pair(const NullableDatum *args, - struct StatsArgInfo *arginfo, - int argnum1, int argnum2) +stats_check_arg_pair(const NullableDatum *arg1, const NullableDatum *arg2, + const char *argname1, const char *argname2) { - if (args[argnum1].isnull && args[argnum2].isnull) + if (arg1->isnull && arg2->isnull) return true; - if (args[argnum1].isnull || args[argnum2].isnull) + if (arg1->isnull || arg2->isnull) { - int nullarg = args[argnum1].isnull ? argnum1 : argnum2; - int otherarg = args[argnum1].isnull ? argnum2 : argnum1; + const char *nullarg = arg1->isnull ? argname1 : argname2; + const char *otherarg = arg1->isnull ? argname2 : argname1; ereport(WARNING, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("argument \"%s\" must be specified when argument \"%s\" is specified", - arginfo[nullarg].argname, - arginfo[otherarg].argname))); + nullarg, otherarg))); return false; } diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c index 9269418a074c..2cc594aecb69 100644 --- a/contrib/postgres_fdw/postgres_fdw.c +++ b/contrib/postgres_fdw/postgres_fdw.c @@ -6253,11 +6253,12 @@ import_fetched_statistics(Relation relation, int attrcnt) { PGresult *res; - NullableDatum args[ATTSTATS_NUM_FIELDS]; + NullableDatum version; + RelationStatsValues relvalues; - /* Set the 'version' parameter, which is common to both statistics. */ - args[0].value = Int32GetDatum(remstats->version); - args[0].isnull = false; + /* Set the 'version' value, which is common to both statistics. */ + version.value = Int32GetDatum(remstats->version); + version.isnull = false; /* * We import attribute statistics first, if any, because those are more @@ -6274,6 +6275,7 @@ import_fetched_statistics(Relation relation, { int row = remattrmap[mapidx].res_index; AttrNumber attnum = remattrmap[mapidx].local_attnum; + AttributeStatsValues attvalues; /* All mappings should have been assigned a result set row. */ Assert(row >= 0); @@ -6284,41 +6286,38 @@ import_fetched_statistics(Relation relation, /* Clear existing attribute statistics. */ delete_attribute_statistics(relation, attnum, false); - /* Set the remaining parameters. */ - set_float_arg(&args[1], + /* Set the remaining values. */ + attvalues.version = version; + set_float_arg(&attvalues.null_frac, get_opt_value(res, row, ATTSTATS_NULL_FRAC)); - set_int32_arg(&args[2], + set_int32_arg(&attvalues.avg_width, get_opt_value(res, row, ATTSTATS_AVG_WIDTH)); - set_float_arg(&args[3], + set_float_arg(&attvalues.n_distinct, get_opt_value(res, row, ATTSTATS_N_DISTINCT)); - set_text_arg(&args[4], + set_text_arg(&attvalues.most_common_vals, get_opt_value(res, row, ATTSTATS_MOST_COMMON_VALS)); - set_floatarr_arg(&args[5], + set_floatarr_arg(&attvalues.most_common_freqs, get_opt_value(res, row, ATTSTATS_MOST_COMMON_FREQS)); - set_text_arg(&args[6], + set_text_arg(&attvalues.histogram_bounds, get_opt_value(res, row, ATTSTATS_HISTOGRAM_BOUNDS)); - set_float_arg(&args[7], + set_float_arg(&attvalues.correlation, get_opt_value(res, row, ATTSTATS_CORRELATION)); - set_text_arg(&args[8], + set_text_arg(&attvalues.most_common_elems, get_opt_value(res, row, ATTSTATS_MOST_COMMON_ELEMS)); - set_floatarr_arg(&args[9], + set_floatarr_arg(&attvalues.most_common_elem_freqs, get_opt_value(res, row, ATTSTATS_MOST_COMMON_ELEM_FREQS)); - set_floatarr_arg(&args[10], + set_floatarr_arg(&attvalues.elem_count_histogram, get_opt_value(res, row, ATTSTATS_ELEM_COUNT_HISTOGRAM)); - set_text_arg(&args[11], + set_text_arg(&attvalues.range_length_histogram, get_opt_value(res, row, ATTSTATS_RANGE_LENGTH_HISTOGRAM)); - set_float_arg(&args[12], + set_float_arg(&attvalues.range_empty_frac, get_opt_value(res, row, ATTSTATS_RANGE_EMPTY_FRAC)); - set_text_arg(&args[13], + set_text_arg(&attvalues.range_bounds_histogram, get_opt_value(res, row, ATTSTATS_RANGE_BOUNDS_HISTOGRAM)); /* Try to import the statistics. */ if (!import_attribute_statistics(relation, attnum, false, - &args[0], &args[1], &args[2], - &args[3], &args[4], &args[5], - &args[6], &args[7], &args[8], - &args[9], &args[10], &args[11], - &args[12], &args[13])) + &attvalues)) { ereport(WARNING, errmsg("could not import statistics for foreign table \"%s.%s\" --- attribute statistics import failed for column \"%s\" of this foreign table", @@ -6337,20 +6336,22 @@ import_fetched_statistics(Relation relation, Assert(PQnfields(res) == RELSTATS_NUM_FIELDS); Assert(PQntuples(res) == 1); - /* Set the remaining parameters. */ - set_int32_arg(&args[1], get_opt_value(res, 0, RELSTATS_RELPAGES)); - Assert(!args[1].isnull); - set_float_arg(&args[2], get_opt_value(res, 0, RELSTATS_RELTUPLES)); - Assert(!args[2].isnull); + /* Set the remaining values. */ + relvalues.version = version; + set_int32_arg(&relvalues.relpages, + get_opt_value(res, 0, RELSTATS_RELPAGES)); + Assert(!relvalues.relpages.isnull); + set_float_arg(&relvalues.reltuples, + get_opt_value(res, 0, RELSTATS_RELTUPLES)); + Assert(!relvalues.reltuples.isnull); /* We don't import relallvisible/relallfrozen. */ - args[3].value = (Datum) 0; - args[3].isnull = true; - args[4].value = (Datum) 0; - args[4].isnull = true; + relvalues.relallvisible.value = (Datum) 0; + relvalues.relallvisible.isnull = true; + relvalues.relallfrozen.value = (Datum) 0; + relvalues.relallfrozen.isnull = true; /* Try to import the statistics. */ - if (!import_relation_statistics(relation, &args[0], &args[1], - &args[2], &args[3], &args[4])) + if (!import_relation_statistics(relation, &relvalues)) { ereport(WARNING, errmsg("could not import statistics for foreign table \"%s.%s\" --- relation statistics import failed for this foreign table", diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index c546b3d6375d..fb3ee1b071b5 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -185,6 +185,7 @@ AttrMissing AttrNumber AttrResultArgMap AttributeOpts +AttributeStatsValues AuthRequest AuthToken AutoPrewarmReadStreamData @@ -2622,6 +2623,7 @@ Relation RelationData RelationInfo RelationPtr +RelationStatsValues RelationSyncEntry RelcacheCallbackFunction ReleaseMatchCB -- 2.55.0
signature.asc
Description: PGP signature
