Re: Import Statistics in postgres_fdw before resorting to sampling.

On Fri, Jan 9, 2026 at 4:35 AM Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>
wrote:

> On Wed, Jan 7, 2026 at 11:34 AM Corey Huinker <corey(dot)huinker(at)gmail(dot)com>
> wrote:
>
> >
> > Anyway, here's v8, incorporating the documentation feedback and
> Matheus's notes.
>
> I went through the patches. I have one question: The column names of
> the foreign table on the local server are sorted using qsort, which
> uses C sorting. The column names the result obtained from the foreign
> server are sorted using ORDER BY clause. If the default collation on
> the foreign server is different from the one used by qsort(), the
> merge sort in import_fetched_statistics() may fail. Shouldn't these
> two sorts use the same collation?
>

I wondered about that, but somehow thought that because they're of type
pg_catalog.name rather than text/varchar that they weren't subject to
collation settings. We definitely should explicitly sort by C collation
regardless.

>
> Some minor comments
>
> /* avoid including explain_state.h here */
> typedef struct ExplainState ExplainState;
> -
>
> unintended line deletion?
>

Yeah, must have been.

>
> fetch_remote_statistics() fetches the statistics twice if the first
> attempt fails. I think we need same check after the second attempt as
> well. The second attempt should not fail, but I think we need some
> safety checks and Assert at least, in case the foreign server
> misbehaves.
>

I think the only test *not* done on re-fetch is checking the relkind of the
relation, are you speaking of another check? It's really no big deal to do
that one again, but I made the distinction early on in the coding, and in
retrospect there are relatively few checks we'd consider skipping on the
second pass, so maybe it's not worth the distinction.

Since you're joining the thread, we have an outstanding debate about what
the desired basic workflow should be, and I think we should get some
consensus before we paint ourselves into a corner.

1. The Simplest Possible Model

* There is no remote_analyze functionality
* fetch_stats defaults to false
* Failure to fetch stats results in a failure, no failover to sampling.

2. Simplest Model, but with Failover

* Same as #1, but if we aren't satisfied with the stats we get from the
remote, we issue a WARNING, then fall back to sampling, trusting that the
user will eventually turn off fetch_stats on tables where it isn't working.

3. Analyze and Retry

* Same as #2, but we add remote_analyze option (default false).
* If the first attempt fails AND remote_analyze is set on, then we send the
remote analyze, then retry. Only if that fails do we fall back to sampling.

4. Analyze and Retry, Optimistic

* Same as #3, but fetch_stats defaults to ON, because the worst case
scenario is that we issue a few queries that return 0-1 rows before giving
up and just sampling.
* This is the option that Nathan advocated for in our initial conversation
about the topic, and I found it quite persuasive at the time, but he's been
slammed with other stuff and hasn't been able to add to this thread.

5. Fetch With Retry Or Sample, Optimisitc

* If fetch_stats is on, AND the remote table is seemingly capable of
holding stats, attempt to fetch them, possibly retrying after ANALYZE
depending on remote_analyze.
* If fetching stats failed, just error, as a way to prime the user into
changing the table's setting.
* This is what's currently implemented, and it's not quite what anyone
wants. Defaulting fetch_stats to true doesn't seem great, but not
defaulting it to true will reduce adoption of this feature.

6. Fetch With Retry Or Sample, Pessimistic

* Same as #5, but with fetch_stats = false.