Re: IMPORT FOREIGN SCHEMA statement

From: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
To: Ronan Dunklau <ronan(dot)dunklau(at)dalibo(dot)com>
Cc: PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: IMPORT FOREIGN SCHEMA statement
Date: 2014-06-19 14:00:35
Message-ID: CAB7nPqSn4__BfBZtJ4JdWvqMBx31ALYxrx5mxgiL4i+WttFcNA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

>> 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.
--
Michael

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Kevin Grittner 2014-06-19 14:11:14 Re: calculation for NUM_FIXED_LWLOCKS
Previous Message Fujii Masao 2014-06-19 13:52:04 Re: pg_receivexlog add synchronous mode