Subject: use of SPI by postgresImportForeignStatistics

From: Robert Haas <[email-masked]>
Date: Mon, 15 Jun 2026 13:50:46 -0400

Hi,

I'm concerned about the way that postgresImportForeignStatistics is
doing what it does. First, it's using SPI to execute two constant SQL
queries, attimport_sql and attclear_sql. However, it doesn't seem to
set the search_path, and these queries aren't fully robust against a
possibly-unsafe search_path (e.g. the call to
pg_restore_attribute_stats is schema-qualified, but the type names are
not!). Furthermore, the function we're calling takes a schema name and
a relation name as two separate arguments, rather than a single OID
argument. I'm not sure it's completely guaranteed that we'll end up
affecting the same relation that we locked in
postgresImportForeignStatistics, because e.g. what if the containing
schema is being concurrently renamed?

But even if it is absolutely guaranteed that we latch onto the correct
relation, this seems like the fundamentally wrong way to do this kind
of thing. It seems crazy to me that instead of exposing an interface
that would be well-suited to direct use by an FDW, the
statistics-import stuff exposes an interface that can only be
reasonably called via the FunctionCallInfo interface, which then
results in postgres_fdw needing to jump through hoops to construct and
execute SQL statements.

This doesn't look entirely easy to straighten out. A function like
attribute_statistics_update() is just not a good idea -- it mixes
together considerations that only exist when you want to call
something from SQL with general validity checking that's needed
regardless.

But I don't think we should ship this like this. Best case scenario,
it's overly complicated. Medium case scenario, it's also unreliable.
Worst case scenario, there are security vulnerabilities in here.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

---

From: Etsuro Fujita <[email-masked]>
Date: Tue, 16 Jun 2026 21:04:37 +0900

Hi Robert,

On Tue, Jun 16, 2026 at 2:50 AM Robert Haas <[email-masked]> wrote:
> not!).

Good catch!

> Furthermore, the function we're calling takes a schema name and

Ugh.  As pg_restore_attribute_stats/pg_restore_relation_stats are
volatile functions, SPI executes the queries in read-write mode,
causing an error, like "schema "foo" does not exist", or other
unexpected results in such a case.  Not sure what to do about this
issue right now.  Will think about it some more.

> results in postgres_fdw needing to jump through hoops to construct and

I thought it would be a good idea to use
pg_restore_attribute_stats/pg_restore_relation_stats, because future
changes in attribute/relation stats would be absorbed by these
functions, which would lower the maintenance cost of this feature.

Thanks for the comments!

Best regards,
Etsuro Fujita

---

From: Robert Haas <[email-masked]>
Date: Tue, 16 Jun 2026 10:34:37 -0400

> functions, which would lower the maintenance cost of this feature.

I agree that we want to reuse code, but this isn't the right way to do
it. For example, when we want to look up the OID of a relation from
SQL, we can say 'whatever'::regclass::oid. But when we want to do the
same thing from C, we don't construct a SELECT statement and execute
it via SPI. Instead, we have functions like RangeVarGetRelidExtended()
which provide access to the same underlying functionality more
directly.

Another example is converting strings to integers. The user calls
int4in(), which then hands off the call to pg_strtoint32_safe(), which
can also be called via pg_strtoint32(). Hence, C code should prefer to
use pg_strtoint32(), while SQL will go through int4in(). Both
ultimately reach the underlying pg_strtoint32_safe() function, but the
interfaces are different, so that we can have it be suitable both for
SQL access and for C access.

This needs to work more like that. The underlying code that ingests
and updates the stats should be shared, but the stuff that is specific
to a FunctionCallInfo interface needs to be separated out so that we
don't need to go through that when calling from C.

---

From: Corey Huinker <[email-masked]>
Date: Tue, 16 Jun 2026 14:08:51 -0400

> don't need to go through that when calling from C.
Back when pg_restore_attribute_stats and pg_restore_relation_stats were
being converted from positional to variadic functions, there was a patchset
or two (possibly un-posted, because I couldn't find it just now) where the
variadic functions re-marshalled the arguments into a call to a
fixed-parameter function. If that model were revived, we could have a more
conventional DirectFunctonCall() interface.

Is it possible that we never built a direct function call interface to a
variadic because we never needed one? Perhaps that time is now.

---

From: Corey Huinker <[email-masked]>
Date: Tue, 16 Jun 2026 15:11:45 -0400

> variadic because we never needed one? Perhaps that time is now.

Obviously, even that wouldn't get us to the no-FunctionCallInfo-at-all
goal, but it would get us out of the SPI situation. If we did have a
non-fcinfo function API, that API would either have to pass char *params
almost exclusively, or else each caller would have to do the translation or
float-array strings to double[] and such, which would be a lot of work for
the caller.

Would you be ok with a function like attribute_statistics_update, but
purely with cstring args? Obviously the callers would have to modify their
calls each time a new stat param is added, but that is seemingly preferable
for you than the current situation.

---

From: Robert Haas <[email-masked]>
Date: Tue, 16 Jun 2026 17:53:55 -0400

On Tue, Jun 16, 2026 at 3:11 PM Corey Huinker <[email-masked]> wrote:
> Obviously, even that wouldn't get us to the no-FunctionCallInfo-at-all goal, but it would get us out of the SPI situation. If we did have a non-fcinfo function API, that API would either have to pass char *params almost exclusively, or else each caller would have to do the translation or float-array strings to double[] and such, which would be a lot of work for the caller.
>
> Would you be ok with a function like attribute_statistics_update, but purely with cstring args? Obviously the callers would have to modify their calls each time a new stat param is added, but that is seemingly preferable for you than the current situation.

I feel pretty strongly that we want a way to identify the relation by
OID rather than by schema and name. I'm not sure whether attributes
should be identified by name or by number in this context. Apart from
those things, if passing strings and leaving it up to the called
function to interpret them makes most sense, that's OK with me.

I agree with you that getting rid of SPI is a much higher priority
than getting rid of the FunctionCallInfo stuff altogether. I could
live with DirectFunctionCall if we don't have a better option. I'm a
bit suspicious that this will lead to code bloat in postgres_fdw or
elsewhere, but maybe not.

---

From: Corey Huinker <[email-masked]>
Date: Tue, 16 Jun 2026 18:50:20 -0400

> OID rather than by schema and name.

Previous versions of pg_set_attribute_stats() were like that, but it went
away as the use-cases folded into pg_restore_attribute_stats(). So it's
do-able, but will make sharing code with the existing function harder in
the near term. We may have to settle for something that takes the oid, but
then plugs into the existing functions until such time as we can refactor
the existing callers.

> I'm not sure whether attributes

Identifying attributes by attnum is mostly for index expressions, so
attname would be the preference here.

> live with DirectFunctionCall if we don't have a better option.
>

Ok, good to know that I have that as a fallback position.

I think the right first step is to create the functions
import_relation_stats() and import_attribute_stats() which take the oid of
the destination relation and the rest of the parameters are const char
pointers, which is the ideal case for a bunch of values being fetched from
PQgetValue(). Those functions will for now just make the call to
attribute_statistics_update() / relation_statistics_update(), but at least
it will be localized inside relation_stats.c and attribute_stats.c where we
can make internal refactors without disturbing anyone else.

Now, we *could* make these 2 new functions take the oid/oid+attname
followed by a variadic array of keyname+value string pairs like the
existing pg_restore_*_stats() functions, so the functions call signature
would be somewhat future-proofed. Let me know if that violates your vision
of what this should be.

>  I'm a

postgres_fdw will get slimmer for not having SPI in it. relation_stats.c
and attribute_stats.c will have some temporary bloat until things can be
refactored.

---

From: Robert Haas <[email-masked]>
Date: Tue, 16 Jun 2026 18:57:04 -0400

On Tue, Jun 16, 2026 at 6:50 PM Corey Huinker <[email-masked]> wrote:
> postgres_fdw will get slimmer for not having SPI in it. relation_stats.c and attribute_stats.c will have some temporary bloat until things can be refactored.

Whatever ends up in core can (indeed must) be reused by every FDW. So
making things simpler for the FDW and more complex for core seems
likely to be the right tradeoff in general.

---

From: Etsuro Fujita <[email-masked]>
Date: Wed, 17 Jun 2026 19:23:07 +0900

Hi Corey,

On Wed, Jun 17, 2026 at 7:50 AM Corey Huinker <[email-masked]> wrote:
> I think the right first step is to create the functions import_relation_stats() and import_attribute_stats() which take the oid of the destination relation and the rest of the parameters are const char pointers, which is the ideal case for a bunch of values being fetched from PQgetValue(). Those functions will for now just make the call to attribute_statistics_update() / relation_statistics_update(), but at least it will be localized inside relation_stats.c and attribute_stats.c where we can make internal refactors without disturbing anyone else.

+1

> Now, we *could* make these 2 new functions take the oid/oid+attname followed by a variadic array of keyname+value string pairs like the existing pg_restore_*_stats() functions, so the functions call signature would be somewhat future-proofed. Let me know if that violates your vision of what this should be.

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.

Thanks!

Best regards,
Etsuro Fujita

---

From: Corey Huinker <[email-masked]>
Date: Wed, 17 Jun 2026 12:00:02 -0400

> are provided for FDW authors only, and 2) we don't make changes to the

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.

---

From: Corey Huinker <[email-masked]>
Date: Wed, 17 Jun 2026 12:03:33 -0400

> likely to be the right tradeoff in general.

I had assumed we wanted a generic C API to these two functions, but if we
want something that is specific to FDWs, that might change where the
functions land in the header files. It's an easy change to make if we
change our minds, but it would be good to know if we do or do not want
something specific to FDWs. Personally, I think FDWs will be 90-95% of the
usages outside of the existing SQL-defined functions, but 95% is not 100%,
so I'd be inclined to leave them in a statistics/stats utils header.

---

From: Robert Haas <[email-masked]>
Date: Wed, 17 Jun 2026 19:19:05 -0400

On Wed, Jun 17, 2026 at 12:03 PM Corey Huinker <[email-masked]> wrote:
> I had assumed we wanted a generic C API to these two functions, but if we want something that is specific to FDWs, that might change where the functions land in the header files. It's an easy change to make if we change our minds, but it would be good to know if we do or do not want something specific to FDWs. Personally, I think FDWs will be 90-95% of the usages outside of the existing SQL-defined functions, but 95% is not 100%, so I'd be inclined to leave them in a statistics/stats utils header.

I don't want it to be specific to FDWs, just convenient for FDWs.
Agree they should stay in a statistics header.

---

From: Etsuro Fujita <[email-masked]>
Date: Thu, 18 Jun 2026 19:42:56 +0900

On Thu, Jun 18, 2026 at 1:00 AM Corey Huinker <[email-masked]> wrote:

> 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 think that that's acceptable, considering that
heap_create_with_catalog() has 21 parameters.

Thanks!

Best regards,
Etsuro Fujita

---

From: Corey Huinker <[email-masked]>
Date: Thu, 18 Jun 2026 17:23:40 -0400

> heap_create_with_catalog() has 21 parameters.

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-masked]>
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-masked]>
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

---

From: Etsuro Fujita <[email-masked]>
Date: Fri, 19 Jun 2026 21:30:02 +0900

On Fri, Jun 19, 2026 at 6:23 AM Corey Huinker <[email-masked]> wrote:
> 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.

Ok, I'll review the patches.

Thanks!

Best regards,
Etsuro Fujita

---

From: Robert Haas <[email-masked]>
Date: Fri, 19 Jun 2026 09:45:18 -0400

> 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 r…

I think that's the right order of priority, but I don't think that
having import_attribute_statistics construct an fcinfo is great.
Ideally, attribute_statistics_update would call
import_attribute_statistics rather than the other way around.

---

From: Corey Huinker <[email-masked]>
Date: Sat, 20 Jun 2026 00:11:17 -0400

On Fri, Jun 19, 2026 at 9:45 AM Robert Haas <[email-masked]> wrote:

> > 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

Me either, I'm looking at "phase 2" already where
relation/attribute_statistics_update becomes a conventional function, and
pg_clear_attribute_stats and pg_restore_attribute_stats (and their relstats
equivalents) marshal their parameters to call that instead.

> import_attribute_statistics rather than the other way around.

I think we're mostly on the same page. If we require the caller to
understand the stats data structures to a greater level of detail, and
require it to do the transformations to the proper input types
(BlockNumbers, floats, float arrays, cstrings for anyarray, etc), then the
import_attribute_statistics functon and the new attribute_statistics_update
would be one-in-the-same. The v1 patch leaned heavily (perhaps too far)
towards letting the caller pass along string values fetched via
PQgetvalue() from a pg_stats without modification.

---

From: Corey Huinker <[email-masked]>
Date: Mon, 22 Jun 2026 15:08:33 -0400

On Sat, Jun 20, 2026 at 12:11 AM Corey Huinker <[email-masked]>
wrote:

>> > 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

An update on my progress on Phase 2:

I was able to convert relation_statistics_update() with relatively little
fuss, but ran into trouble in doing so for attribute_statistics_update().

Initially the plan was to have a data structure
RelationStatsData/AttributeStatsData which contained a series of boolean
has_FOO flags alongside FOO of the actual stat type (int32, float, float[],
or cstring for anyarray value) and have the respective functions convert
their parameters to this common structure before calling the common
function.

The first bit of dissonance comes from the SQL-level functions having
schemaname+relname parameters, and the reloid is resolved via
RangeVarGetRelidExtended() which has a callback to check for correct
permissions and setting the proper lock level. However, the C-caller would
either have the reloid already, or an already open Relation, but no
assurance that the caller has the correct permissions for that table or the
correct lock level on the table. So either we make an equivalent to
RangeVarGetRelidExtended() that takes an oid, or the C-caller has to derive
a RangeVar, call the existing RangeVarGetRelidExtended() function, and
verify the result reloid against the supplied parameter. I went with
deriving the RangeVar and putting an Assert on the before/after reloids,
but perhaps the smarter play is to make a function that checks for
ShareUpdateExclusiveLock on the Relation, and then does the equivalent of
RangeVarCallbackForStats().

A smaller bit of dissonance was with RecoveryInProgress(), which if I
recall we're checking before RangeVarGetRelidExtended() to avoid trying to
take a lock that will fail. That check might not be meaningful if the C
call takes a relation, thus ensuring that some level of locking worked,
thus we aren't in recovery.

Next is the existing validation functions stats_check_required_arg(),
stats_check_arg_array(), and stats_check_arg_pairs() all work on values
indexed by the positional functioncallinfo and the corresponding
relstatsinfo/attstatsinfo structure, and this makes a lot of type checking
and value checking compact and uniform.  If we want to keep this sort of
uniformity, the resulting StatsData structure will end up looking a lot
like the FunctionCallInfo that we already had.

Next is the fact that the end-destination for every value passed in is a
Datum for a pg_statistic heaptuple. Most Datum values are checked only for
their null-ness and if they're the correct type, so the value itself is
usually just passed directly from fcinfo into the heap tuple values[]
array. The float[] values are checked for number of elements and whether
any elements are NULL, but that is done via array functions that take a
Datum input. Only in a few cases do we actually look at the actual internal
value of the Datum (reltuples, attname, attnum, the anyarrays), so there's
little to gain there.

There's some additional hassle in the fact that
pg_restore_attribute_stats() can take an attnum parameter OR an attname
parameter, but not both. I was able to resolve that in a semi-elegant
fashion, but the other issues have convinced me that we're probably better
off continuing to use the FunctionCallInfo version of
attribute_stats_update(), though perhaps with a different name, allowing us
to use that name for the new API call instead of
import_attribute_statistics().

One thing we *do* need to change from my v1 patch is moving the recovery
check and the RangeVar check out of relation_statistics_update() and
attribute_statsistics_update(), and having the respective callers do those
checks themselves first, passing in the now-authoritative reloid from
stats_acquire_relation_lock().

To that end, here's a new and rebased patch set:

0001 - exactly the same as before
0002 - exactly the same as before
0003 - New stat_utils.c function stats_acquire_relation_lock() which covers
the RangeVar and Recovery checks common to all the functions that modify
relation or attribute stats.
0004 - Add a small regression test to relation stats
0005 - Use stats_acquire_relation_lock() in relstats functions, and rename
the publicly-facing relstats C function.
0006 - Rename the publicly-facing attstats functions.
0007 - Use stats_acquire_relation_lock() in attstats functions.
From efae549731c95c36c1b11d0668ef3397854439a9 Mon Sep 17 00:00:00 2001
From: Corey Huinker <[email-masked]>
Date: Wed, 17 Jun 2026 16:04:55 -0400
Subject: [PATCH v2 1/7] Remove SPI in favor of import_relation_statistics.

 
 	PG_RETURN_BOOL(result);
 }
+
+/*

 	if (ok)
 
 	if (ok)
 
+	char	   *relpages = NULL;

base-commit: 80bb0ebcc11fb9cb5904d490640408b3d7003a17
-- 
2.54.0

From c0be3905e22eafcfa09ed1f9644008897b210416 Mon Sep 17 00:00:00 2001
From: Corey Huinker <[email-masked]>
Date: Thu, 18 Jun 2026 16:28:10 -0400
Subject: [PATCH v2 2/7] Remove SPI in favor of import_attribute_statistics.

 
 	PG_RETURN_BOOL(result);
 }
+
+/*

+
+/*
 
 	char	   *relpages = NULL;
 	char	   *reltuples = NULL;

 

-			/*

 import_cleanup:
-	if (attimport_plan)
 
-/*

From 3f857611b69c95f325aba5ed8049e376326db8fd Mon Sep 17 00:00:00 2001
From: Corey Huinker <[email-masked]>
Date: Mon, 22 Jun 2026 13:43:37 -0400
Subject: [PATCH v2 3/7] New function: stats_acquire_relation_lock().

Introduce a new funciton, stats_acquire_relation_lock() which will
perform the RangeVarGetRelidExtended() call used to verify the correct
reloid of the relation, verify that the caller has the correct
permissions for the operation, and acquiring the lock.

Additionally, it will check RecoveryInProgress() before acquiring a lock
because we can't acquire locks during recovery, let alone set
statistics.

This function will be used by all of the callers of
relation_statistics_update() and attribute_statistics_update().
---
 src/include/statistics/stat_utils.h |  2 ++
 src/backend/statistics/stat_utils.c | 26 ++++++++++++++++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/src/include/statistics/stat_utils.h b/src/include/statistics/stat_utils.h
index 74da7790579..bcdfa616ce6 100644
--- a/src/include/statistics/stat_utils.h
+++ b/src/include/statistics/stat_utils.h
@@ -37,6 +37,8 @@ extern bool stats_check_arg_pair(FunctionCallInfo fcinfo,
 extern void RangeVarCallbackForStats(const RangeVar *relation,
 									 Oid relId, Oid oldRelId, void *arg);
 
+extern Oid stats_acquire_relation_lock(const char *nspname, const char *relname);
+
 extern bool stats_fill_fcinfo_from_arg_pairs(FunctionCallInfo pairs_fcinfo,
 											 FunctionCallInfo positional_fcinfo,
 											 struct StatsArgInfo *arginfo);
diff --git a/src/backend/statistics/stat_utils.c b/src/backend/statistics/stat_utils.c
index a673e3c704b..75792ff966a 100644
--- a/src/backend/statistics/stat_utils.c
+++ b/src/backend/statistics/stat_utils.c
@@ -26,6 +26,7 @@
 #include "catalog/pg_statistic.h"
 #include "funcapi.h"
 #include "miscadmin.h"
+#include "nodes/makefuncs.h"
 #include "nodes/nodeFuncs.h"
 #include "statistics/stat_utils.h"
 #include "storage/lmgr.h"
@@ -753,3 +754,28 @@ statatt_init_empty_tuple(Oid reloid, int16 attnum, bool inherited,
 		nulls[Anum_pg_statistic_stacoll1 + slotnum - 1] = false;
 	}
 }
+
+/*
+ * Convenience routine to encapsulate the RangeVarGetRelidExtended()
+ * that all external callers must make before calling
+ * relation_statistics_update() or attribute_statistics_update().
+ *
+ * Check for recovery in progress before acquiring any locks.
+ */
+Oid
+stats_acquire_relation_lock(const char *nspname, const char *relname)
+{
+	Oid		reloid;
+	Oid		locked_table = InvalidOid;
+
+	if (RecoveryInProgress())
+		ereport(ERROR,
+				(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+				 errmsg("recovery is in progress"),
+				 errhint("Statistics cannot be modified during recovery.")));
+
+	reloid = RangeVarGetRelidExtended(makeRangeVar((char *) nspname, (char *) relname, -1),
+									  ShareUpdateExclusiveLock, 0,
+									  RangeVarCallbackForStats, &locked_table);
+	return reloid;
+}
-- 
2.54.0

From 6cda28a063bdb854fbfb42dcdce2650730bdeea2 Mon Sep 17 00:00:00 2001
From: Corey Huinker <[email-masked]>
Date: Mon, 22 Jun 2026 14:05:32 -0400
Subject: [PATCH v2 5/7] Rename import_relation_statistics() to
 relation_statistics_update().

Rename existing relation_statistics_update() to update_relstats() and
change the call signature to have the relation oid. All callers now
independently resovle the reloid via stats_acquire_relation_lock().
---
 src/include/statistics/relation_stats.h |   2 +-
 src/backend/statistics/relation_stats.c | 194 +++++++++++++-----------
 contrib/postgres_fdw/postgres_fdw.c     |   2 +-
 3 files changed, 105 insertions(+), 93 deletions(-)

diff --git a/src/include/statistics/relation_stats.h b/src/include/statistics/relation_stats.h
index f68a1a755a1..423ff8961f2 100644
--- a/src/include/statistics/relation_stats.h
+++ b/src/include/statistics/relation_stats.h
@@ -14,7 +14,7 @@
 
 #include "access/genam.h"
 
-bool import_relation_statistics(Relation rel, const char *relpages,
+bool relation_statistics_update(Relation rel, const char *relpages,
 								const char *reltuples, const char *relallvisible,
 								const char *relallfrozen);
 
diff --git a/src/backend/statistics/relation_stats.c b/src/backend/statistics/relation_stats.c
index 9c308ee50e5..2e07df78d8b 100644
--- a/src/backend/statistics/relation_stats.c
+++ b/src/backend/statistics/relation_stats.c
@@ -32,8 +32,8 @@

 /*
- * Positional argument numbers, names, and types for
- * relation_statistics_update().
+ * Positional argument numbers, names, and types for positional_fcinfo
+ * used by update_relstats().
  */
 
 enum relation_stats_argnum
@@ -58,18 +58,15 @@ static struct StatsArgInfo relarginfo[] =
 	[NUM_RELATION_STATS_ARGS] = {0}
 };
 
-static bool relation_statistics_update(FunctionCallInfo fcinfo);
+static bool update_relstats(Oid reloid, FunctionCallInfo fcinfo);
 
 /*
  * Internal function for modifying statistics for a relation.
  */
 static bool
-relation_statistics_update(FunctionCallInfo fcinfo)
+update_relstats(Oid reloid, FunctionCallInfo fcinfo)
 {
 	bool		result = true;
-	char	   *nspname;
-	char	   *relname;
-	Oid			reloid;
 	Relation	crel;
 	BlockNumber relpages = 0;
 	bool		update_relpages = false;
@@ -85,23 +82,6 @@ relation_statistics_update(FunctionCallInfo fcinfo)
 	Datum		values[4] = {0};
 	bool		nulls[4] = {0};
 	int			nreplaces = 0;
-	Oid			locked_table = InvalidOid;
-
-	stats_check_required_arg(fcinfo, relarginfo, RELSCHEMA_ARG);
-	stats_check_required_arg(fcinfo, relarginfo, RELNAME_ARG);
-
-	nspname = TextDatumGetCString(PG_GETARG_DATUM(RELSCHEMA_ARG));
-	relname = TextDatumGetCString(PG_GETARG_DATUM(RELNAME_ARG));
-
-	if (RecoveryInProgress())
-		ereport(ERROR,
-				(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
-				 errmsg("recovery is in progress"),
-				 errhint("Statistics cannot be modified during recovery.")));
-
-	reloid = RangeVarGetRelidExtended(makeRangeVar(nspname, relname, -1),
-									  ShareUpdateExclusiveLock, 0,
-									  RangeVarCallbackForStats, &locked_table);
 
 	if (!PG_ARGISNULL(RELPAGES_ARG))
 	{
@@ -203,24 +183,35 @@ relation_statistics_update(FunctionCallInfo fcinfo)
 Datum
 pg_clear_relation_stats(PG_FUNCTION_ARGS)
 {
-	LOCAL_FCINFO(newfcinfo, 6);
+	LOCAL_FCINFO(positional_fcinfo, NUM_RELATION_STATS_ARGS);
+	Oid			reloid;
+	char	   *nspname;
+	char	   *relname;
 
-	InitFunctionCallInfoData(*newfcinfo, NULL, 6, InvalidOid, NULL, NULL);
+	InitFunctionCallInfoData(*positional_fcinfo, NULL, 6, InvalidOid, NULL, NULL);
 
-	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 = UInt32GetDatum(0);
-	newfcinfo->args[2].isnull = false;
-	newfcinfo->args[3].value = Float4GetDatum(-1.0);
-	newfcinfo->args[3].isnull = false;
-	newfcinfo->args[4].value = UInt32GetDatum(0);
-	newfcinfo->args[4].isnull = false;
-	newfcinfo->args[5].value = UInt32GetDatum(0);
-	newfcinfo->args[5].isnull = false;
+	positional_fcinfo->args[RELSCHEMA_ARG].value = PG_GETARG_DATUM(0);
+	positional_fcinfo->args[RELSCHEMA_ARG].isnull = PG_ARGISNULL(0);
+	positional_fcinfo->args[RELNAME_ARG].value = PG_GETARG_DATUM(1);
+	positional_fcinfo->args[RELNAME_ARG].isnull = PG_ARGISNULL(1);
+	positional_fcinfo->args[RELPAGES_ARG].value = UInt32GetDatum(0);
+	positional_fcinfo->args[RELPAGES_ARG].isnull = false;
+	positional_fcinfo->args[RELTUPLES_ARG].value = Float4GetDatum(-1.0);
+	positional_fcinfo->args[RELTUPLES_ARG].isnull = false;
+	positional_fcinfo->args[RELALLVISIBLE_ARG].value = UInt32GetDatum(0);
+	positional_fcinfo->args[RELALLVISIBLE_ARG].isnull = false;
+	positional_fcinfo->args[RELALLFROZEN_ARG].value = UInt32GetDatum(0);
+	positional_fcinfo->args[RELALLFROZEN_ARG].isnull = false;
 
-	relation_statistics_update(newfcinfo);
+	stats_check_required_arg(positional_fcinfo, relarginfo, RELSCHEMA_ARG);
+	stats_check_required_arg(positional_fcinfo, relarginfo, RELNAME_ARG);
+
+	nspname = TextDatumGetCString(positional_fcinfo->args[RELSCHEMA_ARG].value);
+	relname = TextDatumGetCString(positional_fcinfo->args[RELNAME_ARG].value);
+
+	reloid = stats_acquire_relation_lock(nspname, relname);
+
+	update_relstats(reloid, positional_fcinfo);
 	PG_RETURN_VOID();
 }
 
@@ -229,6 +220,9 @@ pg_restore_relation_stats(PG_FUNCTION_ARGS)
 {
 	LOCAL_FCINFO(positional_fcinfo, NUM_RELATION_STATS_ARGS);
 	bool		result = true;
+	Oid			reloid;
+	char	   *nspname;
+	char	   *relname;
 
 	InitFunctionCallInfoData(*positional_fcinfo, NULL,
 							 NUM_RELATION_STATS_ARGS,
@@ -238,7 +232,15 @@ pg_restore_relation_stats(PG_FUNCTION_ARGS)
 										  relarginfo))
 		result = false;
 
-	if (!relation_statistics_update(positional_fcinfo))
+	stats_check_required_arg(positional_fcinfo, relarginfo, RELSCHEMA_ARG);
+	stats_check_required_arg(positional_fcinfo, relarginfo, RELNAME_ARG);
+
+	nspname = TextDatumGetCString(positional_fcinfo->args[RELSCHEMA_ARG].value);
+	relname = TextDatumGetCString(positional_fcinfo->args[RELNAME_ARG].value);
+
+	reloid = stats_acquire_relation_lock(nspname, relname);
+
+	if (!update_relstats(reloid, positional_fcinfo))
 		result = false;
 
 	PG_RETURN_BOOL(result);
@@ -248,92 +250,102 @@ pg_restore_relation_stats(PG_FUNCTION_ARGS)
  * Convenience routine to parse BlockNumber values, and emit a warning
  * on parse errors.
  *
- * Returns 0 if the value is NULL or invalid.
+ * Returns Datum of 0 if the value is NULL or invalid.
  */
-static BlockNumber
+static Datum
 str_to_blocknumber(const char *s)
 {
-	const BlockNumber default_value = 0;
+	BlockNumber result = 0;
 
-	BlockNumber result;
-	ErrorSaveContext escontext = {T_ErrorSaveContext};
-
-	if (!s)
-		return default_value;
-
-	result = uint32in_subr(s, NULL, "BlockNumber", (Node *) &escontext);
-
-	if (escontext.error_occurred)
+	if (s)
 	{
-		escontext.error_data->elevel = WARNING;
-		ThrowErrorData(escontext.error_data);
-		FreeErrorData(escontext.error_data);
+		BlockNumber scratch_result;
+		ErrorSaveContext escontext = {T_ErrorSaveContext};
 
-		return default_value;
+		scratch_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);
+		}
+		else
+			result = scratch_result;
 	}
 
-	return result;
+	return UInt32GetDatum(result);
 }
 
 /*
  * Convenience routine to parse float values, and emit a warning on parse
  * errors.
  *
- * Returns -1.0 if the value is NULL or invalid.
+ * Returns Datum of -1.0 if the value is NULL or invalid.
  */
-static float
+static Datum
 str_to_float(const char *s)
 {
-	const float default_value = -1.0;
+	float result = -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)
+	if (s)
 	{
-		escontext.error_data->elevel = WARNING;
-		ThrowErrorData(escontext.error_data);
-		FreeErrorData(escontext.error_data);
-		return default_value;
+		ErrorSaveContext escontext = {T_ErrorSaveContext};
+
+		float	scratch_result;
+
+		scratch_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);
+		}
+		else
+			result = scratch_result;
 	}
 
-	return result;
+	return Float4GetDatum(result);
 }
 
 /*
  * Import relation statistics from regular string inputs.
  */
 bool
-import_relation_statistics(Relation rel, const char *relpages,
+relation_statistics_update(Relation rel, const char *relpages,
 						   const char *reltuples, const char *relallvisible,
 						   const char *relallfrozen)
 {
-	LOCAL_FCINFO(newfcinfo, NUM_RELATION_STATS_ARGS);
+	LOCAL_FCINFO(positional_fcinfo, NUM_RELATION_STATS_ARGS);
+	Oid			reloid;
+	char	   *nspname;
+	char	   *relname;
 
-	InitFunctionCallInfoData(*newfcinfo, NULL, NUM_RELATION_STATS_ARGS, InvalidOid, NULL, NULL);
+	InitFunctionCallInfoData(*positional_fcinfo, NULL, NUM_RELATION_STATS_ARGS,
+							 InvalidOid, NULL, NULL);
+
+	nspname = get_namespace_name(RelationGetNamespace(rel));
+	relname = RelationGetRelationName(rel);
+
+	reloid = stats_acquire_relation_lock(nspname, relname);
 
 	/*
 	 * 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;
+	positional_fcinfo->args[RELSCHEMA_ARG].value = CStringGetTextDatum(nspname);
+	positional_fcinfo->args[RELSCHEMA_ARG].isnull = false;
+	positional_fcinfo->args[RELNAME_ARG].value = CStringGetTextDatum(relname);
+	positional_fcinfo->args[RELNAME_ARG].isnull = false;
+	positional_fcinfo->args[RELPAGES_ARG].value = str_to_blocknumber(relpages);
+	positional_fcinfo->args[RELPAGES_ARG].isnull = false;
+	positional_fcinfo->args[RELTUPLES_ARG].value = str_to_float(reltuples);
+	positional_fcinfo->args[RELTUPLES_ARG].isnull = false;
+	positional_fcinfo->args[RELALLVISIBLE_ARG].value = str_to_blocknumber(relallvisible);
+	positional_fcinfo->args[RELALLVISIBLE_ARG].isnull = false;
+	positional_fcinfo->args[RELALLFROZEN_ARG].value = str_to_blocknumber(relallfrozen);
+	positional_fcinfo->args[RELALLFROZEN_ARG].isnull = false;
 
-	return relation_statistics_update(newfcinfo);
+	return update_relstats(reloid, positional_fcinfo);
 }
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index 8aa589d5a56..1d203f5fcf5 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -6086,7 +6086,7 @@ import_fetched_statistics(Relation relation,
 	if (!PQgetisnull(remstats->rel, 0, RELSTATS_RELTUPLES))
 		reltuples = PQgetvalue(remstats->rel, 0, RELSTATS_RELTUPLES);
 
-	if (!import_relation_statistics(relation, relpages, reltuples,
+	if (!relation_statistics_update(relation, relpages, reltuples,
 									NULL, NULL))
 	{
 		ereport(WARNING,
-- 
2.54.0

From 3eec089f361d42b24c8e9c2c3bfc92f9006b8e15 Mon Sep 17 00:00:00 2001
From: Corey Huinker <[email-masked]>
Date: Mon, 22 Jun 2026 13:48:44 -0400
Subject: [PATCH v2 4/7] postgres_fdw: additional regression reverse
 set-difference test

Add an addtional test to postgres_fdw.sql which selects the reverse
set-difference operation, allowing us to see the values in the row that
should have been set to certain values but somehow wasn't. The current
test would only show the values of the source table but not the value
found in the destination table.
---
 contrib/postgres_fdw/expected/postgres_fdw.out | 11 +++++++++++
 contrib/postgres_fdw/sql/postgres_fdw.sql      |  8 ++++++++
 2 files changed, 19 insertions(+)

diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index e931fa64e0d..b46f38a04fe 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -12950,6 +12950,17 @@ WHERE oid = 'public.dtest_ftable'::regclass;
 ----------+-----------
 (0 rows)
 
+SELECT relpages, reltuples
+FROM pg_class
+WHERE oid = 'public.dtest_ftable'::regclass
+EXCEPT
+SELECT relpages, reltuples
+FROM pg_class
+WHERE oid = 'public.dtest_table'::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,
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
index 627177123b3..5a4f05c89e5 100644
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -4601,6 +4601,14 @@ SELECT relpages, reltuples
 FROM pg_class
 WHERE oid = 'public.dtest_ftable'::regclass;
 
+SELECT relpages, reltuples
+FROM pg_class
+WHERE oid = 'public.dtest_ftable'::regclass
+EXCEPT
+SELECT relpages, reltuples
+FROM pg_class
+WHERE oid = 'public.dtest_table'::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,
-- 
2.54.0

From c95cc2d44e98847695aba94ea04a44906ebf5d51 Mon Sep 17 00:00:00 2001
From: Corey Huinker <[email-masked]>
Date: Mon, 22 Jun 2026 14:16:31 -0400
Subject: [PATCH v2 6/7] Rename attribute_statistics functions.

The static attribute_statistics_update() is now update_attstats(). The
function import_attribute_stats() is now attribute_statisticss_update()
and delete_attribute_statistics() is now attribute_statistics_delete().

Changes to function signatures will happen in a later patch.
---
 src/include/statistics/attribute_stats.h |  4 ++--
 src/backend/statistics/attribute_stats.c | 14 +++++++-------
 contrib/postgres_fdw/postgres_fdw.c      |  4 ++--
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/src/include/statistics/attribute_stats.h b/src/include/statistics/attribute_stats.h
index 4ebde0c7c3b..8a4ee5112e4 100644
--- a/src/include/statistics/attribute_stats.h
+++ b/src/include/statistics/attribute_stats.h
@@ -15,9 +15,9 @@
 #include "access/genam.h"

-bool delete_attribute_statistics(Relation rel, AttrNumber attnum, bool inherited);
+bool attribute_statistics_delete(Relation rel, AttrNumber attnum, bool inherited);
 
-bool import_attribute_statistics(Relation rel, const char *attname,
+bool attribute_statistics_update(Relation rel, const char *attname,
 								 AttrNumber attnum, bool inherited,
 								 const char *null_frac, const char *avg_width,
 								 const char *n_distinct,
diff --git a/src/backend/statistics/attribute_stats.c b/src/backend/statistics/attribute_stats.c
index f67d62c0104..7dccce83c65 100644
--- a/src/backend/statistics/attribute_stats.c
+++ b/src/backend/statistics/attribute_stats.c
@@ -34,7 +34,7 @@
 
 /*
  * Positional argument numbers, names, and types for
- * attribute_statistics_update() and pg_restore_attribute_stats().
+ * update_attstats() and pg_restore_attribute_stats().
  */
 
 enum attribute_stats_argnum
@@ -106,7 +106,7 @@ static struct StatsArgInfo cleararginfo[] =
 	[C_NUM_ATTRIBUTE_STATS_ARGS] = {0}
 };
 
-static bool attribute_statistics_update(FunctionCallInfo fcinfo);
+static bool update_attstats(FunctionCallInfo fcinfo);
 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);
@@ -128,7 +128,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)
+update_attstats(FunctionCallInfo fcinfo)
 {
 	char	   *nspname;
 	char	   *relname;
@@ -685,7 +685,7 @@ pg_restore_attribute_stats(PG_FUNCTION_ARGS)
 										  attarginfo))
 		result = false;
 
-	if (!attribute_statistics_update(positional_fcinfo))
+	if (!update_attstats(positional_fcinfo))
 		result = false;
 
 	PG_RETURN_BOOL(result);
@@ -808,7 +808,7 @@ set_floatarr_arg(NullableDatum *arg, const char *s)
  * Delete attribute statistics.
  */
 bool
-delete_attribute_statistics(Relation rel, AttrNumber attnum, bool inherited)
+attribute_statistics_delete(Relation rel, AttrNumber attnum, bool inherited)
 {
 	return delete_pg_statistic(RelationGetRelid(rel), attnum, inherited);
 }
@@ -820,7 +820,7 @@ delete_attribute_statistics(Relation rel, AttrNumber attnum, bool inherited)
  * To convey a "null"/not-set value for attnum, use InvalidAttrNumber.
  */
 bool
-import_attribute_statistics(Relation rel, const char *attname,
+attribute_statistics_update(Relation rel, const char *attname,
 							AttrNumber attnum, bool inherited,
 							const char *null_frac, const char *avg_width,
 							const char *n_distinct,
@@ -878,5 +878,5 @@ import_attribute_statistics(Relation rel, const char *attname,
 	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);
+	return update_attstats(newfcinfo);
 }
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index 1d203f5fcf5..9680af902ca 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -6048,9 +6048,9 @@ import_fetched_statistics(Relation relation,
 			 */
 			CHECK_FOR_INTERRUPTS();
 
-			delete_attribute_statistics(relation, attnum, false);
+			attribute_statistics_delete(relation, attnum, false);
 
-			if (!import_attribute_statistics(relation, attname,
+			if (!attribute_statistics_update(relation, attname,
 											 InvalidAttrNumber,
 											 false,
 											 get_opt_value(res, row, ATTSTATS_NULL_FRAC),
-- 
2.54.0

From 79564c6211d7dd8f80062d9acbcea20969698803 Mon Sep 17 00:00:00 2001
From: Corey Huinker <[email-masked]>
Date: Mon, 22 Jun 2026 14:48:12 -0400
Subject: [PATCH v2 7/7] Make callers of update_attstats() use
 stats_acquire_relation_lock().

This is the same change that was made in relation_stats.c, but with the
function renaming already done beforehand.
---
 src/backend/statistics/attribute_stats.c | 109 +++++++++++------------
 1 file changed, 51 insertions(+), 58 deletions(-)

index 7dccce83c65..f903d2c0003 100644
@@ -33,8 +33,7 @@
 
 /*
- * Positional argument numbers, names, and types for
- * update_attstats() and pg_restore_attribute_stats().
+ * Positional argument numbers, names, and types for update_attstats()
  */
 
 enum attribute_stats_argnum
@@ -106,7 +105,7 @@ static struct StatsArgInfo cleararginfo[] =
 	[C_NUM_ATTRIBUTE_STATS_ARGS] = {0}
 };
 
-static bool update_attstats(FunctionCallInfo fcinfo);
+static bool update_attstats(Oid reloid, FunctionCallInfo fcinfo);
 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);
@@ -128,15 +127,12 @@ static bool delete_pg_statistic(Oid reloid, AttrNumber attnum, bool stainherit);
  * and other statistic kinds may still be updated.
  */
 static bool
-update_attstats(FunctionCallInfo fcinfo)
+update_attstats(Oid reloid, FunctionCallInfo fcinfo)
 {
-	char	   *nspname;
 	char	   *relname;
-	Oid			reloid;
 	char	   *attname;
 	AttrNumber	attnum;
 	bool		inherited;
-	Oid			locked_table = InvalidOid;
 
 	Relation	starel;
 	HeapTuple	statup;
@@ -173,20 +169,8 @@ update_attstats(FunctionCallInfo fcinfo)
 	stats_check_required_arg(fcinfo, attarginfo, ATTRELSCHEMA_ARG);
 	stats_check_required_arg(fcinfo, attarginfo, ATTRELNAME_ARG);
 
-	nspname = TextDatumGetCString(PG_GETARG_DATUM(ATTRELSCHEMA_ARG));
 	relname = TextDatumGetCString(PG_GETARG_DATUM(ATTRELNAME_ARG));
 
-	if (RecoveryInProgress())
-		ereport(ERROR,
-				(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
-				 errmsg("recovery is in progress"),
-				 errhint("Statistics cannot be modified during recovery.")));
-
-	/* lock before looking up attribute */
-	reloid = RangeVarGetRelidExtended(makeRangeVar(nspname, relname, -1),
-									  ShareUpdateExclusiveLock, 0,
-									  RangeVarCallbackForStats, &locked_table);
-
 	/* user can specify either attname or attnum, but not both */
 	if (!PG_ARGISNULL(ATTNAME_ARG))
 	{
@@ -605,7 +589,6 @@ pg_clear_attribute_stats(PG_FUNCTION_ARGS)
 	char	   *attname;
 	AttrNumber	attnum;
 	bool		inherited;
-	Oid			locked_table = InvalidOid;
 
 	stats_check_required_arg(fcinfo, cleararginfo, C_ATTRELSCHEMA_ARG);
 	stats_check_required_arg(fcinfo, cleararginfo, C_ATTRELNAME_ARG);
@@ -614,16 +597,7 @@ pg_clear_attribute_stats(PG_FUNCTION_ARGS)
 
 	nspname = TextDatumGetCString(PG_GETARG_DATUM(C_ATTRELSCHEMA_ARG));
 	relname = TextDatumGetCString(PG_GETARG_DATUM(C_ATTRELNAME_ARG));
-
-	if (RecoveryInProgress())
-		ereport(ERROR,
-				(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
-				 errmsg("recovery is in progress"),
-				 errhint("Statistics cannot be modified during recovery.")));
-
-	reloid = RangeVarGetRelidExtended(makeRangeVar(nspname, relname, -1),
-									  ShareUpdateExclusiveLock, 0,
-									  RangeVarCallbackForStats, &locked_table);
+	reloid = stats_acquire_relation_lock(nspname, relname);
 
 	attname = TextDatumGetCString(PG_GETARG_DATUM(C_ATTNAME_ARG));
 	attnum = get_attnum(reloid, attname);
@@ -677,6 +651,9 @@ pg_restore_attribute_stats(PG_FUNCTION_ARGS)
 {
 	LOCAL_FCINFO(positional_fcinfo, NUM_ATTRIBUTE_STATS_ARGS);
 	bool		result = true;
+	Oid			reloid;
+	char	   *nspname;
+	char	   *relname;
 
 	InitFunctionCallInfoData(*positional_fcinfo, NULL, NUM_ATTRIBUTE_STATS_ARGS,
 							 InvalidOid, NULL, NULL);
@@ -685,7 +662,15 @@ pg_restore_attribute_stats(PG_FUNCTION_ARGS)
 										  attarginfo))
 		result = false;
 
-	if (!update_attstats(positional_fcinfo))
+	stats_check_required_arg(positional_fcinfo, attarginfo, ATTRELSCHEMA_ARG);
+	stats_check_required_arg(positional_fcinfo, attarginfo, ATTRELNAME_ARG);
+
+	nspname = TextDatumGetCString(positional_fcinfo->args[ATTRELSCHEMA_ARG].value);
+	relname = TextDatumGetCString(positional_fcinfo->args[ATTRELNAME_ARG].value);
+
+	reloid = stats_acquire_relation_lock(nspname, relname);
+
+	if (!update_attstats(reloid, positional_fcinfo))
 		result = false;
 
 	PG_RETURN_BOOL(result);
@@ -835,48 +820,56 @@ attribute_statistics_update(Relation rel, const char *attname,
 							const char *range_empty_frac,
 							const char *range_bounds_histogram)
 {
-	LOCAL_FCINFO(newfcinfo, NUM_ATTRIBUTE_STATS_ARGS);
+	LOCAL_FCINFO(positional_fcinfo, NUM_ATTRIBUTE_STATS_ARGS);
+	Oid			reloid;
+	char	   *nspname;
+	char	   *relname;
 
-	InitFunctionCallInfoData(*newfcinfo, NULL, NUM_ATTRIBUTE_STATS_ARGS, InvalidOid, NULL, NULL);
+	InitFunctionCallInfoData(*positional_fcinfo, NULL, NUM_ATTRIBUTE_STATS_ARGS, InvalidOid, NULL, NULL);
+
+	nspname = get_namespace_name(RelationGetNamespace(rel));
+	relname = RelationGetRelationName(rel);
+
+	reloid = stats_acquire_relation_lock(nspname, relname);
 
 	/*
 	 * 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;
+	positional_fcinfo->args[ATTRELSCHEMA_ARG].value = CStringGetTextDatum(nspname);
+	positional_fcinfo->args[ATTRELSCHEMA_ARG].isnull = false;
+	positional_fcinfo->args[ATTRELNAME_ARG].value = CStringGetTextDatum(relname);
+	positional_fcinfo->args[ATTRELNAME_ARG].isnull = false;
 
-	set_text_arg(&newfcinfo->args[ATTNAME_ARG], attname);
+	set_text_arg(&positional_fcinfo->args[ATTNAME_ARG], attname);
 
 	if (attnum != InvalidAttrNumber)
 	{
-		newfcinfo->args[ATTNUM_ARG].value = Int16GetDatum(attnum);
-		newfcinfo->args[ATTNUM_ARG].isnull = false;
+		positional_fcinfo->args[ATTNUM_ARG].value = Int16GetDatum(attnum);
+		positional_fcinfo->args[ATTNUM_ARG].isnull = false;
 	}
 	else
 	{
-		newfcinfo->args[ATTNUM_ARG].value = (Datum) 0;
-		newfcinfo->args[ATTNUM_ARG].isnull = true;
+		positional_fcinfo->args[ATTNUM_ARG].value = (Datum) 0;
+		positional_fcinfo->args[ATTNUM_ARG].isnull = true;
 	}
 
-	newfcinfo->args[INHERITED_ARG].value = BoolGetDatum(inherited);
-	newfcinfo->args[INHERITED_ARG].isnull = false;
+	positional_fcinfo->args[INHERITED_ARG].value = BoolGetDatum(inherited);
+	positional_fcinfo->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);
+	set_float_arg(&positional_fcinfo->args[NULL_FRAC_ARG], null_frac);
+	set_int32_arg(&positional_fcinfo->args[AVG_WIDTH_ARG], avg_width);
+	set_float_arg(&positional_fcinfo->args[N_DISTINCT_ARG], n_distinct);
+	set_text_arg(&positional_fcinfo->args[MOST_COMMON_VALS_ARG], most_common_vals);
+	set_floatarr_arg(&positional_fcinfo->args[MOST_COMMON_FREQS_ARG], most_common_freqs);
+	set_text_arg(&positional_fcinfo->args[HISTOGRAM_BOUNDS_ARG], histogram_bounds);
+	set_float_arg(&positional_fcinfo->args[CORRELATION_ARG], correlation);
+	set_text_arg(&positional_fcinfo->args[MOST_COMMON_ELEMS_ARG], most_common_elems);
+	set_floatarr_arg(&positional_fcinfo->args[MOST_COMMON_ELEM_FREQS_ARG], most_common_elem_freqs);
+	set_floatarr_arg(&positional_fcinfo->args[ELEM_COUNT_HISTOGRAM_ARG], elem_count_histogram);
+	set_text_arg(&positional_fcinfo->args[RANGE_LENGTH_HISTOGRAM_ARG], range_length_histogram);
+	set_float_arg(&positional_fcinfo->args[RANGE_EMPTY_FRAC_ARG], range_empty_frac);
+	set_text_arg(&positional_fcinfo->args[RANGE_BOUNDS_HISTOGRAM_ARG], range_bounds_histogram);
 
-	return update_attstats(newfcinfo);
+	return update_attstats(reloid, positional_fcinfo);
 }
-- 
2.54.0

---

From: jian he <[email-masked]>
Date: Tue, 23 Jun 2026 19:43:14 +0800

+/*

Just a quick thought: the above can be replaced by InputFunctionCallSafe?

--
jian
https://www.enterprisedb.com/

---

From: Robert Haas <[email-masked]>
Date: Tue, 23 Jun 2026 08:10:52 -0400

On Mon, Jun 22, 2026 at 3:08 PM Corey Huinker <[email-masked]> wrote:
> The first bit of dissonance comes from the SQL-level functions having schemaname+relname parameters, and the reloid is resolved via RangeVarGetRelidExtended() which has a callback to check for correct permissions and setting the proper lock level. However, the C-caller would either have the reloid already, or an already open Relation, but no assurance that the caller has the correct permissions for that table or the correct lock level on the table. So either we make an equivalent to RangeVarGetRelidExtended() that takes an oid, or the C-caller has to derive a RangeVar, call the existing RangeVarGetRelidExtended() function, and verify the result reloid against the supplied parameter. I went with deriving the RangeVar and putting an Assert on the before/after reloids, but perhaps the smarter play is to make a function that checks for ShareUpdateExclusiveLock on the Relation, and then does the equivalent of RangeVarCallbackForStats().

I don't think this is really a problem. If the caller is specifying
the OID, they should have called RangeVarGetRelidExtended themselves.
Permissions-checking, locking, and opening the relation should all
happen simultaneously, and the logic shouldn't be duplicated later.

> A smaller bit of dissonance was with RecoveryInProgress(), which if I recall we're checking before RangeVarGetRelidExtended() to avoid trying to take a lock that will fail. That check might not be meaningful if the C call takes a relation, thus ensuring that some level of locking worked, thus we aren't in recovery.

I don't quite follow this part.

> Next is the existing validation functions stats_check_required_arg(), stats_check_arg_array(), and stats_check_arg_pairs() all work on values indexed by the positional functioncallinfo and the corresponding relstatsinfo/attstatsinfo structure, and this makes a lot of type checking and value checking compact and uniform.  If we want to keep this sort of uniformity, the resulting StatsData structure will end up looking a lot like the FunctionCallInfo that we already had.

Yeah, this is worth thinking about. You could consider putting an
array inside the struct and use #defines for the indexes. And instead
of having a separate Boolean for each index, you could use the same
index to reference the N'th bit of a single integer flag variable.

> Next is the fact that the end-destination for every value passed in is a Datum for a pg_statistic heaptuple. Most Datum values are checked only for their null-ness and if they're the correct type, so the value itself is usually just passed directly from fcinfo into the heap tuple values[] array. The float[] values are checked for number of elements and whether any elements are NULL, but that is done via array functions that take a Datum input. Only in a few cases do we actually look at the actual internal value of the Datum (reltuples, attname, attnum, the anyarrays), so there's little to gain there.

Right, so the question is whether it makes more sense to pass down C
strings or Datums.

> There's some additional hassle in the fact that pg_restore_attribute_stats() can take an attnum parameter OR an attname parameter, but not both. I was able to resolve that in a semi-elegant fashion, but the other issues have convinced me that we're probably better off continuing to use the FunctionCallInfo version of attribute_stats_update(), though perhaps with a different name, allowing us to use that name for the new API call instead of import_attribute_statistics().

On that particular point, I think I'm still unconvinced, but I also
haven't looked deeply into this just yet, so maybe I'm wrong.

---

From: Corey Huinker <[email-masked]>
Date: Tue, 23 Jun 2026 11:32:48 -0400

> Just a quick thought: the above can be replaced by InputFunctionCallSafe?

Potentially, yes. The functions were originally returning the actual data
types rather than the Datum serializations. Which way those functions go
depends a lot on the data structure they end up populating, which is
currently in flux.

---

From: Corey Huinker <[email-masked]>
Date: Tue, 23 Jun 2026 11:53:22 -0400

> happen simultaneously, and the logic shouldn't be duplicated later.

It seems dangerous to me to provide an externally visible function that
assumes the caller has already self-verified that they have permission to
modify stats for an object.

It's possible that the existing analyze() code already has verified these
permissions, but at this moment I'm unsure.

> I don't quite follow this part.

I mean that I don't know if that check is required in situations where the
caller already has an open relation with the right lock, and if we can
avoid the call by having the API take a Relation rather than an Oid, then
that would be preferable.

> index to reference the N'th bit of a single integer flag variable.

That's basically what I was trying, though I wasn't brave enough to use
bitflags in a v1 patch.

The #defines already exist now in the form of enums that act as indexes
into the FunctionCallInfo array. But if my data structure is an array of
Datums with an array of is-not-nulls, then I'm just not that far away from
the FunctionCallInfo structure we already have.

> Right, so the question is whether it makes more sense to pass down C

Requiring Datums simplifies things greatly inside the existing functions,
but pushes that complexity to the caller. My first proposal was to keep
complexity to an absolute minimum on the caller side, but your comments
make me think there's considerable tolerance for more complexity on the
caller side.

> haven't looked deeply into this just yet, so maybe I'm wrong.

The issue becomes moot if we require the caller to construct their own
isnull and values arrays as was suggested above.

---

From: Corey Huinker <[email-masked]>
Date: Mon, 29 Jun 2026 03:14:11 -0500

> make me think there's considerable tolerance for more complexity on the

I've had some time to look this over, and it's pretty clear that we don't
actually need the whole positional FunctionCallInfo, we just need the
fcinfo->args of it. We also need the ability to pass statistical values
along with the values that comprise the key of the
relation/attribute/object to be modified.

There are important differences in the parameters needed by the different
types of functions. The pg_restore_*() SQL function calls need to identify
the schema+relname of the relation being modified, and already have all of
the values as typed Datums, whereas the internal API calls already have an
identified and locked open Relation, but all of the statistical parameters
are just cstrings.

The solution I chose was to create common functions that take a relation
object and an array of just the statistical parameters. This requires the
pg_restore_* functions to resolve and lock the relation themselves, and
then carry forward just the subset of parameters that are statistics (right
types, but wrong number of arguments). Conversely, the internal API
functions need to map their array of cstring values to the correctly typed
Datums (wrong types, but right number of arguments in the right order).

Attached is v2. The patches are broken down into very small bites to aid
digestion and to confirm that tests pass after each comparatively minor
change.

0001-0003: standardize enum values to further indicate which enum the
values belong to. This will be valuable later when values from an array of
one length have to be mapped to values in an array of a different
length. One patch each for relation stats, attribute stats, extended stats.

0004: Refactors the stats_check_* functions that previously took an fcinfo,
but now just take a NullableDatum array.

0005-0007: Change the function signature of
(relation|attribute|extended)_statistics_update() to no longer take
FunctionCallInfo as a parameter and instead take a NullableDatum[].

0008: Changes stats_fill_fcinfo_from_args() which maps a keyword args
fcinfo to a positional fcinfo to map directly to a NullableDatum[], and
changes all callers to use the new function and signature.

0009: rename relation_statistics_update -> update_relstats and
relation_stats_argnum -> relation_args_argnum to make room for
public-facing objects in the following patch.
0010: Introduce new public facing relation_statistics_update, which takes
isnull/string arrays that will be filled out by the caller, currently
postgres_fdw, and calls update_relstats same as pg_restore_relation_stats().

0011: same as 0009, but for attribute stats
0012: same as 0010, but for attribute stats

0013: remove SPI calls from postgres_fdw, and use the newly public
relation_statistics_update() and attribute_statistics_update() functions
instead.

---

From: Etsuro Fujita <[email-masked]>
Date: Mon, 29 Jun 2026 20:50:45 +0900

On Mon, Jun 29, 2026 at 5:14 PM Corey Huinker <[email-masked]> wrote:

>> Requiring Datums simplifies things greatly inside the existing functions, but pushes that complexity to the caller. My first proposal was to keep complexity to an absolute minimum on the caller side, but your comments make me think there's considerable tolerance for more complexity on the caller side.
>
>
> I've had some time to look this over, and it's pretty clear that we don't actually need the whole positional FunctionCallInfo, we just need the fcinfo->args of it. We also need the ability to pass statistical values along with the values that comprise the key of the relation/attribute/object to be modified.
>
> There are important differences in the parameters needed by the different types of functions. The pg_restore_*() SQL function calls need to identify the schema+relname of the relation being modified, and already have all of the values as typed Datums, whereas the internal API calls already have an identified and locked open Relation, but all of the statistical parameters are just cstrings.
>
> The solution I chose was to create common functions that take a relation object and an array of just the statistical parameters. This requires the pg_restore_* functions to resolve and lock the relation themselves, and then carry forward just the subset of parameters that are statistics (right types, but wrong number of arguments). Conversely, the internal API functions need to map their array of cstring values to the correctly typed Datums (wrong types, but right number of arguments in the right order).
>
> Attached is v2. The patches are broken down into very small bites to aid digestion and to confirm that tests pass after each comparatively minor change.

Thanks for doing that work!

I like this refactoring, but while it's rather mechanical, it's pretty
large, so I think it's too late to do the refactoring at this time
just before the beta 2 release.  So I'd vote for going with your
v1-0001 and v1-0002 and doing the refactoring in v20.  As mentioned by
Robert, I don't think it's good to call LOCAL_FCINFO() in
import_relation_statistics() and import_attribute_statistics() to call
the guts of those functions either, but that is *consistent* with the
existing way pg_restore_relation_stats() and
pg_restore_attribute_stats() do that, so that is actually not that
bad.  Also, as you mentioned above, it's inefficient for the new API
functions to lock an already-locked relation, and validate an
already-validated attname/attnum, but I think it would be negligible.

Best regards,
Etsuro Fujita

---

From: Robert Haas <[email-masked]>
Date: Mon, 29 Jun 2026 14:17:02 -0400

> already-validated attname/attnum, but I think it would be negligible.

Fujita-san, is it then your plan to get those two patches committed?

---

From: Corey Huinker <[email-masked]>
Date: Mon, 29 Jun 2026 21:10:41 -0400

> Fujita-san, is it then your plan to get those two patches committed?

Are we ok with changing the functions that postgres_fdw makes from v19 to
v20? Because what I put in the v2 patch does not match the v1 patch. If we
are NOT ok with that, then we'd need to:

1. rename the two relation_stats_argnum -> relation_args_argnum and
attribute_stats_argnum -> attribute_args_argnum out of the way of the ones
created in relation_stats.h and attribute_stats.h. The actual enumeration
values can stay the same, as there are no conflicts there, just ambiguity
about which set of values they belong to.
2. rename the existing static functions relation_statistics_update ->
update_relstats and attribute_statistics_update -> update_relstats to make
way for the public functions of the same names.
3. Create new relation_statistics_update and attribute_statistics_update,
with the isnull/values, and have those fcinfo-invoke the respective
pg_restore functions just like v1 does.

That would at least make the user API consistent in v19 vs v20.

I'm on vacation this week, but time is short for beta2, so I'll make an
exception to get the above into beta2. Let me know if you want me to write
that up.

---

From: Etsuro Fujita <[email-masked]>
Date: Tue, 30 Jun 2026 21:29:58 +0900

>> Fujita-san, is it then your plan to get those two patches committed?

Yes, it is.

> Are we ok with changing the functions that postgres_fdw makes from v19 to v20? Because what I put in the v2 patch does not match the v1 patch. If we are NOT ok with that, then we'd need to:
>
> 1. rename the two relation_stats_argnum -> relation_args_argnum and attribute_stats_argnum -> attribute_args_argnum out of the way of the ones created in relation_stats.h and attribute_stats.h. The actual enumeration values can stay the same, as there are no conflicts there, just ambiguity about which set of values they belong to.
> 2. rename the existing static functions relation_statistics_update -> update_relstats and attribute_statistics_update -> update_relstats to make way for the public functions of the same names.
> 3. Create new relation_statistics_update and attribute_statistics_update, with the isnull/values, and have those fcinfo-invoke the respective pg_restore functions just like v1 does.
>
> I'm on vacation this week, but time is short for beta2, so I'll make an exception to get the above into beta2. Let me know if you want me to write that up.

Thanks!

Here is an updated patch, which merges your v1-0001 and v1-0002.  One
notable change I made to your version is the signatures of
import_relation_statistics() and import_attribute_statistics().  As
mentioned upthread, the proposed signatures of these functions would
be too tailored for postgres_fdw, and might not be useful for other
FDWs, so I changed them to have NullableDatum arguments for stats-data
inputs like this:

bool
import_relation_statistics(Relation rel,
                           NullableDatum *relpages,
                           NullableDatum *reltuples,
                           NullableDatum *relallvisible,
                           NullableDatum *relallfrozen)

bool
import_attribute_statistics(Relation rel, AttrNumber attnum, bool inherited,
                            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)

As postgresImportForeignStatistics() is provided for FDWs that want to
calculate statistics remotely, so I'm assuming that they have enough
knowledge about the stats-data structures.  I think we could instead
use the values/isnull arrays as proposed in your v2, but the problem
about doing so is error messages in functions in stat_utils.c like
this:

        ereport(WARNING,
                (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
                 errmsg("argument \"%s\" must not be a multidimensional array",
                        arginfo[argnum].argname)));

Note that the message uses the word "argument", so I think it's better
for the new functions as well to have individual arguments for
stats-data inputs.

Also, I removed the attname argument from
import_attribute_statistics(), to 1) make the signature (and the
internal processing) simple and 2) match it to
delete_attribute_statistics().

In relation to this change, I moved helper functions set_XXX_arg()
that you added to relation_stats.c and attribute_stats.c to
postgres_fdw.c.  The helper functions had soft error handling, but in
the postgres_fdw use, there is no need for that, so I removed that
handling as well.

What do you think?  If there is no problem with this change, we
wouldn't need to worry about the stability of postgres_fdw (and other
FDWs) in v20.

Best regards,
Etsuro Fujita

---

From: Corey Huinker <[email-masked]>
Date: Tue, 30 Jun 2026 10:55:03 -0500

On Tue, Jun 30, 2026 at 7:30 AM Etsuro Fujita <[email-masked]>
wrote:

> > Are we ok with changing the functions that postgres_fdw makes from v19
> to v20? Because what I put in the v2 patch does not match the v1 patch. If
> we are NOT ok with that, then we'd need to:
> >
> > 3. Create new relation_statistics_update and
> attribute_statistics_update, with the isnull/values, and have those
> fcinfo-invoke the respective pg_restore functions just like v1 does.
> >
> knowledge about the stats-data structures.

>   I think we could instead

That's a good example of how our error messages get a bit clumsy with so
many ways of calling things, so I'm fine with your solution. It also
eliminates the possibility that a caller fails to notice a newly added stat
parameter and instead calls the input function with the older short array
length.

My initial concern was that this would bloat up postgres_fdw with type
conversion code, but the patch shows that it isn't that bad.

One thing we do lose in this is the assurance that the Datums passed are of
the correct type. Presently that is checked via
stats_fill_fcinfo_from_arg_pairs, which allows the *_statistics_update()
functions to trust the type correctness of the Datums that they're passed,
but I can be convinced that it isn't a big problem.

> delete_attribute_statistics().

I think that's reasonable. Internal callers have already resolved which
attribute they want to modify. The attname parameter was there so that
pg_restore_* calls could survive dump/restores where there were dropped
columns so the attnum has changed.

> the postgres_fdw use, there is no need for that, so I removed that

My inclination would be to move the set_*_arg functions could be moved to
some common utility file in v20, as other FDWs will find themselves with
the same need.

As for removing the error-handling, it might still be handy because it
would allow us to give a more context-aware error message, rather than the
very narrow "this_string is an invalid this_type", for string that the user
most certainly never saw. Having said that, it's something we could easily
change back to error-safe if we wanted to.

> wouldn't need to worry about the stability of postgres_fdw (and other

If you're fine with the names import_relation_statistics and
import_attribute_statistics, then yes. Those names made sense to me in the
narrow case of being called from an FDW handler and handing the function a
bunch of strings, but in a more general case with properly typed datums,
the name feels less appropriate, but it's just a name and I wouldn't let it
get in the way of the overall patch.

---

From: Corey Huinker <[email-masked]>
Date: Tue, 30 Jun 2026 11:21:08 -0500

> conversion code, but the patch shows that it isn't that bad.

One additional thought - in an offline conversation I had with Robert Haas,
he had taken the position that any FDW was likely pulling values across a
wire protocol and then synthesizing postgres-looking statistics from that,
so going with cstring inputs was fine, possibly preferable. I'm
highlighting that we're now passing in NullableDatums in case he wanted to
object to that change.

---

From: Etsuro Fujita <[email-masked]>
Date: Wed, 1 Jul 2026 21:09:58 +0900

On Wed, Jul 1, 2026 at 12:55 AM Corey Huinker <[email-masked]> wrote:
> On Tue, Jun 30, 2026 at 7:30 AM Etsuro Fujita <[email-masked]> wrote:

>> knowledge about the stats-data structures.

CORRECTION: s/postgresImportForeignStatistics()/ImportForeignStatistics()/
 Sorry for that.

> My inclination would be to move the set_*_arg functions could be moved to some common utility file in v20, as other FDWs will find themselves with the same need.

Seems like a good idea.  I moved the set_*_arg functions to
stat_utils.c, and renamed them to stats_set_*_arg.  Attached is a new
version of the patch.

> As for removing the error-handling, it might still be handy because it would allow us to give a more context-aware error message, rather than the very narrow "this_string is an invalid this_type", for string that the user most certainly never saw. Having said that, it's something we could easily change back to error-safe if we wanted to.

Ok, I think we could do so later if needed.

> If you're fine with the names import_relation_statistics and import_attribute_statistics, then yes. Those names made sense to me in the narrow case of being called from an FDW handler and handing the function a bunch of strings, but in a more general case with properly typed datums, the name feels less appropriate, but it's just a name and I wouldn't let it get in the way of the overall patch.

I like the names, but I'm open to suggestions.

I modified the patch further.  To support these differences:

"There are important differences in the parameters needed by the
different types of functions. The pg_restore_*() SQL function calls
need to identify the schema+relname of the relation being modified,
and already have all of the values as typed Datums, whereas the
internal API calls already have an identified and locked open
Relation, but all of the statistical parameters are just cstrings."

I incorporated your v2-0010 and v2-0012 into the patch: 1) separate
the guts of relation_statistics_update() and
attribute_statistics_update() into new functions
relation_statistics_update_internal() and
attribute_statistics_update_internal(), and 2) modify
import_relation_statistics() and import_attribute_statistics() to call
these new functions instead.  Also, I changed the stats-data
argument-type in import_relation_statistics() and
import_attribute_statistics() from NullableDatum * to const
NullableDatum *.

Best regards,
Etsuro Fujita

---

From: Corey Huinker <[email-masked]>
Date: Wed, 1 Jul 2026 16:34:53 -0400

On Wed, Jul 1, 2026 at 8:10 AM Etsuro Fujita <[email-masked]>
wrote:

> > My inclination would be to move the set_*_arg functions could be moved
> to some common utility file in v20, as other FDWs will find themselves with
> the same need.
>

I'm concerned that by making these non-error-safe function calls available,
we create a barrier to making those same functions error-safe in the future.

> Ok, I think we could do so later if needed.

I agree we could do it later if they were private functions, no problem.
But if they're available outside of postgres_fdw then changing them to be
error-safe potentially disrupts other callers. Someone please let me know
if I'm being unnecessarily cautious here.

> import_attribute_statistics() from NullableDatum * to const

Other Thoughts:

1. The internal functions should accept a server_version_number parameter,
and we should document that setting that parameter to 0 mean that we can
assume the current version. I know that we presently have no translation
issues going forward with statistics, but someday that won't be the case,
and if we don't have this parameter in place then it'll be too late to fix
it.

2. Did you put import_relation_statistics and import_attribute_statistics
in statistics.h because you see them as long-term publicly visible
functions, and what's in stats_utils.h to be more subject to change? If so,
I could get behind that. If not, then I fall back to my position that they
seem like they belong in new relation_stats.h and attribute_stats.h, or
stats_utils.h.

3. The import_stats_functions in their current "bridging" form should do
null checks on all of the NullableDatum pointers, or at least Assert()s on
them.

---

From: Etsuro Fujita <[email-masked]>
Date: Fri, 3 Jul 2026 18:47:54 +0900

On Thu, Jul 2, 2026 at 5:35 AM Corey Huinker <[email-masked]> wrote:
> On Wed, Jul 1, 2026 at 8:10 AM Etsuro Fujita <[email-masked]> wrote:

> I'm concerned that by making these non-error-safe function calls available, we create a barrier to making those same functions error-safe in the future.
>
> I agree we could do it later if they were private functions, no problem. But if they're available outside of postgres_fdw then changing them to be error-safe potentially disrupts other callers. Someone please let me know if I'm being unnecessarily cautious here.

Ah, I misunderstood your comments.  I agree with you here, so I'll
move back the functions to postgres_fdw.c.

> Other Thoughts:
>
> 1. The internal functions should accept a server_version_number parameter, and we should document that setting that parameter to 0 mean that we can assume the current version. I know that we presently have no translation issues going forward with statistics, but someday that won't be the case, and if we don't have this parameter in place then it'll be too late to fix it.

Good idea!  Will do.

> 2. Did you put import_relation_statistics and import_attribute_statistics in statistics.h because you see them as long-term publicly visible functions, and what's in stats_utils.h to be more subject to change? If so, I could get behind that. If not, then I fall back to my position that they seem like they belong in new relation_stats.h and attribute_stats.h, or stats_utils.h.

The answer is the former.  I'll remove the changes made to
stats_utils.h, though.

> 3. The import_stats_functions in their current "bridging" form should do null checks on all of the NullableDatum pointers, or at least Assert()s on them.

Will do.

Thanks for the comments!

Best regards,
Etsuro Fujita

---

From: Etsuro Fujita <[email-masked]>
Date: Wed, 8 Jul 2026 21:04:51 +0900

> move back the functions to postgres_fdw.c.

Done.

> > 1. The internal functions should accept a server_version_number parameter, and we should document that setting that parameter to 0 mean that we can assume the current version. I know that we pres…

I used the name "version" for the consistency with
pg_restore_relation_stats/pg_restore_attribute_stats.  Also, as I
couldn't find a note about the arrangement in the source code, I added
this comment to match a comment in stats_fill_fcinfo_from_arg_pairs:

 * For now, the 'version' argument is ignored. In the future it can be used
 * to interpret older statistics properly.

> > 3. The import_stats_functions in their current "bridging" form should do null checks on all of the NullableDatum pointers, or at least Assert()s on them.

Done.  As the functions are provided for developers, I just added the
assertions.

Also, I updated comments/docs a little bit.  Attached is a new version
of the patch.  I'm planning to push and back-patch it, if no
objections.

Best regards,
Etsuro Fujita