public inbox for [email protected]
help / color / mirror / Atom feedFrom: Chao Li <[email protected]>
To: Etsuro Fujita <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: Corey Huinker <[email protected]>
Subject: Re: postgres_fdw: fix cumulative stats after imported foreign-table stats
Date: Sat, 4 Jul 2026 06:23:45 -0700
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAPmGK17uDi89_D16UHrcct5_JwbtY_ojFtkd5gXsvJNiVJDg5Q@mail.gmail.com>
References: <[email protected]>
<CAPmGK17uDi89_D16UHrcct5_JwbtY_ojFtkd5gXsvJNiVJDg5Q@mail.gmail.com>
> On Jul 3, 2026, at 02:10, Etsuro Fujita <[email protected]> wrote:
>
> Hi Chao,
>
> On Fri, Jun 12, 2026 at 12:49 PM Chao Li <[email protected]> wrote:
>> I think the root cause is that, with 28972b6fc, when stats are imported successfully for a foreign table, do_analyze_rel() is skipped. But do_analyze_rel() is the only place that calls pgstat_report_analyze() to update cumulative stats.
>>
>> To fix this, I think we need to add an output parameter to ImportForeignStatistics to pass out total live rows. AFAIK, the imported remote relation stats have no dead-tuple estimate, so analyze_rel() can pass 0 as the dead-tuple estimate when calling pgstat_report_analyze(). That gives us the needed data to call pgstat_report_analyze() after a successful import.
>
> The root-cause analysis is correct, but I don't think that the fix is
> the right way to go, because if we modified pgstat_report_analyze() to
> report more ANALYZE stats, we would need to modify the
> ImportForeignStatistics API as well, which isn't great. To avoid
> that, how about calling pgstat_report_analyze() in
> postgresImportForeignStatistics(), like the attached? (Actually, I
> designed the API as something we entirely replace do_analyze_rel()
> with.) I modified the test case as well.
>
> Sorry for the delay.
>
> Best regards,
> Etsuro Fujita
> <Fix-ANALYZE-report-in-postgres-fdw-stat-import-efujita.patch>
Hi Etsuro-san,
Thanks for your review and for proposing an alternative fix.
I thought postgres_fdw was where we interact with the remote server, while analyze_rel() is the code deciding that ANALYZE succeeded. Ideally, cumulative ANALYZE reporting would be driven from that level, not from an FDW callback implementation.
But I understand your concern about changing the ImportForeignStatistics API, so I’m fine with your proposal. I have integrated your changes into v2.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
Attachments:
[application/octet-stream] v2-0001-Fix-ANALYZE-reporting-after-imported-foreign-tabl.patch (8.7K, ../[email protected]/2-v2-0001-Fix-ANALYZE-reporting-after-imported-foreign-tabl.patch)
download | inline diff:
From 2c2f0e6253e0f57e88793619f3e83babf186ef4f Mon Sep 17 00:00:00 2001
From: "Chao Li (Evan)" <[email protected]>
Date: Fri, 12 Jun 2026 11:20:19 +0800
Subject: [PATCH v2] Fix ANALYZE reporting after imported foreign-table stats
Commit 28972b6fc added the ImportForeignStatistics FDW callback, which
lets ANALYZE import remote statistics instead of sampling a foreign table.
When that path succeeds, analyze_rel() skips do_analyze_rel(). That also
skips pgstat_report_analyze(), so cumulative ANALYZE stats such as
analyze_count, last_analyze_time, and live_tuples are not updated even
though ANALYZE succeeded.
Fix by having postgres_fdw report ANALYZE to the cumulative stats system
after a successful statistics import. The imported live-row estimate comes
from the remote reltuples value. Since imported relation stats do not
include a dead-tuple estimate, report zero dead tuples for this path.
Add regression coverage for postgres_fdw stats import to verify that a
successful imported ANALYZE updates live_tuples and analyze_count.
Author: Chao Li <[email protected]>
Reported-by: Chao Li <[email protected]>
Reviewed-by: Etsuro Fujita <[email protected]>
Discussion: https://postgr.es/m/[email protected]
---
.../postgres_fdw/expected/postgres_fdw.out | 40 +++++++++++++++++++
contrib/postgres_fdw/postgres_fdw.c | 17 ++++++++
contrib/postgres_fdw/sql/postgres_fdw.sql | 10 +++++
src/backend/commands/analyze.c | 3 +-
4 files changed, 69 insertions(+), 1 deletion(-)
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index 0805c56cb1b..5ebae1cedc2 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -12973,9 +12973,29 @@ ALTER FOREIGN TABLE simport_ftable OPTIONS (ADD restore_stats 'true');
ANALYZE simport_ftable; -- should fail
WARNING: could not import statistics for foreign table "public.simport_ftable" --- remote table "public.simport_table" has no relation statistics to import
ANALYZE simport_table;
+SELECT pg_stat_reset_single_table_counters('public.simport_ftable'::regclass);
+ pg_stat_reset_single_table_counters
+-------------------------------------
+
+(1 row)
+
ANALYZE VERBOSE simport_ftable; -- should work
INFO: importing statistics for foreign table "public.simport_ftable"
INFO: finished importing statistics for foreign table "public.simport_ftable"
+SELECT pg_stat_force_next_flush();
+ pg_stat_force_next_flush
+--------------------------
+
+(1 row)
+
+SELECT pg_stat_get_live_tuples('public.simport_ftable'::regclass),
+ pg_stat_get_dead_tuples('public.simport_ftable'::regclass),
+ pg_stat_get_analyze_count('public.simport_ftable'::regclass);
+ pg_stat_get_live_tuples | pg_stat_get_dead_tuples | pg_stat_get_analyze_count
+-------------------------+-------------------------+---------------------------
+ 0 | 0 | 1
+(1 row)
+
ALTER TABLE simport_table ALTER COLUMN c1 SET STATISTICS 0;
ALTER TABLE simport_table ALTER COLUMN c2 SET STATISTICS 0;
INSERT INTO simport_table VALUES (1, 'foo'), (1, 'foo'), (2, 'bar'), (2, 'bar');
@@ -12988,9 +13008,29 @@ ANALYZE simport_ftable; -- should fail
WARNING: could not import statistics for foreign table "public.simport_ftable" --- no attribute statistics found for column "c2" of remote table "public.simport_table"
ALTER TABLE simport_table ALTER COLUMN c2 SET STATISTICS DEFAULT;
ANALYZE simport_table;
+SELECT pg_stat_reset_single_table_counters('public.simport_ftable'::regclass);
+ pg_stat_reset_single_table_counters
+-------------------------------------
+
+(1 row)
+
ANALYZE VERBOSE simport_ftable; -- should work
INFO: importing statistics for foreign table "public.simport_ftable"
INFO: finished importing statistics for foreign table "public.simport_ftable"
+SELECT pg_stat_force_next_flush();
+ pg_stat_force_next_flush
+--------------------------
+
+(1 row)
+
+SELECT pg_stat_get_live_tuples('public.simport_ftable'::regclass),
+ pg_stat_get_dead_tuples('public.simport_ftable'::regclass),
+ pg_stat_get_analyze_count('public.simport_ftable'::regclass);
+ pg_stat_get_live_tuples | pg_stat_get_dead_tuples | pg_stat_get_analyze_count
+-------------------------+-------------------------+---------------------------
+ 4 | 0 | 1
+(1 row)
+
ANALYZE VERBOSE simport_ftable (c1); -- should work
INFO: importing statistics for foreign table "public.simport_ftable"
INFO: finished importing statistics for foreign table "public.simport_ftable"
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index 6fa45773c30..e5917eb7449 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -41,6 +41,7 @@
#include "optimizer/restrictinfo.h"
#include "optimizer/tlist.h"
#include "parser/parsetree.h"
+#include "pgstat.h"
#include "postgres_fdw.h"
#include "statistics/statistics.h"
#include "storage/latch.h"
@@ -52,6 +53,7 @@
#include "utils/rel.h"
#include "utils/sampling.h"
#include "utils/selfuncs.h"
+#include "utils/timestamp.h"
PG_MODULE_MAGIC_EXT(
.name = "postgres_fdw",
@@ -336,6 +338,8 @@ typedef struct
PGresult *rel;
PGresult *att;
int server_version_num;
+ double livetuples;
+ double deadtuples;
} RemoteStatsResults;
/* Column order in relation stats query */
@@ -5597,6 +5601,7 @@ postgresImportForeignStatistics(Relation relation, List *va_cols, int elevel)
RemoteStatsResults remstats = {.rel = NULL, .att = NULL};
RemoteAttributeMapping *remattrmap = NULL;
int attrcnt = 0;
+ TimestampTz starttime = 0;
bool restore_stats = false;
bool ok = false;
ListCell *lc;
@@ -5656,6 +5661,8 @@ postgresImportForeignStatistics(Relation relation, List *va_cols, int elevel)
(errmsg("importing statistics for foreign table \"%s.%s\"",
schemaname, relname)));
+ starttime = GetCurrentTimestamp();
+
ok = fetch_remote_statistics(relation, va_cols,
table, schemaname, relname,
&attrcnt, &remattrmap, &remstats);
@@ -5665,9 +5672,15 @@ postgresImportForeignStatistics(Relation relation, List *va_cols, int elevel)
attrcnt, remattrmap, &remstats);
if (ok)
+ {
+ pgstat_report_analyze(relation,
+ remstats.livetuples, remstats.deadtuples,
+ (va_cols == NIL), starttime);
+
ereport(elevel,
(errmsg("finished importing statistics for foreign table \"%s.%s\"",
schemaname, relname)));
+ }
PQclear(remstats.rel);
PQclear(remstats.att);
@@ -5807,6 +5820,10 @@ fetch_remote_statistics(Relation relation,
}
}
+ /* We assume that we have no dead tuple. */
+ remstats->deadtuples = 0.0;
+ remstats->livetuples = reltuples;
+
ok = true;
fetch_cleanup:
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
index 8162c5496bf..e868da00ace 100644
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -4587,7 +4587,12 @@ ANALYZE simport_ftable; -- should fail
ANALYZE simport_table;
+SELECT pg_stat_reset_single_table_counters('public.simport_ftable'::regclass);
ANALYZE VERBOSE simport_ftable; -- should work
+SELECT pg_stat_force_next_flush();
+SELECT pg_stat_get_live_tuples('public.simport_ftable'::regclass),
+ pg_stat_get_dead_tuples('public.simport_ftable'::regclass),
+ pg_stat_get_analyze_count('public.simport_ftable'::regclass);
ALTER TABLE simport_table ALTER COLUMN c1 SET STATISTICS 0;
ALTER TABLE simport_table ALTER COLUMN c2 SET STATISTICS 0;
@@ -4604,7 +4609,12 @@ ANALYZE simport_ftable; -- should fail
ALTER TABLE simport_table ALTER COLUMN c2 SET STATISTICS DEFAULT;
ANALYZE simport_table;
+SELECT pg_stat_reset_single_table_counters('public.simport_ftable'::regclass);
ANALYZE VERBOSE simport_ftable; -- should work
+SELECT pg_stat_force_next_flush();
+SELECT pg_stat_get_live_tuples('public.simport_ftable'::regclass),
+ pg_stat_get_dead_tuples('public.simport_ftable'::regclass),
+ pg_stat_get_analyze_count('public.simport_ftable'::regclass);
ANALYZE VERBOSE simport_ftable (c1); -- should work
diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
index f66e80b757c..2edd88087f5 100644
--- a/src/backend/commands/analyze.c
+++ b/src/backend/commands/analyze.c
@@ -230,7 +230,8 @@ analyze_rel(Oid relid, RangeVar *relation,
if (fdwroutine->ImportForeignStatistics != NULL &&
fdwroutine->ImportForeignStatistics(onerel, va_cols, elevel))
stats_imported = true;
- else
+
+ if (!stats_imported)
{
bool ok = false;
--
2.50.1 (Apple Git-155)
view thread (12+ 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], [email protected]
Subject: Re: postgres_fdw: fix cumulative stats after imported foreign-table stats
In-Reply-To: <[email protected]>
* 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