[PG19][PATCH] Make postgres_fdw statistics import atomic

Started by Nikolay Samokhvalov2 days ago6 messageshackers
Beta feature

Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.

appliessuccessCI history

You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:

docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t253799
psql -h localhost -U postgres

Built from patchset v1 (message #1), September 17, 2026 at 10:00 AM.

Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:

git clone --branch t253799_1 https://github.com/hackorum-dev/postgres.git

In a checkout you already have, add the fork once:

git remote add hackorum https://github.com/hackorum-dev/postgres.git

then, for this patchset and every later one:

git fetch hackorum t253799_1 && git checkout t253799_1

Patchset v1 (message #1) is on t253799_1

Jump to latest
#1Nikolay Samokhvalov
samokhvalov@gmail.com

Hi hackers,

postgres_fdw can leave partial pg_statistic changes behind when statistics
import fails and analyze falls back to sampling.

My AI correctness harness found this while I was testing new PG19 features.
It also prepared and tested the attached patch. I have not fully reviewed
that patch by hand because I am testing many PG19 areas in parallel. I
nevertheless think it is useful to post: I mostly trust this harness, and
the exact reproducer and patch passed its independent execution and review
gates. Please treat the patch as AI-prepared and review it in the usual way.

import_fetched_statistics() deletes and updates one attribute at a time.
attribute_statistics_update_internal() can update a partial row and return
false after a conversion warning. Earlier attributes have already been
updated too. If the fallback sample is empty, analyze does not replace
attribute statistics, so those changes commit.

Here is a complete reproducer. Run it with psql as a superuser against a
fresh PG19 server that accepts a loopback connection on its Unix socket:

create extension postgres_fdw;
create table remote_t (a, b) as values (11, '21'::text), (11, '21');
analyze remote_t;

create server loopback foreign data wrapper postgres_fdw
options (host :'HOST', port :'PORT', dbname :'DBNAME');
create user mapping for current_user server loopback options (user :'USER');
create foreign table ft (a int, b int) server loopback
options (table_name 'remote_t', import_stats 'true');
analyze ft;

update remote_t set a = 111, b = 'bad-x';
analyze remote_t;
delete from remote_t;
analyze ft;

select attname, coalesce(most_common_vals::text, 'NULL') as mcv
from pg_stats
where tablename = 'ft'
order by attname;

On REL_19_STABLE at e7c1b57012b the decisive output is below (psql's file
and line prefixes are omitted):

  WARNING:  invalid input syntax for type integer: "bad-x"
  WARNING:  could not import statistics for foreign table "public.ft"
--- attribute statistics import failed for column "b" of this foreign
table
   attname |  mcv
  ---------+-------
   a       | {111}
   b       | NULL
  (2 rows)

The attached patch runs the local catalog import in an internal
subtransaction. It releases the subtransaction on success and rolls it
back when the import returns false. Errors are rolled back and rethrown.
The remote fetch stays outside the subtransaction.

The new regression test fails without the code change with the same partial
update. With the patch, the old coherent statistics survive:

attname | mcv
---------+------
a | {11}
b | {21}
(2 rows)

postgres_fdw regression and isolation tests pass. I also applied the exact
attachment to a clean REL_19_STABLE checkout and reran both suites.

Nik

Attachments:

t253799_1
0001-postgres_fdw-Make-statistics-import-atomic.patchapplication/octet-stream; name=0001-postgres_fdw-Make-statistics-import-atomic.patchDownload+104-3
#2Nathan Bossart
nathandbossart@gmail.com
In reply to: Nikolay Samokhvalov (#1)
Re: [PG19][PATCH] Make postgres_fdw statistics import atomic

[RMT hat]

On Tue, Sep 15, 2026 at 02:36:14AM -0700, Nikolay Samokhvalov wrote:

postgres_fdw can leave partial pg_statistic changes behind when statistics
import fails and analyze falls back to sampling.

Corey and Fujita-san: this is listed as an open item for v19. Please
provide an update on its status as soon as you're able.

--
nathan

#3Corey Huinker
corey.huinker@gmail.com
In reply to: Nikolay Samokhvalov (#1)
Re: [PG19][PATCH] Make postgres_fdw statistics import atomic

On Tue, Sep 15, 2026 at 5:36 AM Nikolay Samokhvalov <nik@postgres.ai> wrote:

import_fetched_statistics() deletes and updates one attribute at a time.

attribute_statistics_update_internal() can update a partial row and return
false after a conversion warning. Earlier attributes have already been
updated too. If the fallback sample is empty, analyze does not replace
attribute statistics, so those changes commit.

So this situation happens when a remote table has pg_stats statistics, but
has no underlying data to sample. There are two ways that can happen:

1) The table was populated, analyzed, and then the rows were removed via
delete/truncate.
2) The table was not populated since last analysis or creation, but someone
used statistics import functions on it.

Here is a complete reproducer. Run it with psql as a superuser against a
fresh PG19 server that accepts a loopback connection on its Unix socket:

create extension postgres_fdw;
create table remote_t (a, b) as values (11, '21'::text), (11, '21');

Text field on source table.

analyze remote_t;

create server loopback foreign data wrapper postgres_fdw
options (host :'HOST', port :'PORT', dbname :'DBNAME');
create user mapping for current_user server loopback options (user
:'USER');
create foreign table ft (a int, b int) server loopback

options (table_name 'remote_t', import_stats 'true');

Integer field on destination table. Which means that if 'bad-x' was a
potential value that could have been sent over the wire for processing by
the FDW table and we did a regular query:

# update remote_t set a = 111, b = 'bad-x';
UPDATE 2
# SELECT * FROM ft;
ERROR: invalid input syntax for type integer: "bad-x"
CONTEXT: column "b" of foreign table "ft"

So already we're dealing with a very brittle situation. Not only is there a
type mismatch in the "b" columns that can cause ordinary queries to fail,
but we've shown that situation can and does happen.

As is, this will fail in production until someone remote comes along and
deletes the bad-x row, and the situation can re-occur until someone local
redefines or removes the "b" column.

update remote_t set a = 111, b = 'bad-x';
analyze remote_t;
delete from remote_t;

So how would a table that didn't have stats import have handled this?

# create foreign table ft2 (a int, b int) server loopback options
(table_name 'remote_t');
CREATE FOREIGN TABLE
# analyze ft2;
ERROR: invalid input syntax for type integer: "bad-x"
CONTEXT: column "b" of foreign table "ft2"

And how would the first foreign table handle this if we fixed the records
in a way that didn't also leave the table empty?

# UPDATE remote_t SET b = 4;
UPDATE 2
# analyze ft;
WARNING: invalid input syntax for type integer: "bad-x"
WARNING: could not import statistics for foreign table "public.ft" ---
attribute statistics import failed for column "b" of this foreign table
ANALYZE

# SELECT attname, most_common_vals FROM pg_stats WHERE tablename = 'ft';
attname | most_common_vals
---------+------------------
a | {111}
b | {4}
(2 rows)

So fallback to sampling works as intended, but not if the remote table was
empty.

So this problem occurs only when a foreign table is mis-configured in such
a way as to create a conversion error on the inputs from the remote table's
stats, this went undiscovered (i.e. nobody queried the foreign table)
before the foreign table was analyzed, AND the remote table has been
emptied since it was analyzed but not yet re-analyzed at the time when the
remote table was analyzed.

And the negative consequence of this is that a query plan will assume that
the table contains rows when it actually does not. That seems like a
problem we already have in this situation.

CREATE TABLE ihaverows (x int);
INSERT INTO ihaverows SELECT g.g FROM generate_series(1,100000) AS g;
ANALYZE ihaverows;
DELETE FROM ihaverows;
SELECT relname, relpages, reltuples FROM pg_class where relname =
'ihaverows';

relname | relpages | reltuples
-----------+----------+-----------
ihaverows | 443 | 100000
(1 row)

So the impact is that until the remote table is re-analyzed and then the
foreign table is re-analyzed, we get over-estimates on an empty table. In a
real-world situation that table would be repopulated and reanalyzed fairly
quickly to restore business operations, and the bad stats issue corrects
itself, exactly as would happen if we truncated and reloaded a local table.
I'd call that low-risk, low-impact.

Furthermore, It's pretty reasonable to assume that the table will be
re-loaded with data shortly, in which case the "bad" stats and "bad" row
estimate would then be more accurate for the table which is populated but
not yet re-analyzed than would stale stats showing a table to be empty when
it actually has lots of rows. I mention this because this is the exact
scenario that caused me to want to create statistics import functions in
the first place: A partitioned table is being queried
frequently/continuously by an application. An ETL is loading millions of
rows into a partition that was empty just minutes ago. The stats for the
partition reported it as empty, so the query planner said because the
partition was empty, a table scan was the cheapest plan....oops. In this
situation (on an Oracle database) we determined that injecting the stats
from a populated partition into the empty partiion, while all wrong on
histograms, gave a better row estimate than the collected stats (0 rows)
and we knew the ETL would re-analyze the partition when it was done.

As for the C patch itself, it seems overengineered. It would be easier to
enhance the failure case when calling import_attribute_statistics to make a
note of where it was in the attribute loop, then walk backward calling
delete_pg_statistic() on the attributes that we had set before the failure.
But we shouldn't need to do that, as do_analyze_rel() should already handle
cleaning up pg_statistics rows for a table that used to have rows but now
doesn't....so I'd need to dig more to determine if this corner case only
happens if the foreign table had never had a successful analyze before the
bad data was inserted, but this dive is probably already deeper than most
people wanted, so I'm going to take a decompression stop here.

Summary: I can't see how this would happen at all in a foreign table that
correctly represented the remote data. Even then, it only happens if the
table is analyzed in a specific middle step of remediating the remote data,
and during that time the impact is low (no data loss, no incorrect
queries), and would resolve itself after the expected remediation. If this
needs a fix at all, it's probably not unique to postgres_fdw, so I
speculate that we'd want to teach do_analyze_rel() to know that a table
that analyzed as empty, but had an attempted remote stats fetch will need
to be zeroed out even if it already looks zeroed out.

#4Nikolay Samokhvalov
samokhvalov@gmail.com
In reply to: Corey Huinker (#3)
Re: [PG19][PATCH] Make postgres_fdw statistics import atomic

Thanks Corey. Shouldn't an empty fallback preserve the old
pg_statistic rows, as analyze normally does? Here a={11}, b={21}
becomes a={111}, b=NULL; deleting the processed rows would not restore
the old state. Nik

#5Etsuro Fujita
fujita.etsuro@lab.ntt.co.jp
In reply to: Nikolay Samokhvalov (#4)
Re: [PG19][PATCH] Make postgres_fdw statistics import atomic

On Thu, Sep 17, 2026 at 8:24 AM Nikolay Samokhvalov <nik@postgres.ai> wrote:

Thanks Corey. Shouldn't an empty fallback preserve the old
pg_statistic rows, as analyze normally does?

I don't think so, because if the fallback sample is empty, we have
reltuples=0 in pg_class, meaning that any attribute stats are
effectively ignored in planning.

Also, I think the scenario you showed upthread is not supported, or at
least not recommended:

* You declared the type of a column of the foreign table differently
from the remote table, but that isn't recommended, as noted in the
documentation: "It is generally recommended that the columns of a
foreign table be declared with exactly the same data types, and
collations if applicable, as the referenced columns of the remote
table..."

* You imported remote stats without re-analyzing the remote table
after the delete operation, but that isn't supported, as noted in the
documentation; "When using this option, it is the user's
responsibility to ensure that the existing statistics for the remote
table are up-to-date."

And I think any surprising behavior arising from such a use is the
user's fault rather than the system's fault.

Thanks for the testing!

Best regards,
Etsuro Fujita

#6Nikolay Samokhvalov
samokhvalov@gmail.com
In reply to: Etsuro Fujita (#5)
Re: [PG19][PATCH] Make postgres_fdw statistics import atomic

On Thu, Sep 17, 2026, Etsuro Fujita <etsuro.fujita@gmail.com> wrote:

* You imported remote stats without re-analyzing the remote table
after the delete operation, but that isn't supported, as noted in the
documentation; "When using this option, it is the user's
responsibility to ensure that the existing statistics for the remote
table are up-to-date."

Thanks Etsuro. Agreed, my reproducer violates that requirement.
I haven't reproduced this with matching types and current remote
stats, so I'll withdraw the patch for now.

Attribute stats can still affect join estimates when reltuples=0;
I tested that. But that doesn't show this needs fixing.

Nik