Hi Dinesh, Thanks for the patch. v2 needed rebasing, but I also went ahead and applied my comments to v3.
1/ Forward all user options instead of hardcoding ExplainState booleans The current approach reads individual ExplainState fields to construct the remote EXPLAIN: ``` appendStringInfo(&explain_sql, ", VERBOSE %s", es->verbose ? "TRUE" : "FALSE"); ... ..... ``` After thinking about this a bit, this creates a maintenance problem. Any new non-ANALYZE EXPLAIN option added to core requires someone to remember to update postgres_fdw. I previously suggested a comment in explain_state.h to remind developers, but I think we should do something better. The explain_validate_options_hook already receives the user's original option list. We save it in PgFdwExplainState and use to construct the remote EXPLAIN SQL, skipping only remote_plans and generic_plan. New core EXPLAIN options will automatically be forwarded to the remote when the user specifies them, with no postgres_fdw code change needed. If the remote doesn't recognize an option, it errors clearly rather than silently ignoring it. I tested this against a PG 16 remote. EXPLAIN (REMOTE_PLANS, MEMORY) correctly errors with "unrecognized EXPLAIN option 'memory'" since PG 16 does not have MEMORY. This seems better than silent omission. 2/ I also made some comment improvements. Some were just too verbose. 3/ Documentation improvements The generic plan limitation only matters when the remote SQL contains parameter placeholders. For queries with only literals (e.g., WHERE id = 42), the generic plan is identical to a custom plan. The docs should clarify this rather than making it seem like a general limitation. I also added some concrete examples showing both cases where a literal is used vs a parameter. Attached is my attempt at the above. What do you think? -- Sami Imseih Amazon Web Services (AWS)
v3-0001-postgres_fdw-show-remote-EXPLAIN-plans-via-REMOTE.patch
Description: Binary data
