> I have refactored the commit on the latest version of PG and added a few more 
> tests.

Thanks for the update!

> To simplify the roll out of this feature, I decided to work on analyze=false 
> use case first.

I did not go through the entire patch yet, but a few things stood out
from my first pass.

1/
RegisterExtensionExplainOption is called during _PG_init, which is fine, but I
also wonder if we can call this during postgresExplainForeignScan as well?

The reason being is for _PG_init to be invoked, the user must load postgres_fdw
(LOAD, session_preload_libraries, shared_preload_libraries), which from my
experience is not very common in postgres_fdw. Users ordinarily just
"CREATE EXTENSION..."

So this needs to be documented [0]

2/

Does this behave sanely with multiple fdw connections? Can we add
tests for this?

+
+                               /*
+                                * add one of the tables to
foreign_scan_table to get the
+                                * serverId for remote plans
+                                */
+                               if (list_length(foreign_scan_table) == 0)
+                                       foreign_scan_table =
lappend_oid(foreign_scan_table, rte->relid);
+

[0] https://www.postgresql.org/docs/current/postgres-fdw.html

--
Sami Imseih
Amazon Web Services (AWS)


Reply via email to