Horton

Re: Import Statistics in postgres_fdw before resorting to sampling.

Chao Li <li.evan.chao@gmail.com>

View thread
From: Chao Li <li.evan.chao@gmail.com>
To: Corey Huinker <corey.huinker@gmail.com>
Cc: Etsuro Fujita <etsuro.fujita@gmail.com>, Michael Paquier <michael@paquier.xyz>, Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>, pgsql-hackers@postgresql.org
Date: 2025-11-24T09:25:13Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Add support for importing statistics from remote servers.


> On Nov 23, 2025, at 15:27, Corey Huinker <corey.huinker@gmail.com> wrote:
> 
>   My apologies for the delayed response.
> 
> Valuable responses are worth waiting for.
> 
> I've reorganized some things a bit, mostly to make resource cleanup simpler. 
> <v4-0001-Add-remote-statistics-fetching-to-postgres_fdw.patch>


Few comments:

1 - commit message
```
effort and the user is better of setting fetch_stats to false for that
```

I guess “of” should be “off”

2 - postgres-fdw.sgml
```
+       server, determines wheter an <command>ANALYZE</command> on a foreign
```

Typo: wheter => whether 

3 - postgres-fdw.sgml
```
+       data sampling if no statistics were availble. This option is only
```

Typo: availble => available

4 - option.c
```
+		/* fetch_size is available on both server and table */
+		{"fetch_stats", ForeignServerRelationId, false},
+		{"fetch_stats", ForeignTableRelationId, false},
```

In the comment, I guess fetch_size should be fetch_stats.

5 - analyze.c
```
+			 * XXX: Should this be it's own fetch type? If not, then there might be
```

Typo: “it’s own” => “its own”

6 - analyze.c
```
+				case FDW_IMPORT_STATS_NOTFOUND:
+					ereport(INFO,
+							errmsg("Found no remote statistics for \"%s\"",
+								   RelationGetRelationName(onerel)));
```

`Found no remote statistics for \"%s\””` could be rephrased as `""No remote statistics found for foreign table \"%s\””`, sounds better wording in server log.

Also, I wonder if this message at INFO level is too noisy?

7 - postgres_fdw.c
```
+		default:
+			ereport(INFO,
+					errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+					errmsg("Remote table %s.%s does not support statistics.",
+						   quote_identifier(remote_schemaname),
+						   quote_identifier(remote_relname)),
+					errdetail("Remote relation if of relkind \"%c\"", relkind));
```

I think “if of” should be “is of”.

8 - postgres_fdw.c
```
+						errmsg("Attribute statistics found for %s.%s but no columns matched",
+							   quote_identifier(schemaname),
+							   quote_identifier(relname))));
```

Instead of using "%s.%s” and calling quote_identifier() twice, there is a simple function to use: quote_qualified_identifier(schemaname, relname).

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/