BOOLARRAYOID should be defined to 1000, not 1003 (which clashes against NAMEARRAYOID).
Oops, a bad copy-paste. Thanks for catching that.
I had a more detailed look at the postgres_fdw patch:
1) Having an example of options usable with postgres_fdw would be a good thing to test the FDW API more extensively. What about simply disable_nulls to disable NULL/NOT NULL creation on the tables imported.
2) (Should have noticed that earlier, sorry) I don't see any advantages but only complications in using PQexecParams to launch the queries checking schema existence remotely and fetching back table definitions as we use them only once per import. Why not removing the parameter part and build only plain query strings using StringInfo? This would have the merit to really simplify fetch_remote_tables.
3) If you add an option, let's get rid of PGFDW_IMPORTRESULT_NUMCOLS as the number of result columns would change.
4) Instead of using a double-layer of arrays when processing results, I think that it would make the whole process more readable to have one array for each result column (attname, atttype, atttypmod, attnotnull) initialized each time a new row is processed and then process through them to get the array items. I imagine that a small function doing the array parsing called on each array result would bring more readability as well to the process flow.
5) This could be costly with large sets of tables in LIMIT TO... I imagine that we can live with this O(n log(n)) process as LIMIT TO should not get big..
What do you think should be documented, and where ?
Two things coming to my mind:
- If postgres_fdw can use options with IMPORT, they should be explicitly listed in the section "FDW Options of postgres_fdw".
- Documenting that postgres_fdw support IMPORT FOREIGN SCHEMA. A simple paragraph in the main section of postgres_fdw containing descriptions would be enough.
Once we get that done, I'll do another round of review on this patch and I think that we will be able to mark it as ready for committer.
