[Review] inherit support for foreign tables

Shigeru Hanada wrote:
> Attached patch allows a foreign table to be a child of a table. It
> also allows foreign tables to have CHECK constraints.

Sorry for the delay. I started to look at this patch.

> 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?

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

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

I'll look at this patch more closely.

Thanks,

Best regards,
Etsuro Fujita