Re: explain plans for foreign servers

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)