Re: IMPORT FOREIGN SCHEMA statement

On Thu, Jun 19, 2014 at 11:00 PM, Michael Paquier <michael(dot)paquier(at)gmail(dot)com
> wrote:

> >> This seems to be related to re-using the table-name between
> invocations. The
> >> attached patch should fix point 2. As for point 1, I don't know the
> cause for
> >> it. Do you have a reproducible test case ?
> > Sure. I'll try harder when looking in more details at the patch for
> > postgres_fdw :)
> With v2, I have tried randomly some of the scenarios of v1 plus some
> extras, but was not able to reproduce it.
>
> > I'll look into the patch for postgres_fdw later.
> And here are some comments about it, when applied on top of the other 2
> patches.
> 1) Code compiles without warning, regression tests pass.
> 2) In fetch_remote_tables, the palloc for the parameters should be
> done after the number of parameters is determined. In the case of
> IMPORT_ALL, some memory is wasted for nothing.
> 3) Current code is not able to import default expressions for a table.
> A little bit of processing with pg_get_expr would be necessary:
> select pg_catalog.pg_get_expr(adbin, adrelid) from pg_attrdef;
> There are of course bonus cases like SERIAL columns coming immediately
> to my mind but it would be possible to determine if a given column is
> serial with pg_get_serial_sequence.
> This would be a good addition for the FDW IMO.
> 4) The same applies of course to the constraint name: CREATE FOREIGN
> TABLE foobar (a int CONSTRAINT toto NOT NULL) for example.
> 5) A bonus idea: enable default expression obtention and null/not null
> switch by default but add options to disable their respective
> obtention.
> 6) Defining once PGFDW_IMPORTRESULT_NUMCOLS at the top of
> postgres_fdw.c without undefining would be perfectly fine.
> 7) In postgresImportForeignSchema, the palloc calls and the
> definitions of the variables used to save the results should be done
> within the for loop.
> 8) At quick glance, the logic of postgresImportForeignSchema looks
> awkward... I'll have a second look with a fresher mind later on this
> one.
>
While having a second look at the core patch, I have found myself
re-hacking it, fixing a couple of bugs and adding things that have been
missing in the former implementation:
- Deletions of unnecessary structures to simplify code and make it cleaner
- Fixed a bug related to the management of local schema name. A FDW was
free to set the schema name where local tables are created, this should not
be the case.
- Improved documentation, examples and other things, fixed doc padding for
example
- Added some missing stuff in equalfuncs.c and copyfuncs.c
- Some other things.
With that, core patch looks pretty nice actually, and I think that we
should let a committer have a look at this part at least for this CF.

Also, the postgres_fdw portion has been updated based on the previous core
patch modified, using a version that Ronan sent me, which has addressed the
remarks I sent before. This patch is still lacking documentation, some
cleanup, and regression tests are broken, but it can be used to test the
core feature. I unfortunately don't have much time today but I am sending
this patch either way to let people play with IMPORT SCHEMA if I don't come
back to it before.
Regards,
--
Michael