Re: Import Statistics in postgres_fdw before resorting to sampling. - Mailing list pgsql-hackers
| From | Corey Huinker |
|---|---|
| Subject | Re: Import Statistics in postgres_fdw before resorting to sampling. |
| Date | |
| Msg-id | CADkLM=ce0qES4suTDaTmjLXekGcuNwirYBNFDFb-iY8fX8-xpA@mail.gmail.com Whole thread Raw |
| In response to | Re: Import Statistics in postgres_fdw before resorting to sampling. (Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>) |
| List | pgsql-hackers |
On Fri, Jan 9, 2026 at 4:35 AM Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> wrote:
On Wed, Jan 7, 2026 at 11:34 AM Corey Huinker <corey.huinker@gmail.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.
* 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.
pgsql-hackers by date: