On Mon, Apr 27, 2015 at 4:20 PM, Etsuro Fujita
<fujita.etsuro@lab.ntt.co.jp> wrote:
> On 2015/04/27 15:51, Michael Paquier wrote:
>>
>> On Mon, Apr 27, 2015 at 3:15 PM, Etsuro Fujita
>> <fujita.etsuro@lab.ntt.co.jp> wrote:
>>>
>>> I noticed that there is no postgres_fdw option to control whether check
>>> constraints on remote tables are included in the definitions of foreign
>>> tables imported from a remote PG server when performing IMPORT FOREIGN
>>> SCHEMA, while we now allow check constraints on foreign tables.
>
>
>> I guess that the addition of this option makes sense, but I think that
>> this patch is doing it wrong by using ALTER FOREIGN TABLE and by
>> changing the queries authorized in ImportForeignSchema(). The point of
>> IMPORT FOREIGN SCHEMA is to authorize only CREATE FOREIGN TABLE and
>> not other types of queries, not to mention that CREATE FOREIGN TABLE
>> supports the definitions of constraints at table and column-level.
>
>
> I don't think so. IMO, I think it'd be more convenient and flexible to
> allow ALTER FOREIGN TABLE for creating foreign table definitions during
> ImportForeignSchema().
Authorizing ALTER FOREIGN TABLE as query string that a FDW can use
with IMPORT FOREIGN SCHEMA is a different feature than what is
proposed in this patch, aka an option for postgres_fdw and meritates a
discussion on its own because it impacts all the FDWs and not only
postgres_fdw. Now, related to this patch, we could live without
authorizing ALTER FOREIGN TABLE because CREATE FOREIGN TABLE does
authorize the definition of CHECK constraints. Also, I imagine that it
would be more natural to combine the fetch of the constraint
expression in the existing query with a join on conrelid so as to not
replicate the logic that your patch is doing with
FDW_IMPORT_SCHEMA_LIMIT_TO and FDW_IMPORT_SCHEMA_EXCEPT that reuses
the same logic to re-build the [ NOT ] IN clause.
--
Michael