Re: [Review] inherit support for foreign tables - Mailing list pgsql-hackers

Hi Fujita-san,

Thanks for the review.

2014/1/23 Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp>:
> Shigeru Hanada wrote:
>> Though this would be debatable, in current implementation, constraints
>> defined on a foreign table (now only NOT NULL and CHECK are supported)
>> are not enforced during INSERT or UPDATE executed against foreign
>> tables. This means that retrieved data might violates the constraints
>> defined on local side. This is debatable issue because integrity of
>> data is important for DBMS, but in the first cut, it is just
>> documented as a note.
>
> I agree with you, but we should introduce a new keyword such as
> ASSERTIVE or something in 9.4, in preparation for the enforcement of
> constraints on the local side in a future release? What I'm concerned
> about is, otherwise, users will have to rewrite those DDL queries at
> that point. No?

In the original thread, whether the new syntax is necessary (maybe
ASSERTIVE will not be used though) is under discussion.  Anyway, your
point, decrease user's DDL rewrite less as possible is important.
Could you post review result and/or your opinion as the reply to the
original thread, then we include other people interested in this
feature can share discussion.

>> Because I don't see practical case to have a foreign table as a
>> parent, and it avoid a problem about recursive ALTER TABLE operation,
>> foreign tables can't be a parent.
>
> I agree with you on that point.
>
>> For other commands recursively processed such as ANALYZE, foreign
>> tables in the leaf of inheritance tree are ignored.
>
> I'm not sure that in the processing of the ANALYZE command, we should
> ignore child foreign tables. It seems to me that the recursive
> processing is not that hard. Rather what I'm concerned about is that if
> the recursive processing is allowed, then autovacuum will probably have
> to access to forign tables on the far side in order to acquire those
> sample rows. It might be undesirable from the viewpoint of security or
> from the viewpoint of efficiency.

As you say, it's not difficult to do recursive ANALYZE including
foreign tables.  The reason why ANALYZE ignores descendant foreign
tables is consistent behavior.  At the moment, ANALYZE ignores foreign
tables in top-level (non-child) when no table name was given, and if
we want to ANALYZE foreign tables we need to specify explicitly.

I think it's not so bad to show WARNING or INFO message about foreign
table ignorance, including which is not a child.

> --- a/doc/src/sgml/ref/create_foreign_table.sgml
> +++ b/doc/src/sgml/ref/create_foreign_table.sgml
> @@ -22,6 +22,7 @@ CREATE FOREIGN TABLE [ IF NOT EXISTS ] <replaceable
> class="PARAMETER">table_name
> <replaceable class="PARAMETER">column_name</replaceable> <replaceable
> class="PARAMETER">data_type</replaceable> [ OPTIONS ( <replaceable
> class="PA\
> RAMETER">option</replaceable> '<replaceable
> class="PARAMETER">value</replaceable>' [, ... ] ) ] [ COLLATE
> <replaceable>collation</replaceable> ] [ <rep\
> laceable class="PARAMETER">column_constraint</replaceable> [ ... ] ]
> [, ... ]
> ] )
> +[ INHERITS ( <replaceable>parent_table</replaceable> [, ... ] ) ]
> SERVER <replaceable class="parameter">server_name</replaceable>
> [ OPTIONS ( <replaceable class="PARAMETER">option</replaceable>
> '<replaceable class="PARAMETER">value</replaceable>' [, ... ] ) ]
>
> @@ -159,6 +160,17 @@ CREATE FOREIGN TABLE [ IF NOT EXISTS ] <replaceable
> class="PARAMETER">table_name
> </varlistentry>
>
> <varlistentry>
> + <term><replaceable class="PARAMETER">parent_table</replaceable></term>
> + <listitem>
> + <para>
> + The name of an existing table or foreign table from which the new foreign
> + table automatically inherits all columns, see
> + <xref linkend="ddl-inherit"> for details of table inheritance.
> + </para>
> + </listitem>
> + </varlistentry>
>
> Correct? I think we should not allow a foreign table to be a parent.

Oops, good catch.
-- 
Shigeru HANADA