Thread

Re: explain plans for foreign servers

Sami Imseih <samimseih@gmail.com> — 2025-12-09T21:08:08Z

Hi,

I spent more time reviewing this patch. Here are additional comments.

1/ Remove unnecessary includes

#include "commands/explain_state.h" from option.c.
#include "utils/json.h" from postgres_fdw.c.

2/ Add new EXPLAIN options

MEMORY and SUMMARY flags added to the remote EXPLAIN query formatting.
These do not require ANALYZE to be used.

A larger question is how would we want to ensure that new core EXPLAIN
options can be automatically set?

This is not quite common, but perhaps it is a good thing to add a comment
near ExplainState explaining that if a new option is added, make sure
that postgre_fdw remote_plans are updated.

```
typedef struct ExplainState
{
  StringInfo  str;      /* output buffer */
  /* options */
  bool    verbose;    /* be verbose */
  bool    analyze;    /* print actual times */
  bool    costs;      /* print estimated co

```

3/ Removed unnecessary pstrdup() when appending remote plan rows to
explain_plan.

Removed unnecessary pstrdup() when appending remote plan rows to explain_plan.

```
+                       appendStringInfo(&explain->explain_plan,
"%s\n", pstrdup(PQgetvalue(res, i, 0)));
```


4/ Simplify foreign table OID handling in postgresExplainForeignScan

I am not sure why we need a list that can only hold a single value.
Can we just use an Oid variable to store this?


5/ Encapsulates getting the connection, executing the remote EXPLAIN,
and releasing the connection.

Replaces repeated code in postgresExplainForeignScan,
postgresExplainForeignModify, and postgresExplainDirectModify.

For #4 and #5, attached is my attempt to simplify these routines. What
do you think?

6/ Updated typedefs.list

... to include PgFdwExplainRemotePlans and PgFdwExplainState.


7/ Tests

I quickly skimmed the tests, but I did not see a join push-down test.
We should add
that.


--
Sami Imseih
Amazon Web Services (AWS)