diff --git a/contrib/postgres_fdw/deparse.c b/contrib/postgres_fdw/deparse.c index 673b678826c..ff9fe0f87e4 100644 --- a/contrib/postgres_fdw/deparse.c +++ b/contrib/postgres_fdw/deparse.c @@ -2647,11 +2647,14 @@ deparseAnalyzeSizeSql(StringInfo buf, Relation rel) /* * Construct SELECT statement to acquire the number of pages, the number of - * rows, and the relkind of a relation. + * rows, and the relkind/relhassubclass of a relation. * - * Note: we just return the remote server's reltuples value, which might - * be off a good deal, but it doesn't seem worth working harder. See + * Note: we just return the remote server's relpages/reltuples values, which + * might be off a good deal, but it doesn't seem worth working harder. See * comments in postgresAcquireSampleRowsFunc. + * + * Note: in the stats-import case it is the user's responsibility to ensure + * that those stats values are up-to-date. */ void deparseAnalyzeInfoSql(StringInfo buf, Relation rel) @@ -2662,7 +2665,7 @@ deparseAnalyzeInfoSql(StringInfo buf, Relation rel) initStringInfo(&relname); deparseRelation(&relname, rel); - appendStringInfoString(buf, "SELECT relpages, reltuples, relkind FROM pg_catalog.pg_class WHERE oid = "); + appendStringInfoString(buf, "SELECT relpages, reltuples, relkind, relhassubclass FROM pg_catalog.pg_class WHERE oid = "); deparseStringLiteral(buf, relname.data); appendStringInfoString(buf, "::pg_catalog.regclass"); } diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index 517d15cf1fa..a6295674daf 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -13823,6 +13823,31 @@ CREATE FOREIGN TABLE simport_fview (c1 int, c2 text) ALTER FOREIGN TABLE simport_fview OPTIONS (ADD import_stats 'true'); ANALYZE simport_fview; -- should fail WARNING: could not import statistics for foreign table "public.simport_fview" --- remote table "public.simport_view" is of relkind "v" which cannot have statistics +CREATE TABLE simport_pt (c1 int not null, c2 text) PARTITION BY LIST (c1); +CREATE TABLE simport_p1 PARTITION OF simport_pt FOR VALUES IN (1); +CREATE TABLE simport_p2 PARTITION OF simport_pt FOR VALUES IN (2); +CREATE FOREIGN TABLE simport_fpt (c1 int not null, c2 text) + SERVER loopback OPTIONS (table_name 'simport_pt'); +INSERT INTO simport_pt VALUES (1, 'foo'), (1, 'foo'), (2, 'bar'), (2, 'bar'); +-- Check that we have relpages = 0 for simport_fpt regardless of the method +ANALYZE simport_fpt; +SELECT relpages FROM pg_class WHERE oid = 'public.simport_fpt'::regclass; + relpages +---------- + 0 +(1 row) + +ALTER FOREIGN TABLE simport_fpt OPTIONS (ADD import_stats 'true'); +ANALYZE simport_pt; +ANALYZE VERBOSE simport_fpt; -- should work +INFO: importing statistics for foreign table "public.simport_fpt" +INFO: finished importing statistics for foreign table "public.simport_fpt" +SELECT relpages FROM pg_class WHERE oid = 'public.simport_fpt'::regclass; + relpages +---------- + 0 +(1 row) + -- This tests build_remattrmap()'s deparsing of column names that include -- single quotes or backslashes CREATE TABLE dtest_table ("col'quote" int, "col\backslash" int); @@ -13875,6 +13900,8 @@ DROP FOREIGN TABLE simport_ftable; DROP FOREIGN TABLE simport_fview; DROP VIEW simport_view; DROP TABLE simport_table; +DROP FOREIGN TABLE simport_fpt; +DROP TABLE simport_pt; DROP FOREIGN TABLE dtest_ftable; DROP TABLE dtest_table; -- =================================================================== diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c index 9269418a074..f69631e2c63 100644 --- a/contrib/postgres_fdw/postgres_fdw.c +++ b/contrib/postgres_fdw/postgres_fdw.c @@ -355,6 +355,7 @@ enum RelStatsColumns RELSTATS_RELPAGES = 0, RELSTATS_RELTUPLES, RELSTATS_RELKIND, + RELSTATS_RELHASSUBCLASS, RELSTATS_NUM_FIELDS, }; @@ -385,6 +386,7 @@ typedef struct PGresult *att; /* result for attribute stats query */ double livetuples; /* livetuples estimates, for pgstat report */ double deadtuples; /* deadtuples estimates, for pgstat report */ + char relkind; /* relkind of remote table */ int version; /* version of remote server */ } RemoteStatsResults; @@ -5294,7 +5296,7 @@ postgresGetAnalyzeInfoForForeignTable(Relation relation, bool *can_tablesample) if (PQntuples(res) != 1 || PQnfields(res) != RELSTATS_NUM_FIELDS) elog(ERROR, "unexpected result from deparseAnalyzeInfoSql query"); - /* We don't use relpages here */ + /* We don't use relpages/relhassubclass here */ reltuples = strtod(PQgetvalue(res, 0, RELSTATS_RELTUPLES), NULL); relkind = *(PQgetvalue(res, 0, RELSTATS_RELKIND)); PQclear(res); @@ -5850,7 +5852,7 @@ fetch_remote_statistics(Relation relation, * RELKIND_PARTITIONED_INDEX can have rows in pg_stats, they obviously * can't support a foreign table. */ - relkind = *PQgetvalue(relstats, 0, RELSTATS_RELKIND); + remstats->relkind = relkind = *PQgetvalue(relstats, 0, RELSTATS_RELKIND); switch (relkind) { case RELKIND_RELATION: @@ -5866,6 +5868,23 @@ fetch_remote_statistics(Relation relation, goto fetch_cleanup; } + /* + * If the remote table is inherited, relpages/reltuples in pg_class for + * it show stats for the parent table, not for the inheritance set. We + * could calculate stats for the set by fetching the relation stats for + * child tables as well; but for now, just fallback to sampling. + */ + if ((relkind == RELKIND_RELATION || relkind == RELKIND_FOREIGN_TABLE) && + strcmp(PQgetvalue(relstats, 0, RELSTATS_RELHASSUBCLASS), "t") == 0) + { + ereport(WARNING, + errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("could not import statistics for foreign table \"%s.%s\" --- remote table \"%s.%s\" is inherited", + local_schemaname, local_relname, + remote_schemaname, remote_relname)); + goto fetch_cleanup; + } + /* * If the reltuples value > 0, then we can expect to find attribute stats * for the remote table. @@ -6005,7 +6024,12 @@ fetch_attstats(PGconn *conn, int server_version_num, " AND attname = ANY(%s)", column_list); - /* inherited is supported since Postgres 9.0 */ + /* + * inherited is supported since Postgres 9.0 + * + * Note that this is okay because for now, we support only the case where + * the remote table is partitioned (see fetch_remote_statistics()). + */ if (server_version_num >= 90000) appendStringInfoString(&sql, " ORDER BY attname COLLATE \"C\", inherited DESC"); @@ -6337,9 +6361,20 @@ import_fetched_statistics(Relation relation, Assert(PQnfields(res) == RELSTATS_NUM_FIELDS); Assert(PQntuples(res) == 1); - /* Set the remaining parameters. */ - set_int32_arg(&args[1], get_opt_value(res, 0, RELSTATS_RELPAGES)); - Assert(!args[1].isnull); + /* + * Set the remaining parameters. If the remote table is partitioned, set + * the 'relpages' parameter to 0, to match the sampling case. + */ + if (remstats->relkind == RELKIND_PARTITIONED_TABLE) + { + args[1].value = Int32GetDatum(0); + args[2].isnull = false; + } + else + { + set_int32_arg(&args[1], get_opt_value(res, 0, RELSTATS_RELPAGES)); + Assert(!args[1].isnull); + } set_float_arg(&args[2], get_opt_value(res, 0, RELSTATS_RELTUPLES)); Assert(!args[2].isnull); /* We don't import relallvisible/relallfrozen. */ diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql index ec766e2b28a..eaeb90485e8 100644 --- a/contrib/postgres_fdw/sql/postgres_fdw.sql +++ b/contrib/postgres_fdw/sql/postgres_fdw.sql @@ -4960,6 +4960,21 @@ ALTER FOREIGN TABLE simport_fview OPTIONS (ADD import_stats 'true'); ANALYZE simport_fview; -- should fail +CREATE TABLE simport_pt (c1 int not null, c2 text) PARTITION BY LIST (c1); +CREATE TABLE simport_p1 PARTITION OF simport_pt FOR VALUES IN (1); +CREATE TABLE simport_p2 PARTITION OF simport_pt FOR VALUES IN (2); +CREATE FOREIGN TABLE simport_fpt (c1 int not null, c2 text) + SERVER loopback OPTIONS (table_name 'simport_pt'); +INSERT INTO simport_pt VALUES (1, 'foo'), (1, 'foo'), (2, 'bar'), (2, 'bar'); + +-- Check that we have relpages = 0 for simport_fpt regardless of the method +ANALYZE simport_fpt; +SELECT relpages FROM pg_class WHERE oid = 'public.simport_fpt'::regclass; +ALTER FOREIGN TABLE simport_fpt OPTIONS (ADD import_stats 'true'); +ANALYZE simport_pt; +ANALYZE VERBOSE simport_fpt; -- should work +SELECT relpages FROM pg_class WHERE oid = 'public.simport_fpt'::regclass; + -- This tests build_remattrmap()'s deparsing of column names that include -- single quotes or backslashes CREATE TABLE dtest_table ("col'quote" int, "col\backslash" int); @@ -5004,6 +5019,8 @@ DROP FOREIGN TABLE simport_ftable; DROP FOREIGN TABLE simport_fview; DROP VIEW simport_view; DROP TABLE simport_table; +DROP FOREIGN TABLE simport_fpt; +DROP TABLE simport_pt; DROP FOREIGN TABLE dtest_ftable; DROP TABLE dtest_table; diff --git a/doc/src/sgml/postgres-fdw.sgml b/doc/src/sgml/postgres-fdw.sgml index 87b1433aacb..fb6404636ee 100644 --- a/doc/src/sgml/postgres-fdw.sgml +++ b/doc/src/sgml/postgres-fdw.sgml @@ -375,7 +375,7 @@ OPTIONS (ADD password_required 'false'); the local server. If the attempt failed, statistics are collected by row sampling on the foreign table. This option is only useful if the remote table is one that can have - regular statistics (tables and materialized views). + regular statistics (tables, not inherited, and materialized views). When using this option, it is the user's responsibility to ensure that the existing statistics for the foreign table are up-to-date.