Re: IMPORT FOREIGN SCHEMA statement

On Mon, Jun 30, 2014 at 6:01 PM, Ronan Dunklau <ronan(dot)dunklau(at)dalibo(dot)com>
wrote:

> 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..
foreach(lc1, table_names)
{
found = false;
looked_up = ((RangeVar *) lfirst(lc1))->relname;
foreach(lc2, tables)

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.

Regards,
--
Michael