- There is an established idiom in postgres_fdw for how to extract data from lists of DefElems, and this patch does something else instead. Please make it look like postgresIsForeignRelUpdatable, postgresGetForeignRelSize, and postgresImportForeignSchema instead of inventing something new. (Note that your approach would require multiple passes over the list if you needed information on multiple options, whereas the existing approach does not.)
I will look into that. The original patch pre-dates import foreign schema, so I'm not surprised that I missed the established pattern.
- I think the comment in InitPgFdwOptions() could be reduced to a one-line comment similar to those already present.
Aye.
- I would reduce the debug level on the fetch command to something lower than DEBUG1, or drop it entirely. If you keep it, you need to fix the formatting so that the line breaks look like other ereport() calls.
As I mentioned, I plan to drop it entirely. It was only there to show a reviewer that it works as advertised. There's not much to see without it.
- We could consider folding fetch_size into "Remote Execution Options", but maybe that's too clever.
If you care to explain, I'm listening. Otherwise I'm going forward with the other suggestions you've made.
I would not worry about trying to get this into the regression tests.
