From c6cfd11ed460204cb96c5c35c7cc513af14aaa84 Mon Sep 17 00:00:00 2001 From: Corey Huinker 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