On Thu, Jun 18, 2026 at 6:43 AM Etsuro Fujita <[email protected]> wrote:
> On Thu, Jun 18, 2026 at 1:00 AM Corey Huinker <[email protected]> > wrote: > > >> IMO I don't think we need to go that far, because 1) the new functions > >> are provided for FDW authors only, and 2) we don't make changes to the > >> stats that often. > > > > > > That would be simpler, to be sure. However, in the past I've received > pushback on functions that had a large number of parameters, and this would > definitely be a large number of parameters, approximately 17, so I thought > I should at least offer the variadic way. > > > > I'm proceeding with the many-parameters model. > > I think that that's acceptable, considering that > heap_create_with_catalog() has 21 parameters. > > Thanks! > > Best regards, > Etsuro Fujita > Attached are two patches to remove SPI from the postgres_fdw.c, replaced by local C functions. The division into two patches is entirely for readability. Separately, each one represents a half-measure that would benefit no one. The first does the minimal changes needed to get the relation-stats to stop using SPI, and the second does the same for attribute stats, removes all vestiges of SPI from postgres_fdw.c, and adds some tests to make sure that a foreign table has the exact stats of the source loopback table. The function import_attribute_stats() is a bit parameter-happy, but that's mostly necessary. I left the non-key parameters as all type char* because that avoids even more "bool foo_isnull" parameters, and that allows the caller to not have to rework the various stat types into their corresponding C types (float, int4, float[], and the anyarrays that basically aren't cast-able by any client program anyway). I'm open to requiring the caller to use the right datatypes for some of the parameters, but as I said there is no way to "win" with the anyarrays, and even the pg_restore_attribute_stats() function falls back to type text for those. To be clear, this solves the SPI problem, but questions about the design of attribute_statistics_update() and relation_statistics_update() remain, but those concerns are now isolated within their respective files attribute_stats.c and relation_stats.c. The inefficiencies therein aren't really in a critical path, so if we wanted to leave them be until v20 they could, but if time allows I'd at least like try unwinding some of that. But first, let's get SPI out of postgres_fdw.c.
From c6cfd11ed460204cb96c5c35c7cc513af14aaa84 Mon Sep 17 00:00:00 2001 From: Corey Huinker <[email protected]> Date: Wed, 17 Jun 2026 16:04:55 -0400 Subject: [PATCH v1 1/2] Remove SPI in favor of import_relation_statistics. This patch removes the SPI calls in postgres_fdw.c that were related to relation statistics. Instead, the function import_relation_statiticss() has been created specifically for this purpose. The main purpose of this patch is to make a simple API that is usable by any foreign data wrapper, but is not limited to usage by foreign data wrappers. Presently the function marshalls the paratmeters to make a call to relation_statistics_update(). This is not an ideal end-state, as this creates un-necessary value translations. The solution is refactor relation_statistics_update() to be a more conventional, non-fcinfo-style function, and refactor pg_restore_relation_stats() and pg_clear_relation_stats() to accommodate that. This patch removes the difficulties that SPI presents from postgres_fdw.c and localizes the code redundancy to relation_stats.c. --- src/include/statistics/relation_stats.h | 22 ++++++ src/backend/statistics/relation_stats.c | 96 +++++++++++++++++++++++++ contrib/postgres_fdw/postgres_fdw.c | 62 ++++------------ 3 files changed, 133 insertions(+), 47 deletions(-) create mode 100644 src/include/statistics/relation_stats.h diff --git a/src/include/statistics/relation_stats.h b/src/include/statistics/relation_stats.h new file mode 100644 index 00000000000..f68a1a755a1 --- /dev/null +++ b/src/include/statistics/relation_stats.h @@ -0,0 +1,22 @@ +/*------------------------------------------------------------------------- + * + * relation_stats.h + * Functions for the internal manipulation of relation statistics. + * + * Portions Copyright (c) 1996-2026, PostgreSQL Global Development Group + * Portions Copyright (c) 1994, Regents of the University of California + * + * src/include/statistics/relation_stats.h + * + *------------------------------------------------------------------------- + */ +#ifndef RELATION_STATS_H + +#include "access/genam.h" + +bool import_relation_statistics(Relation rel, const char *relpages, + const char *reltuples, const char *relallvisible, + const char *relallfrozen); + +#define RELATION_STATS_H +#endif diff --git a/src/backend/statistics/relation_stats.c b/src/backend/statistics/relation_stats.c index d6631e9a9a4..9c308ee50e5 100644 --- a/src/backend/statistics/relation_stats.c +++ b/src/backend/statistics/relation_stats.c @@ -22,7 +22,9 @@ #include "catalog/namespace.h" #include "nodes/makefuncs.h" #include "statistics/stat_utils.h" +#include "statistics/relation_stats.h" #include "utils/builtins.h" +#include "utils/float.h" #include "utils/fmgroids.h" #include "utils/fmgrprotos.h" #include "utils/lsyscache.h" @@ -241,3 +243,97 @@ pg_restore_relation_stats(PG_FUNCTION_ARGS) PG_RETURN_BOOL(result); } + +/* + * Convenience routine to parse BlockNumber values, and emit a warning + * on parse errors. + * + * Returns 0 if the value is NULL or invalid. + */ +static BlockNumber +str_to_blocknumber(const char *s) +{ + const BlockNumber default_value = 0; + + BlockNumber result; + ErrorSaveContext escontext = {T_ErrorSaveContext}; + + if (!s) + return default_value; + + result = uint32in_subr(s, NULL, "BlockNumber", (Node *) &escontext); + + if (escontext.error_occurred) + { + escontext.error_data->elevel = WARNING; + ThrowErrorData(escontext.error_data); + FreeErrorData(escontext.error_data); + + return default_value; + } + + return result; +} + +/* + * Convenience routine to parse float values, and emit a warning on parse + * errors. + * + * Returns -1.0 if the value is NULL or invalid. + */ +static float +str_to_float(const char *s) +{ + const float default_value = -1.0; + + float result; + + ErrorSaveContext escontext = {T_ErrorSaveContext}; + + if (!s) + return default_value; + + result = float4in_internal((char *) s, NULL, "float", s, (Node *) &escontext); + + if (escontext.error_occurred) + { + escontext.error_data->elevel = WARNING; + ThrowErrorData(escontext.error_data); + FreeErrorData(escontext.error_data); + return default_value; + } + + return result; +} + +/* + * Import relation statistics from regular string inputs. + */ +bool +import_relation_statistics(Relation rel, const char *relpages, + const char *reltuples, const char *relallvisible, + const char *relallfrozen) +{ + LOCAL_FCINFO(newfcinfo, NUM_RELATION_STATS_ARGS); + + InitFunctionCallInfoData(*newfcinfo, NULL, NUM_RELATION_STATS_ARGS, InvalidOid, NULL, NULL); + + /* + * Convert all string inputs to their required datatypes. NULL values are + * left as the default. + */ + 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; + newfcinfo->args[RELPAGES_ARG].value = UInt32GetDatum(str_to_blocknumber(relpages)); + newfcinfo->args[RELPAGES_ARG].isnull = false; + newfcinfo->args[RELTUPLES_ARG].value = Float4GetDatum(str_to_float(reltuples)); + newfcinfo->args[RELTUPLES_ARG].isnull = false; + newfcinfo->args[RELALLVISIBLE_ARG].value = UInt32GetDatum(str_to_blocknumber(relallvisible)); + newfcinfo->args[RELALLVISIBLE_ARG].isnull = false; + newfcinfo->args[RELALLFROZEN_ARG].value = UInt32GetDatum(str_to_blocknumber(relallfrozen)); + newfcinfo->args[RELALLFROZEN_ARG].isnull = false; + + return relation_statistics_update(newfcinfo); +} diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c index 6dbae583ecc..673d56117b8 100644 --- a/contrib/postgres_fdw/postgres_fdw.c +++ b/contrib/postgres_fdw/postgres_fdw.c @@ -43,6 +43,7 @@ #include "parser/parsetree.h" #include "postgres_fdw.h" #include "statistics/statistics.h" +#include "statistics/relation_stats.h" #include "storage/latch.h" #include "utils/builtins.h" #include "utils/float.h" @@ -367,33 +368,6 @@ enum AttStatsColumns ATTSTATS_NUM_FIELDS, }; -/* Relation stats import query */ -static const char *relimport_sql = -"SELECT pg_catalog.pg_restore_relation_stats(\n" -"\t'version', $1,\n" -"\t'schemaname', $2,\n" -"\t'relname', $3,\n" -"\t'relpages', $4::integer,\n" -"\t'reltuples', $5::real)"; - -/* Argument order in relation stats import query */ -enum RelImportSqlArgs -{ - RELIMPORT_SQL_VERSION = 0, - RELIMPORT_SQL_SCHEMANAME, - RELIMPORT_SQL_RELNAME, - RELIMPORT_SQL_RELPAGES, - RELIMPORT_SQL_RELTUPLES, - RELIMPORT_SQL_NUM_FIELDS -}; - -/* Argument types in relation stats import query */ -static const Oid relimport_argtypes[RELIMPORT_SQL_NUM_FIELDS] = -{ - INT4OID, TEXTOID, TEXTOID, TEXTOID, - TEXTOID, -}; - /* Attribute stats import query */ static const char *attimport_sql = "SELECT pg_catalog.pg_restore_attribute_stats(\n" @@ -714,7 +688,8 @@ static bool match_attrmap(PGresult *res, const char *remote_relname, int attrcnt, RemoteAttributeMapping *remattrmap); -static bool import_fetched_statistics(const char *schemaname, +static bool import_fetched_statistics(Relation rel, + const char *schemaname, const char *relname, int attrcnt, const RemoteAttributeMapping *remattrmap, @@ -5661,7 +5636,7 @@ postgresImportForeignStatistics(Relation relation, List *va_cols, int elevel) &attrcnt, &remattrmap, &remstats); if (ok) - ok = import_fetched_statistics(schemaname, relname, + ok = import_fetched_statistics(relation, schemaname, relname, attrcnt, remattrmap, &remstats); if (ok) @@ -6128,7 +6103,8 @@ match_attrmap(PGresult *res, * Import fetched statistics into the local statistics tables. */ static bool -import_fetched_statistics(const char *schemaname, +import_fetched_statistics(Relation relation, + const char *schemaname, const char *relname, int attrcnt, const RemoteAttributeMapping *remattrmap, @@ -6141,6 +6117,9 @@ import_fetched_statistics(const char *schemaname, int spirc; bool ok = false; + char *relpages = NULL; + char *reltuples = NULL; + /* Assign all the invariant parameters common to relation/attribute stats */ values[ATTIMPORT_SQL_VERSION] = Int32GetDatum(remstats->server_version_num); nulls[ATTIMPORT_SQL_VERSION] = ' '; @@ -6233,27 +6212,16 @@ import_fetched_statistics(const char *schemaname, /* * Import relation stats. We only perform this once, so there is no point * in preparing the statement. - * - * We can re-use the values/nulls because the number of parameters is less - * and the first three params are the same as attimport_sql. */ Assert(remstats->rel != NULL); - Assert(PQnfields(remstats->rel) == RELSTATS_NUM_FIELDS); - Assert(PQntuples(remstats->rel) == 1); - map_field_to_arg(remstats->rel, 0, RELSTATS_RELPAGES, - RELIMPORT_SQL_RELPAGES, values, nulls); - map_field_to_arg(remstats->rel, 0, RELSTATS_RELTUPLES, - RELIMPORT_SQL_RELTUPLES, values, nulls); + if (!PQgetisnull(remstats->rel, 0, RELSTATS_RELPAGES)) + relpages = PQgetvalue(remstats->rel, 0, RELSTATS_RELPAGES); - spirc = SPI_execute_with_args(relimport_sql, - RELIMPORT_SQL_NUM_FIELDS, - (Oid *) relimport_argtypes, - values, nulls, false, 1); - if (spirc != SPI_OK_SELECT) - elog(ERROR, "failed to execute relimport_sql query for foreign table \"%s.%s\"", - schemaname, relname); + if (!PQgetisnull(remstats->rel, 0, RELSTATS_RELTUPLES)) + reltuples = PQgetvalue(remstats->rel, 0, RELSTATS_RELTUPLES); - if (!import_spi_query_ok()) + if (!import_relation_statistics(relation, relpages, reltuples, + NULL, NULL)) { ereport(WARNING, errmsg("could not import statistics for foreign table \"%s.%s\" --- relation statistics import failed for this foreign table", base-commit: f04781df5daf112840d08eab39fb95505fda6c9c -- 2.54.0
From 3b9cfbbbcda36ae2f4444b5eefe7a8f2961333f0 Mon Sep 17 00:00:00 2001 From: Corey Huinker <[email protected]> Date: Thu, 18 Jun 2026 16:28:10 -0400 Subject: [PATCH v1 2/2] Remove SPI in favor of import_attribute_statistics. This patch removes the SPI calls in postgres_fdw.c that were related to attribute statistics. Instead, the functions import_attribute_statistics() and clear_attribute_statistics() have been created for this purpose. Like the preceding patch, the goal is to remove SPI from the workflow and isolate all code redundancy in attribute_stats.c, where it can be handled at a later time. Some additional checks were added to the regression tests to ensure that values are properly conveyed from a loopback-foreign table to the local table. --- src/include/statistics/attribute_stats.h | 36 +++ src/backend/statistics/attribute_stats.c | 192 +++++++++++++ .../postgres_fdw/expected/postgres_fdw.out | 41 +++ contrib/postgres_fdw/postgres_fdw.c | 258 +++--------------- contrib/postgres_fdw/sql/postgres_fdw.sql | 32 +++ 5 files changed, 339 insertions(+), 220 deletions(-) create mode 100644 src/include/statistics/attribute_stats.h diff --git a/src/include/statistics/attribute_stats.h b/src/include/statistics/attribute_stats.h new file mode 100644 index 00000000000..4ebde0c7c3b --- /dev/null +++ b/src/include/statistics/attribute_stats.h @@ -0,0 +1,36 @@ +/*------------------------------------------------------------------------- + * + * attribute_stats.h + * Functions for the internal manipulation of attribute statistics. + * + * Portions Copyright (c) 1996-2026, PostgreSQL Global Development Group + * Portions Copyright (c) 1994, Regents of the University of California + * + * src/include/statistics/attribute_stats.h + * + *------------------------------------------------------------------------- + */ +#ifndef ATTRIBUTE_STATS_H + +#include "access/genam.h" + + +bool delete_attribute_statistics(Relation rel, AttrNumber attnum, bool inherited); + +bool import_attribute_statistics(Relation rel, const char *attname, + AttrNumber attnum, bool inherited, + const char *null_frac, const char *avg_width, + const char *n_distinct, + const char *most_common_vals, + const char *most_common_freqs, + const char *histogram_bounds, + const char *correlation, + const char *most_common_elems, + const char *most_common_elem_freqs, + const char *elem_count_histogram, + const char *range_length_histogram, + const char *range_empty_frac, + const char *range_bounds_histogram); + +#define ATTRIBUTE_STATS_H +#endif diff --git a/src/backend/statistics/attribute_stats.c b/src/backend/statistics/attribute_stats.c index 1cc4d657231..f67d62c0104 100644 --- a/src/backend/statistics/attribute_stats.c +++ b/src/backend/statistics/attribute_stats.c @@ -22,10 +22,12 @@ #include "catalog/namespace.h" #include "catalog/pg_operator.h" #include "nodes/makefuncs.h" +#include "statistics/attribute_stats.h" #include "statistics/statistics.h" #include "statistics/stat_utils.h" #include "utils/array.h" #include "utils/builtins.h" +#include "utils/float.h" #include "utils/fmgroids.h" #include "utils/lsyscache.h" #include "utils/syscache.h" @@ -688,3 +690,193 @@ pg_restore_attribute_stats(PG_FUNCTION_ARGS) PG_RETURN_BOOL(result); } + +/* + * Convenience routine for setting optional text arguments + */ +static void +set_text_arg(NullableDatum *arg, const char *s) +{ + if (s) + { + arg->value = CStringGetTextDatum(s); + arg->isnull = false; + } + else + { + arg->value = (Datum) 0; + arg->isnull = true; + } +} + +/* + * Convenience routine for setting optional float arguments + */ +static void +set_float_arg(NullableDatum *arg, const char *s) +{ + arg->value = (Datum) 0; + arg->isnull = true; + + if (s) + { + ErrorSaveContext escontext = {T_ErrorSaveContext}; + + float4 val; + + val = float4in_internal((char *) s, NULL, "float", s, (Node *) &escontext); + + if (escontext.error_occurred) + { + escontext.error_data->elevel = WARNING; + ThrowErrorData(escontext.error_data); + FreeErrorData(escontext.error_data); + } + else + { + arg->value = Float4GetDatum(val); + arg->isnull = false; + } + } +} + +/* + * Convenience routine for setting optional int32 arguments + */ +static void +set_int32_arg(NullableDatum *arg, const char *s) +{ + arg->value = (Datum) 0; + arg->isnull = true; + + if (s) + { + ErrorSaveContext escontext = {T_ErrorSaveContext}; + + int32 val; + + val = pg_strtoint32_safe(s, (Node *) &escontext); + + if (escontext.error_occurred) + { + escontext.error_data->elevel = WARNING; + ThrowErrorData(escontext.error_data); + FreeErrorData(escontext.error_data); + } + else + { + arg->value = Int32GetDatum(val); + arg->isnull = false; + } + } +} + +/* + * Convenience routine for setting optional float[] arguments + */ +static void +set_floatarr_arg(NullableDatum *arg, const char *s) +{ + arg->value = (Datum) 0; + arg->isnull = true; + + if (s) + { + ErrorSaveContext escontext = {T_ErrorSaveContext}; + + FmgrInfo flinfo; + Datum val; + + fmgr_info(F_ARRAY_IN, &flinfo); + + if (!InputFunctionCallSafe(&flinfo, (char *) s, FLOAT4OID, -1, + (Node *) &escontext, &val)) + { + escontext.error_data->elevel = WARNING; + ThrowErrorData(escontext.error_data); + FreeErrorData(escontext.error_data); + } + else + { + arg->value = val; + arg->isnull = false; + } + } +} + +/* + * Delete attribute statistics. + */ +bool +delete_attribute_statistics(Relation rel, AttrNumber attnum, bool inherited) +{ + return delete_pg_statistic(RelationGetRelid(rel), attnum, inherited); +} + +/* + * Import attribute statistics from regular string inputs for all statitical + * values. + * + * To convey a "null"/not-set value for attnum, use InvalidAttrNumber. + */ +bool +import_attribute_statistics(Relation rel, const char *attname, + AttrNumber attnum, bool inherited, + const char *null_frac, const char *avg_width, + const char *n_distinct, + const char *most_common_vals, + const char *most_common_freqs, + const char *histogram_bounds, + const char *correlation, + const char *most_common_elems, + const char *most_common_elem_freqs, + const char *elem_count_histogram, + const char *range_length_histogram, + const char *range_empty_frac, + const char *range_bounds_histogram) +{ + LOCAL_FCINFO(newfcinfo, NUM_ATTRIBUTE_STATS_ARGS); + + InitFunctionCallInfoData(*newfcinfo, NULL, NUM_ATTRIBUTE_STATS_ARGS, InvalidOid, NULL, NULL); + + /* + * Convert all string inputs to their required datatypes. NULL values are + * left as the default. + */ + newfcinfo->args[ATTRELSCHEMA_ARG].value = CStringGetTextDatum(get_namespace_name(RelationGetNamespace(rel))); + newfcinfo->args[ATTRELSCHEMA_ARG].isnull = false; + newfcinfo->args[ATTRELNAME_ARG].value = CStringGetTextDatum(RelationGetRelationName(rel)); + newfcinfo->args[ATTRELNAME_ARG].isnull = false; + + set_text_arg(&newfcinfo->args[ATTNAME_ARG], attname); + + if (attnum != InvalidAttrNumber) + { + newfcinfo->args[ATTNUM_ARG].value = Int16GetDatum(attnum); + newfcinfo->args[ATTNUM_ARG].isnull = false; + } + else + { + newfcinfo->args[ATTNUM_ARG].value = (Datum) 0; + newfcinfo->args[ATTNUM_ARG].isnull = true; + } + + newfcinfo->args[INHERITED_ARG].value = BoolGetDatum(inherited); + newfcinfo->args[INHERITED_ARG].isnull = false; + + set_float_arg(&newfcinfo->args[NULL_FRAC_ARG], null_frac); + set_int32_arg(&newfcinfo->args[AVG_WIDTH_ARG], avg_width); + set_float_arg(&newfcinfo->args[N_DISTINCT_ARG], n_distinct); + set_text_arg(&newfcinfo->args[MOST_COMMON_VALS_ARG], most_common_vals); + set_floatarr_arg(&newfcinfo->args[MOST_COMMON_FREQS_ARG], most_common_freqs); + set_text_arg(&newfcinfo->args[HISTOGRAM_BOUNDS_ARG], histogram_bounds); + set_float_arg(&newfcinfo->args[CORRELATION_ARG], correlation); + set_text_arg(&newfcinfo->args[MOST_COMMON_ELEMS_ARG], most_common_elems); + set_floatarr_arg(&newfcinfo->args[MOST_COMMON_ELEM_FREQS_ARG], most_common_elem_freqs); + set_floatarr_arg(&newfcinfo->args[ELEM_COUNT_HISTOGRAM_ARG], elem_count_histogram); + set_text_arg(&newfcinfo->args[RANGE_LENGTH_HISTOGRAM_ARG], range_length_histogram); + set_float_arg(&newfcinfo->args[RANGE_EMPTY_FRAC_ARG], range_empty_frac); + set_text_arg(&newfcinfo->args[RANGE_BOUNDS_HISTOGRAM_ARG], range_bounds_histogram); + + return attribute_statistics_update(newfcinfo); +} diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index e90289e4ab1..e931fa64e0d 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -12927,6 +12927,47 @@ ANALYZE dtest_table; ANALYZE VERBOSE dtest_ftable; -- should work INFO: importing statistics for foreign table "public.dtest_ftable" INFO: finished importing statistics for foreign table "public.dtest_ftable" +-- dtest_ftables stats should now exactly match dtest_table +-- compare the rowcounts, should get 0 rows back +SELECT COUNT(*) FROM pg_stats +WHERE schemaname = 'public' AND tablename = 'dtest_table' +EXCEPT +SELECT COUNT(*) FROM pg_stats +WHERE schemaname = 'public' AND tablename = 'dtest_ftable'; + count +------- +(0 rows) + +-- compare values, should match +SELECT relpages, reltuples +FROM pg_class +WHERE oid = 'public.dtest_table'::regclass +EXCEPT +SELECT relpages, reltuples +FROM pg_class +WHERE oid = 'public.dtest_ftable'::regclass; + relpages | reltuples +----------+----------- +(0 rows) + +-- test only a few stats columns common to integer types +SELECT attname, inherited, null_frac, avg_width, n_distinct, + most_common_vals::text as mcv, most_common_freqs, + histogram_bounds::text as hb, correlation +FROM pg_stats +WHERE schemaname = 'public' +AND tablename = 'dtest_table' +EXCEPT +SELECT attname, inherited, null_frac, avg_width, n_distinct, + most_common_vals::text as mcv, most_common_freqs, + histogram_bounds::text as hb, correlation +FROM pg_stats +WHERE schemaname = 'public' +AND tablename = 'dtest_ftable'; + attname | inherited | null_frac | avg_width | n_distinct | mcv | most_common_freqs | hb | correlation +---------+-----------+-----------+-----------+------------+-----+-------------------+----+------------- +(0 rows) + -- cleanup DROP FOREIGN TABLE simport_ftable; DROP FOREIGN TABLE simport_fview; diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c index 673d56117b8..8aa589d5a56 100644 --- a/contrib/postgres_fdw/postgres_fdw.c +++ b/contrib/postgres_fdw/postgres_fdw.c @@ -24,7 +24,6 @@ #include "commands/vacuum.h" #include "executor/execAsync.h" #include "executor/instrument.h" -#include "executor/spi.h" #include "foreign/fdwapi.h" #include "funcapi.h" #include "miscadmin.h" @@ -43,6 +42,7 @@ #include "parser/parsetree.h" #include "postgres_fdw.h" #include "statistics/statistics.h" +#include "statistics/attribute_stats.h" #include "statistics/relation_stats.h" #include "storage/latch.h" #include "utils/builtins.h" @@ -368,109 +368,6 @@ enum AttStatsColumns ATTSTATS_NUM_FIELDS, }; -/* Attribute stats import query */ -static const char *attimport_sql = -"SELECT pg_catalog.pg_restore_attribute_stats(\n" -"\t'version', $1,\n" -"\t'schemaname', $2,\n" -"\t'relname', $3,\n" -"\t'attnum', $4,\n" -"\t'inherited', false::boolean,\n" -"\t'null_frac', $5::real,\n" -"\t'avg_width', $6::integer,\n" -"\t'n_distinct', $7::real,\n" -"\t'most_common_vals', $8,\n" -"\t'most_common_freqs', $9::real[],\n" -"\t'histogram_bounds', $10,\n" -"\t'correlation', $11::real,\n" -"\t'most_common_elems', $12,\n" -"\t'most_common_elem_freqs', $13::real[],\n" -"\t'elem_count_histogram', $14::real[],\n" -"\t'range_length_histogram', $15,\n" -"\t'range_empty_frac', $16::real,\n" -"\t'range_bounds_histogram', $17)"; - -/* Argument order in attribute stats import query */ -enum AttImportSqlArgs -{ - ATTIMPORT_SQL_VERSION = 0, - ATTIMPORT_SQL_SCHEMANAME, - ATTIMPORT_SQL_RELNAME, - ATTIMPORT_SQL_ATTNUM, - ATTIMPORT_SQL_NULL_FRAC, - ATTIMPORT_SQL_AVG_WIDTH, - ATTIMPORT_SQL_N_DISTINCT, - ATTIMPORT_SQL_MOST_COMMON_VALS, - ATTIMPORT_SQL_MOST_COMMON_FREQS, - ATTIMPORT_SQL_HISTOGRAM_BOUNDS, - ATTIMPORT_SQL_CORRELATION, - ATTIMPORT_SQL_MOST_COMMON_ELEMS, - ATTIMPORT_SQL_MOST_COMMON_ELEM_FREQS, - ATTIMPORT_SQL_ELEM_COUNT_HISTOGRAM, - ATTIMPORT_SQL_RANGE_LENGTH_HISTOGRAM, - ATTIMPORT_SQL_RANGE_EMPTY_FRAC, - ATTIMPORT_SQL_RANGE_BOUNDS_HISTOGRAM, - ATTIMPORT_SQL_NUM_FIELDS -}; - -/* Argument types in attribute stats import query */ -static const Oid attimport_argtypes[ATTIMPORT_SQL_NUM_FIELDS] = -{ - INT4OID, TEXTOID, TEXTOID, INT2OID, - TEXTOID, TEXTOID, TEXTOID, TEXTOID, - TEXTOID, TEXTOID, TEXTOID, TEXTOID, - TEXTOID, TEXTOID, TEXTOID, TEXTOID, - TEXTOID, -}; - -/* - * The mapping of attribute stats query columns to the positional arguments in - * the prepared pg_restore_attribute_stats() statement. - */ -typedef struct -{ - enum AttStatsColumns res_field; - enum AttImportSqlArgs arg_num; -} AttrResultArgMap; - -#define NUM_MAPPED_ATTIMPORT_ARGS 13 - -static const AttrResultArgMap attr_result_arg_map[NUM_MAPPED_ATTIMPORT_ARGS] = -{ - {ATTSTATS_NULL_FRAC, ATTIMPORT_SQL_NULL_FRAC}, - {ATTSTATS_AVG_WIDTH, ATTIMPORT_SQL_AVG_WIDTH}, - {ATTSTATS_N_DISTINCT, ATTIMPORT_SQL_N_DISTINCT}, - {ATTSTATS_MOST_COMMON_VALS, ATTIMPORT_SQL_MOST_COMMON_VALS}, - {ATTSTATS_MOST_COMMON_FREQS, ATTIMPORT_SQL_MOST_COMMON_FREQS}, - {ATTSTATS_HISTOGRAM_BOUNDS, ATTIMPORT_SQL_HISTOGRAM_BOUNDS}, - {ATTSTATS_CORRELATION, ATTIMPORT_SQL_CORRELATION}, - {ATTSTATS_MOST_COMMON_ELEMS, ATTIMPORT_SQL_MOST_COMMON_ELEMS}, - {ATTSTATS_MOST_COMMON_ELEM_FREQS, ATTIMPORT_SQL_MOST_COMMON_ELEM_FREQS}, - {ATTSTATS_ELEM_COUNT_HISTOGRAM, ATTIMPORT_SQL_ELEM_COUNT_HISTOGRAM}, - {ATTSTATS_RANGE_LENGTH_HISTOGRAM, ATTIMPORT_SQL_RANGE_LENGTH_HISTOGRAM}, - {ATTSTATS_RANGE_EMPTY_FRAC, ATTIMPORT_SQL_RANGE_EMPTY_FRAC}, - {ATTSTATS_RANGE_BOUNDS_HISTOGRAM, ATTIMPORT_SQL_RANGE_BOUNDS_HISTOGRAM}, -}; - -/* Attribute stats clear query */ -static const char *attclear_sql = -"SELECT pg_catalog.pg_clear_attribute_stats($1, $2, $3, false)"; - -/* Argument order in attribute stats clear query */ -enum AttClearSqlArgs -{ - ATTCLEAR_SQL_SCHEMANAME = 0, - ATTCLEAR_SQL_RELNAME, - ATTCLEAR_SQL_ATTNAME, - ATTCLEAR_SQL_NUM_FIELDS -}; - -/* Argument types in attribute stats clear query */ -static const Oid attclear_argtypes[ATTCLEAR_SQL_NUM_FIELDS] = -{ - TEXTOID, TEXTOID, TEXTOID, -}; - /* * SQL functions */ @@ -688,15 +585,12 @@ static bool match_attrmap(PGresult *res, const char *remote_relname, int attrcnt, RemoteAttributeMapping *remattrmap); -static bool import_fetched_statistics(Relation rel, +static bool import_fetched_statistics(Relation relation, const char *schemaname, const char *relname, int attrcnt, const RemoteAttributeMapping *remattrmap, RemoteStatsResults *remstats); -static void map_field_to_arg(PGresult *res, int row, int field, - int arg, Datum *values, char *nulls); -static bool import_spi_query_ok(void); static void produce_tuple_asynchronously(AsyncRequest *areq, bool fetch); static void fetch_more_data_begin(AsyncRequest *areq); static void complete_pending_request(AsyncRequest *areq); @@ -6099,6 +5993,19 @@ match_attrmap(PGresult *res, return true; } + +/* + * Conenience routine to fetch + */ +static char * +get_opt_value(PGresult *res, int row, int col) +{ + if (PQgetisnull(res, row, col)) + return NULL; + + return PQgetvalue(res, row, col); +} + /* * Import fetched statistics into the local statistics tables. */ @@ -6110,28 +6017,11 @@ import_fetched_statistics(Relation relation, const RemoteAttributeMapping *remattrmap, RemoteStatsResults *remstats) { - SPIPlanPtr attimport_plan = NULL; - SPIPlanPtr attclear_plan = NULL; - Datum values[ATTIMPORT_SQL_NUM_FIELDS]; - char nulls[ATTIMPORT_SQL_NUM_FIELDS]; - int spirc; bool ok = false; char *relpages = NULL; char *reltuples = NULL; - /* Assign all the invariant parameters common to relation/attribute stats */ - values[ATTIMPORT_SQL_VERSION] = Int32GetDatum(remstats->server_version_num); - nulls[ATTIMPORT_SQL_VERSION] = ' '; - - values[ATTIMPORT_SQL_SCHEMANAME] = CStringGetTextDatum(schemaname); - nulls[ATTIMPORT_SQL_SCHEMANAME] = ' '; - - values[ATTIMPORT_SQL_RELNAME] = CStringGetTextDatum(relname); - nulls[ATTIMPORT_SQL_RELNAME] = ' '; - - SPI_connect(); - /* * We import attribute statistics first, if any, because those are more * prone to errors. This avoids making a modification of pg_class that @@ -6139,26 +6029,16 @@ import_fetched_statistics(Relation relation, */ if (remstats->att != NULL) { - Assert(PQnfields(remstats->att) == ATTSTATS_NUM_FIELDS); - Assert(PQntuples(remstats->att) >= 1); + PGresult *res = remstats->att; - attimport_plan = SPI_prepare(attimport_sql, ATTIMPORT_SQL_NUM_FIELDS, - (Oid *) attimport_argtypes); - if (attimport_plan == NULL) - elog(ERROR, "failed to prepare attimport_sql query"); - - attclear_plan = SPI_prepare(attclear_sql, ATTCLEAR_SQL_NUM_FIELDS, - (Oid *) attclear_argtypes); - if (attclear_plan == NULL) - elog(ERROR, "failed to prepare attclear_sql query"); - - nulls[ATTIMPORT_SQL_ATTNUM] = ' '; + Assert(PQnfields(res) == ATTSTATS_NUM_FIELDS); + Assert(PQntuples(res) >= 1); for (int mapidx = 0; mapidx < attrcnt; mapidx++) { int row = remattrmap[mapidx].res_index; - Datum *values2 = values + 1; - char *nulls2 = nulls + 1; + AttrNumber attnum = remattrmap[mapidx].local_attnum; + const char *attname = remattrmap[mapidx].local_attname; /* All mappings should have been assigned a result set row. */ Assert(row >= 0); @@ -6168,42 +6048,28 @@ import_fetched_statistics(Relation relation, */ CHECK_FOR_INTERRUPTS(); - /* - * First, clear existing attribute stats. - * - * We can re-use the values/nulls because the number of parameters - * is less and the first two params are the same as the second and - * third ones in attimport_sql. - */ - values2[ATTCLEAR_SQL_ATTNAME] = - CStringGetTextDatum(remattrmap[mapidx].local_attname); + delete_attribute_statistics(relation, attnum, false); - spirc = SPI_execute_plan(attclear_plan, values2, nulls2, false, 1); - if (spirc != SPI_OK_SELECT) - elog(ERROR, "failed to execute attclear_sql query for column \"%s\" of foreign table \"%s.%s\"", - remattrmap[mapidx].local_attname, schemaname, relname); - - values[ATTIMPORT_SQL_ATTNUM] = - Int16GetDatum(remattrmap[mapidx].local_attnum); - - /* Loop through all mappable columns to set remaining arguments */ - for (int i = 0; i < NUM_MAPPED_ATTIMPORT_ARGS; i++) - map_field_to_arg(remstats->att, row, - attr_result_arg_map[i].res_field, - attr_result_arg_map[i].arg_num, - values, nulls); - - spirc = SPI_execute_plan(attimport_plan, values, nulls, false, 1); - if (spirc != SPI_OK_SELECT) - elog(ERROR, "failed to execute attimport_sql query for column \"%s\" of foreign table \"%s.%s\"", - remattrmap[mapidx].local_attname, schemaname, relname); - - if (!import_spi_query_ok()) + if (!import_attribute_statistics(relation, attname, + InvalidAttrNumber, + false, + get_opt_value(res, row, ATTSTATS_NULL_FRAC), + get_opt_value(res, row, ATTSTATS_AVG_WIDTH), + get_opt_value(res, row, ATTSTATS_N_DISTINCT), + get_opt_value(res, row, ATTSTATS_MOST_COMMON_VALS), + get_opt_value(res, row, ATTSTATS_MOST_COMMON_FREQS), + get_opt_value(res, row, ATTSTATS_HISTOGRAM_BOUNDS), + get_opt_value(res, row, ATTSTATS_CORRELATION), + get_opt_value(res, row, ATTSTATS_MOST_COMMON_ELEMS), + get_opt_value(res, row, ATTSTATS_MOST_COMMON_ELEM_FREQS), + get_opt_value(res, row, ATTSTATS_ELEM_COUNT_HISTOGRAM), + get_opt_value(res, row, ATTSTATS_RANGE_LENGTH_HISTOGRAM), + get_opt_value(res, row, ATTSTATS_RANGE_EMPTY_FRAC), + get_opt_value(res, row, ATTSTATS_RANGE_BOUNDS_HISTOGRAM))) { ereport(WARNING, errmsg("could not import statistics for foreign table \"%s.%s\" --- attribute statistics import failed for column \"%s\" of this foreign table", - schemaname, relname, - remattrmap[mapidx].local_attname)); + schemaname, relname, attname)); goto import_cleanup; } } @@ -6232,57 +6098,9 @@ import_fetched_statistics(Relation relation, ok = true; import_cleanup: - if (attimport_plan) - SPI_freeplan(attimport_plan); - if (attclear_plan) - SPI_freeplan(attclear_plan); - SPI_finish(); return ok; } -/* - * Move a string value from a result set to a Text value of a Datum array. - */ -static void -map_field_to_arg(PGresult *res, int row, int field, - int arg, Datum *values, char *nulls) -{ - if (PQgetisnull(res, row, field)) - { - values[arg] = (Datum) 0; - nulls[arg] = 'n'; - } - else - { - const char *s = PQgetvalue(res, row, field); - - values[arg] = CStringGetTextDatum(s); - nulls[arg] = ' '; - } -} - -/* - * Check the 1x1 result set of a pg_restore_*_stats() command for success. - */ -static bool -import_spi_query_ok(void) -{ - TupleDesc tupdesc; - Datum dat; - bool isnull; - - Assert(SPI_tuptable != NULL); - Assert(SPI_processed == 1); - - tupdesc = SPI_tuptable->tupdesc; - Assert(tupdesc->natts == 1); - Assert(TupleDescAttr(tupdesc, 0)->atttypid == BOOLOID); - dat = SPI_getbinval(SPI_tuptable->vals[0], tupdesc, 1, &isnull); - Assert(!isnull); - - return DatumGetBool(dat); -} - /* * Import a foreign schema */ diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql index dfc58beb0d2..627177123b3 100644 --- a/contrib/postgres_fdw/sql/postgres_fdw.sql +++ b/contrib/postgres_fdw/sql/postgres_fdw.sql @@ -4584,6 +4584,38 @@ ANALYZE dtest_table; ANALYZE VERBOSE dtest_ftable; -- should work +-- dtest_ftables stats should now exactly match dtest_table +-- compare the rowcounts, should get 0 rows back +SELECT COUNT(*) FROM pg_stats +WHERE schemaname = 'public' AND tablename = 'dtest_table' +EXCEPT +SELECT COUNT(*) FROM pg_stats +WHERE schemaname = 'public' AND tablename = 'dtest_ftable'; + +-- compare values, should match +SELECT relpages, reltuples +FROM pg_class +WHERE oid = 'public.dtest_table'::regclass +EXCEPT +SELECT relpages, reltuples +FROM pg_class +WHERE oid = 'public.dtest_ftable'::regclass; + +-- test only a few stats columns common to integer types +SELECT attname, inherited, null_frac, avg_width, n_distinct, + most_common_vals::text as mcv, most_common_freqs, + histogram_bounds::text as hb, correlation +FROM pg_stats +WHERE schemaname = 'public' +AND tablename = 'dtest_table' +EXCEPT +SELECT attname, inherited, null_frac, avg_width, n_distinct, + most_common_vals::text as mcv, most_common_freqs, + histogram_bounds::text as hb, correlation +FROM pg_stats +WHERE schemaname = 'public' +AND tablename = 'dtest_ftable'; + -- cleanup DROP FOREIGN TABLE simport_ftable; DROP FOREIGN TABLE simport_fview; -- 2.54.0
