public inbox for [email protected]
help / color / mirror / Atom feedFrom: Etsuro Fujita <[email protected]>
To: Corey Huinker <[email protected]>
Cc: Robert Haas <[email protected]>
Cc: [email protected] <[email protected]>
Subject: Re: use of SPI by postgresImportForeignStatistics
Date: Tue, 30 Jun 2026 21:29:58 +0900
Message-ID: <CAPmGK14dY-Cex2rc08TinQjHR4Q1L6nqPBfdEBRZusP7n_TZSA@mail.gmail.com> (raw)
In-Reply-To: <CADkLM=f_c7Xoq-bSzw28LgvxPa+OQ3rVow=xwkTyjz1J0=pMdw@mail.gmail.com>
References: <CA+TgmoYqMtWb4zLUkT98oFnEkJ=Wz0Pw-ggDJrp9wnSXPzUaeQ@mail.gmail.com>
<CAPmGK161TJ4G01eptv3sBqgxa4SNHDE9VRqoC6jy5Y6G3D8yDw@mail.gmail.com>
<CA+TgmoZpwfTGqqp=0efV99e_BMYWdLFq2XKRWVSDcukifkZMeA@mail.gmail.com>
<CADkLM=dxe8NbQu4RNi2OFXpaRKdpS0-CHGm6Mqtf_rW155BM4g@mail.gmail.com>
<CADkLM=cV=uezAKyZdyyxnZOBRhGLUSbeYfDj5_jbDCLP9LP24Q@mail.gmail.com>
<CA+TgmoaVcgZJpwxyi7HEfNMJhmm7BXkTQ468jvxkW2QzQZRkvg@mail.gmail.com>
<CADkLM=dy18dDJ+j3-_Ofn11vt5J7zF33oTWnE6N=ePc83uS91A@mail.gmail.com>
<CAPmGK15MfVT2H+PUvA_nAJvySGr_pAVkVx3KcnVXWGO36acWLA@mail.gmail.com>
<CADkLM=dr5bfB8YN-bV1U0NDvyVO7a6uuz0BsAi89a1Md4A==8g@mail.gmail.com>
<CAPmGK14P2GRe78hp6fZ3vExU_9RRDjMsYVOx7AALvUQ-hKTUiQ@mail.gmail.com>
<CADkLM=cL9ZGvO-72QWyXA5Wqv-4P3T1fp+VFJy5e6k3Hf=POMw@mail.gmail.com>
<CA+TgmoZdTVeXRQo__cEr_1y+mUO9nubOX=6PEfw7Cr0UfZQn6w@mail.gmail.com>
<CADkLM=fe=ycPdwT2wE_iYwK8e8viMbWvwWhXR9vDe+55KKF-KQ@mail.gmail.com>
<CADkLM=dWJaw04u9FU=WkykOy7bu7B5eKmoDkfVz6JXh7mdemkw@mail.gmail.com>
<CA+TgmoY3a_S0munZ0wg4hQpMKVMyM_jpi6aGhFR0gAm1jLukag@mail.gmail.com>
<CADkLM=cdKjgphHpFW8kKukHF+DVYKogHcqLXO=BA8MGfiRFqQA@mail.gmail.com>
<CADkLM=c9BEuA9vPjE9Wco6+Lz-HSnnmBZrFhi=-yNi0K8tmZug@mail.gmail.com>
<CAPmGK17W1sUB3JnkRoBpgO0_iSH5HRcc0_nxk8Us4KYsQhuQxg@mail.gmail.com>
<CA+TgmoZd5MDhcx6uav6MWxhM42yWcfaRAHYKNtNkuog=yrKAgw@mail.gmail.com>
<CADkLM=f_c7Xoq-bSzw28LgvxPa+OQ3rVow=xwkTyjz1J0=pMdw@mail.gmail.com>
On Tue, Jun 30, 2026 at 10:10 AM Corey Huinker <[email protected]> wrote:
> On Mon, Jun 29, 2026 at 2:17 PM Robert Haas <[email protected]> wrote:
>> On Mon, Jun 29, 2026 at 7:50 AM Etsuro Fujita <[email protected]> wrote:
>> > 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.
>>
>> 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.
>
> 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.
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
Attachments:
[application/octet-stream] Remove-SPI-from-stat-import-in-postgres-fdw-efujita.patch (25.8K, ../CAPmGK14dY-Cex2rc08TinQjHR4Q1L6nqPBfdEBRZusP7n_TZSA@mail.gmail.com/2-Remove-SPI-from-stat-import-in-postgres-fdw-efujita.patch)
download | inline diff:
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index e90289e4ab1..51d09b27267 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -12927,6 +12927,43 @@ 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_ftable's stats should now exactly match dtest_table's
+-- 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)
+
+-- 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)
+
+-- 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 6dbae583ecc..b171d26b532 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"
@@ -46,6 +45,7 @@
#include "storage/latch.h"
#include "utils/builtins.h"
#include "utils/float.h"
+#include "utils/fmgroids.h"
#include "utils/guc.h"
#include "utils/lsyscache.h"
#include "utils/memutils.h"
@@ -335,7 +335,6 @@ typedef struct
{
PGresult *rel;
PGresult *att;
- int server_version_num;
} RemoteStatsResults;
/* Column order in relation stats query */
@@ -367,136 +366,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"
-"\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
*/
@@ -714,14 +583,18 @@ 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 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 char *get_opt_value(PGresult *res, int row, int col);
+static void set_text_arg(NullableDatum *arg, const char *s);
+static void set_int32_arg(NullableDatum *arg, const char *s);
+static void set_uint32_arg(NullableDatum *arg, const char *s);
+static void set_float_arg(NullableDatum *arg, const char *s);
+static void set_floatarr_arg(NullableDatum *arg, const char *s);
static void produce_tuple_asynchronously(AsyncRequest *areq, bool fetch);
static void fetch_more_data_begin(AsyncRequest *areq);
static void complete_pending_request(AsyncRequest *areq);
@@ -5661,7 +5534,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)
@@ -5725,7 +5598,7 @@ fetch_remote_statistics(Relation relation,
*/
user = GetUserMapping(GetUserId(), table->serverid);
conn = GetConnection(user, false, NULL);
- remstats->server_version_num = server_version_num = PQserverVersion(conn);
+ server_version_num = PQserverVersion(conn);
/* Fetch relation stats. */
remstats->rel = relstats = fetch_relstats(conn, relation);
@@ -6128,58 +6001,31 @@ 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,
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;
-
- /* 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();
+ PGresult *res;
+ NullableDatum args[ATTSTATS_NUM_FIELDS - 1];
/*
* We import attribute statistics first, if any, because those are more
* prone to errors. This avoids making a modification of pg_class that
* will just get rolled back by a failed attribute import.
*/
- if (remstats->att != NULL)
+ res = remstats->att;
+ if (res != NULL)
{
- Assert(PQnfields(remstats->att) == ATTSTATS_NUM_FIELDS);
- Assert(PQntuples(remstats->att) >= 1);
-
- 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;
/* All mappings should have been assigned a result set row. */
Assert(row >= 0);
@@ -6191,128 +6037,192 @@ import_fetched_statistics(const char *schemaname,
/*
* 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);
-
- 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())
+ delete_attribute_statistics(relation, attnum, false);
+
+ set_float_arg(&args[0],
+ get_opt_value(res, row, ATTSTATS_NULL_FRAC));
+ set_int32_arg(&args[1],
+ get_opt_value(res, row, ATTSTATS_AVG_WIDTH));
+ set_float_arg(&args[2],
+ get_opt_value(res, row, ATTSTATS_N_DISTINCT));
+ set_text_arg(&args[3],
+ get_opt_value(res, row, ATTSTATS_MOST_COMMON_VALS));
+ set_floatarr_arg(&args[4],
+ get_opt_value(res, row, ATTSTATS_MOST_COMMON_FREQS));
+ set_text_arg(&args[5],
+ get_opt_value(res, row, ATTSTATS_HISTOGRAM_BOUNDS));
+ set_float_arg(&args[6],
+ get_opt_value(res, row, ATTSTATS_CORRELATION));
+ set_text_arg(&args[7],
+ get_opt_value(res, row, ATTSTATS_MOST_COMMON_ELEMS));
+ set_floatarr_arg(&args[8],
+ get_opt_value(res, row, ATTSTATS_MOST_COMMON_ELEM_FREQS));
+ set_floatarr_arg(&args[9],
+ get_opt_value(res, row, ATTSTATS_ELEM_COUNT_HISTOGRAM));
+ set_text_arg(&args[10],
+ get_opt_value(res, row, ATTSTATS_RANGE_LENGTH_HISTOGRAM));
+ set_float_arg(&args[11],
+ get_opt_value(res, row, ATTSTATS_RANGE_EMPTY_FRAC));
+ set_text_arg(&args[12],
+ get_opt_value(res, row, ATTSTATS_RANGE_BOUNDS_HISTOGRAM));
+
+ if (!import_attribute_statistics(relation, attnum, false,
+ &args[0], &args[1], &args[2],
+ &args[3], &args[4], &args[5],
+ &args[6], &args[7], &args[8],
+ &args[9], &args[10], &args[11],
+ &args[12]))
{
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));
- goto import_cleanup;
+ return false;
}
}
}
/*
- * 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);
-
- 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 (!import_spi_query_ok())
+ * Import relation stats.
+ */
+ res = remstats->rel;
+ Assert(res != NULL);
+ Assert(PQnfields(res) == RELSTATS_NUM_FIELDS);
+ Assert(PQntuples(res) == 1);
+
+ set_uint32_arg(&args[0], get_opt_value(res, 0, RELSTATS_RELPAGES));
+ Assert(!args[0].isnull);
+ set_float_arg(&args[1], get_opt_value(res, 0, RELSTATS_RELTUPLES));
+ Assert(!args[1].isnull);
+ args[2].value = (Datum) 0;
+ args[2].isnull = true;
+ args[3].value = (Datum) 0;
+ args[3].isnull = true;
+
+ if (!import_relation_statistics(relation,
+ &args[0], &args[1], &args[2], &args[3]))
{
ereport(WARNING,
errmsg("could not import statistics for foreign table \"%s.%s\" --- relation statistics import failed for this foreign table",
schemaname, relname));
- goto import_cleanup;
+ return false;
}
- ok = true;
+ return true;
+}
-import_cleanup:
- if (attimport_plan)
- SPI_freeplan(attimport_plan);
- if (attclear_plan)
- SPI_freeplan(attclear_plan);
- SPI_finish();
- return ok;
+/*
+ * 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);
}
/*
- * Move a string value from a result set to a Text value of a Datum array.
+ * Convenience routine for setting optional text arguments
*/
static void
-map_field_to_arg(PGresult *res, int row, int field,
- int arg, Datum *values, char *nulls)
+set_text_arg(NullableDatum *arg, const char *s)
{
- if (PQgetisnull(res, row, field))
+ if (s)
{
- values[arg] = (Datum) 0;
- nulls[arg] = 'n';
+ arg->value = CStringGetTextDatum(s);
+ arg->isnull = false;
}
else
{
- const char *s = PQgetvalue(res, row, field);
+ arg->value = (Datum) 0;
+ arg->isnull = true;
+ }
+}
+
+/*
+ * Convenience routine for setting optional int32 arguments
+ */
+static void
+set_int32_arg(NullableDatum *arg, const char *s)
+{
+ if (s)
+ {
+ int32 val = pg_strtoint32(s);
- values[arg] = CStringGetTextDatum(s);
- nulls[arg] = ' ';
+ arg->value = Int32GetDatum(val);
+ arg->isnull = false;
+ }
+ else
+ {
+ arg->value = (Datum) 0;
+ arg->isnull = true;
}
}
/*
- * Check the 1x1 result set of a pg_restore_*_stats() command for success.
+ * Convenience routine for setting optional uint32 arguments
*/
-static bool
-import_spi_query_ok(void)
+static void
+set_uint32_arg(NullableDatum *arg, const char *s)
{
- TupleDesc tupdesc;
- Datum dat;
- bool isnull;
+ if (s)
+ {
+ uint32 val = uint32in_subr(s, NULL, "uint32", NULL);
- Assert(SPI_tuptable != NULL);
- Assert(SPI_processed == 1);
+ arg->value = UInt32GetDatum(val);
+ 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)
+{
+ if (s)
+ {
+ float4 val = float4in_internal((char *) s, NULL, "float", s, NULL);
- 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);
+ arg->value = Float4GetDatum(val);
+ arg->isnull = false;
+ }
+ else
+ {
+ arg->value = (Datum) 0;
+ arg->isnull = true;
+ }
+}
- return DatumGetBool(dat);
+/*
+ * Convenience routine for setting optional float[] arguments
+ */
+static void
+set_floatarr_arg(NullableDatum *arg, const char *s)
+{
+ if (s)
+ {
+ FmgrInfo flinfo;
+ Datum val;
+
+ fmgr_info(F_ARRAY_IN, &flinfo);
+ val = InputFunctionCall(&flinfo, (char *) s, FLOAT4OID, -1);
+
+ arg->value = val;
+ arg->isnull = false;
+ }
+ else
+ {
+ arg->value = (Datum) 0;
+ arg->isnull = true;
+ }
}
/*
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
index dfc58beb0d2..0d0622b31ce 100644
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -4584,6 +4584,34 @@ ANALYZE dtest_table;
ANALYZE VERBOSE dtest_ftable; -- should work
+-- dtest_ftable's stats should now exactly match dtest_table's
+-- 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;
+
+-- 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';
+
+-- 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;
diff --git a/src/backend/statistics/attribute_stats.c b/src/backend/statistics/attribute_stats.c
index 1cc4d657231..0d88e18096b 100644
--- a/src/backend/statistics/attribute_stats.c
+++ b/src/backend/statistics/attribute_stats.c
@@ -688,3 +688,67 @@ pg_restore_attribute_stats(PG_FUNCTION_ARGS)
PG_RETURN_BOOL(result);
}
+
+/*
+ * Import attribute statistics from NullableDatum inputs for all statitical
+ * values.
+ */
+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)
+{
+ LOCAL_FCINFO(newfcinfo, NUM_ATTRIBUTE_STATS_ARGS);
+
+ InitFunctionCallInfoData(*newfcinfo, NULL, NUM_ATTRIBUTE_STATS_ARGS,
+ InvalidOid, NULL, NULL);
+
+ 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;
+ newfcinfo->args[ATTNAME_ARG].value = (Datum) 0;
+ newfcinfo->args[ATTNAME_ARG].isnull = true;
+ newfcinfo->args[ATTNUM_ARG].value = Int16GetDatum(attnum);
+ newfcinfo->args[ATTNUM_ARG].isnull = false;
+ newfcinfo->args[INHERITED_ARG].value = BoolGetDatum(inherited);
+ newfcinfo->args[INHERITED_ARG].isnull = false;
+
+ newfcinfo->args[NULL_FRAC_ARG] = *null_frac;
+ newfcinfo->args[AVG_WIDTH_ARG] = *avg_width;
+ newfcinfo->args[N_DISTINCT_ARG] = *n_distinct;
+ newfcinfo->args[MOST_COMMON_VALS_ARG] = *most_common_vals;
+ newfcinfo->args[MOST_COMMON_FREQS_ARG] = *most_common_freqs;
+ newfcinfo->args[HISTOGRAM_BOUNDS_ARG] = *histogram_bounds;
+ newfcinfo->args[CORRELATION_ARG] = *correlation;
+ newfcinfo->args[MOST_COMMON_ELEMS_ARG] = *most_common_elems;
+ newfcinfo->args[MOST_COMMON_ELEM_FREQS_ARG] = *most_common_elem_freqs;
+ newfcinfo->args[ELEM_COUNT_HISTOGRAM_ARG] = *elem_count_histogram;
+ newfcinfo->args[RANGE_LENGTH_HISTOGRAM_ARG] = *range_length_histogram;
+ newfcinfo->args[RANGE_EMPTY_FRAC_ARG] = *range_empty_frac;
+ newfcinfo->args[RANGE_BOUNDS_HISTOGRAM_ARG] = *range_bounds_histogram;
+
+ return attribute_statistics_update(newfcinfo);
+}
+
+/*
+ * Delete attribute statistics.
+ */
+bool
+delete_attribute_statistics(Relation rel, AttrNumber attnum, bool inherited)
+{
+ return delete_pg_statistic(RelationGetRelid(rel), attnum, inherited);
+}
diff --git a/src/backend/statistics/relation_stats.c b/src/backend/statistics/relation_stats.c
index d6631e9a9a4..1bd4b6e0dba 100644
--- a/src/backend/statistics/relation_stats.c
+++ b/src/backend/statistics/relation_stats.c
@@ -21,6 +21,7 @@
#include "catalog/indexing.h"
#include "catalog/namespace.h"
#include "nodes/makefuncs.h"
+#include "statistics/statistics.h"
#include "statistics/stat_utils.h"
#include "utils/builtins.h"
#include "utils/fmgroids.h"
@@ -241,3 +242,34 @@ pg_restore_relation_stats(PG_FUNCTION_ARGS)
PG_RETURN_BOOL(result);
}
+
+/*
+ * Import relation statistics from NullableDatum inputs for all statitical
+ * values.
+ */
+bool
+import_relation_statistics(Relation rel,
+ NullableDatum *relpages,
+ NullableDatum *reltuples,
+ NullableDatum *relallvisible,
+ NullableDatum *relallfrozen)
+{
+ LOCAL_FCINFO(newfcinfo, NUM_RELATION_STATS_ARGS);
+
+ InitFunctionCallInfoData(*newfcinfo, NULL, NUM_RELATION_STATS_ARGS,
+ InvalidOid, NULL, NULL);
+
+ 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] = *relpages;
+ newfcinfo->args[RELTUPLES_ARG] = *reltuples;
+ newfcinfo->args[RELALLVISIBLE_ARG] = *relallvisible;
+ newfcinfo->args[RELALLFROZEN_ARG] = *relallfrozen;
+
+ return relation_statistics_update(newfcinfo);
+}
diff --git a/src/include/statistics/statistics.h b/src/include/statistics/statistics.h
index 8f9b9d237fd..b61b85858ff 100644
--- a/src/include/statistics/statistics.h
+++ b/src/include/statistics/statistics.h
@@ -128,4 +128,27 @@ extern StatisticExtInfo *choose_best_statistics(List *stats, char requiredkind,
int nclauses);
extern HeapTuple statext_expressions_load(Oid stxoid, bool inh, int idx);
+extern bool import_relation_statistics(Relation rel,
+ NullableDatum *relpages,
+ NullableDatum *reltuples,
+ NullableDatum *relallvisible,
+ NullableDatum *relallfrozen);
+extern 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);
+extern bool delete_attribute_statistics(Relation rel,
+ AttrNumber attnum, bool inherited);
+
#endif /* STATISTICS_H */
view thread (33+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected]
Subject: Re: use of SPI by postgresImportForeignStatistics
In-Reply-To: <CAPmGK14dY-Cex2rc08TinQjHR4Q1L6nqPBfdEBRZusP7n_TZSA@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox