Hi Hanada-san,

Sorry for the delay.

(2014/01/30 14:01), Shigeru Hanada wrote:
2014-01-27 Etsuro Fujita <fujita.ets...@lab.ntt.co.jp>:
While still reviwing this patch, I feel this patch has given enough
consideration to interactions with other commands, but found the following
incorrect? behabior:

postgres=# CREATE TABLE product (id INTEGER, description TEXT);
CREATE TABLE
postgres=# CREATE FOREIGN TABLE product1 () INHERITS (product) SERVER fs
OPTIONS (filename '/home/foo/product1.csv', format 'csv');
CREATE FOREIGN TABLE
postgres=# ALTER TABLE product ALTER COLUMN description SET STORAGE
EXTERNAL;
ERROR:  "product1" is not a table or materialized view

ISTN the ALTER TABLE simple recursion mechanism (ie ATSimpleRecursion())
should be modified for the ALTER COLUMN SET STORAGE case.

It seems little bit complex than I expected.  Currently foreign tables
deny ALTER TABLE SET STORAGE with message like below, because foreign
tables don't have storage in the meaning of PG heap tables.

      ERROR:  "pgbench1_accounts_c1" is not a table or materialized view

At the moment we don't use attstorage for foreign tables, so allowing
SET STORAGE against foreign tables never introduce visible change
except \d+ output of foreign tables.  But IMO such operation should
not allowed because users would be confused.  So I changed
ATExecSetStorage() to skip on foreign tables.  This allows us to emit
ALTER TABLE SET STORAGE against ordinary tables in upper level of
inheritance tree, but it have effect on only ordinary tables in the
tree.

This also allows direct ALTER FOREIGN TABLE SET STORAGE against
foreign table but the command is silently ignored.  SET STORAGE
support for foreign tables is not documented because it may confuse
users.

With attached patch, SET STORAGE against wrong relations produces
message like this.  Is it confusing to mention foreign table here?

ERROR:  "pgbench1_accounts_pkey" is not a table, materialized view, or
foreign table

ITSM that would be confusing to users. So, I've modified the patch so that we continue to disallow SET STORAGE on a foreign table *in the same manner as before*, but, as your patch does, allow it on an inheritance hierarchy that contains foreign tables, with the semantics that we quietly ignore the foreign tables and apply the operation to the plain tables, by modifying the ALTER TABLE simple recursion mechanism. Attached is the updated version of the patch.

I'll continue the patch review a bit more, though the patch looks good generally to me except for the abobe issue and the way that the ANALYZE command works. For the latter, if there are no objections, I'll merge the ANALYZE patch [1] with your patch.

Thanks,

[1] http://www.postgresql.org/message-id/52eb10ac.4070...@lab.ntt.co.jp

Best regards,
Etsuro Fujita
*** a/doc/src/sgml/ddl.sgml
--- b/doc/src/sgml/ddl.sgml
***************
*** 258,263 **** CREATE TABLE products (
--- 258,274 ----
     even if the value came from the default value definition.
    </para>
  
+   <note>
+    <para>
+     Note that constraints can be defined on foreign tables too, but such
+     constraints are not enforced on insert or update.  Those constraints are
+     "assertive", and work only to tell planner that some kind of optimization
+     such as constraint exclusion can be considerd.  This seems useless, but
+     allows us to use foriegn table as child table (see
+     <xref linkend="ddl-inherit">) to off-load to multiple servers.
+    </para>
+   </note>
+ 
    <sect2 id="ddl-constraints-check-constraints">
     <title>Check Constraints</title>
  
***************
*** 2021,2028 **** CREATE TABLE capitals (
    </para>
  
    <para>
!    In <productname>PostgreSQL</productname>, a table can inherit from
!    zero or more other tables, and a query can reference either all
     rows of a table or all rows of a table plus all of its descendant tables.
     The latter behavior is the default.
     For example, the following query finds the names of all cities,
--- 2032,2039 ----
    </para>
  
    <para>
!    In <productname>PostgreSQL</productname>, a table or foreign table can
!    inherit from zero or more other tables, and a query can reference either 
all
     rows of a table or all rows of a table plus all of its descendant tables.
     The latter behavior is the default.
     For example, the following query finds the names of all cities,
*** a/doc/src/sgml/ref/alter_foreign_table.sgml
--- b/doc/src/sgml/ref/alter_foreign_table.sgml
***************
*** 42,47 **** ALTER FOREIGN TABLE [ IF EXISTS ] <replaceable 
class="PARAMETER">name</replaceab
--- 42,49 ----
      ALTER [ COLUMN ] <replaceable class="PARAMETER">column_name</replaceable> 
SET ( <replaceable class="PARAMETER">attribute_option</replaceable> = 
<replaceable class="PARAMETER">value</replaceable> [, ... ] )
      ALTER [ COLUMN ] <replaceable class="PARAMETER">column_name</replaceable> 
RESET ( <replaceable class="PARAMETER">attribute_option</replaceable> [, ... ] )
      ALTER [ COLUMN ] <replaceable class="PARAMETER">column_name</replaceable> 
OPTIONS ( [ ADD | SET | DROP ] <replaceable 
class="PARAMETER">option</replaceable> ['<replaceable 
class="PARAMETER">value</replaceable>'] [, ... ])
+     INHERIT <replaceable class="PARAMETER">parent_table</replaceable>
+     NO INHERIT <replaceable class="PARAMETER">parent_table</replaceable>
      OWNER TO <replaceable class="PARAMETER">new_owner</replaceable>
      OPTIONS ( [ ADD | SET | DROP ] <replaceable 
class="PARAMETER">option</replaceable> ['<replaceable 
class="PARAMETER">value</replaceable>'] [, ... ])
  </synopsis>
***************
*** 178,183 **** ALTER FOREIGN TABLE [ IF EXISTS ] <replaceable 
class="PARAMETER">name</replaceab
--- 180,205 ----
     </varlistentry>
  
     <varlistentry>
+     <term><literal>INHERIT <replaceable 
class="PARAMETER">parent_table</replaceable></literal></term>
+     <listitem>
+      <para>
+       This form adds the target foreign table as a new child of the specified
+       parent table.
+      </para>
+     </listitem>
+    </varlistentry>
+ 
+    <varlistentry>
+     <term><literal>NO INHERIT <replaceable 
class="PARAMETER">parent_table</replaceable></literal></term>
+     <listitem>
+      <para>
+       This form removes the target foreign table from the list of children of
+       the specified parent table.
+      </para>
+     </listitem>
+    </varlistentry>
+ 
+    <varlistentry>
      <term><literal>OPTIONS ( [ ADD | SET | DROP ] <replaceable 
class="PARAMETER">option</replaceable> ['<replaceable 
class="PARAMETER">value</replaceable>'] [, ... ] )</literal></term>
      <listitem>
       <para>
***************
*** 306,311 **** ALTER FOREIGN TABLE [ IF EXISTS ] <replaceable 
class="PARAMETER">name</replaceab
--- 328,342 ----
         </para>
        </listitem>
       </varlistentry>
+ 
+      <varlistentry>
+       <term><replaceable class="PARAMETER">parent_name</replaceable></term>
+       <listitem>
+        <para>
+         A parent table to associate or de-associate with this foreign table.
+        </para>
+       </listitem>
+      </varlistentry>
      </variablelist>
   </refsect1>
  
*** a/doc/src/sgml/ref/create_foreign_table.sgml
--- b/doc/src/sgml/ref/create_foreign_table.sgml
***************
*** 22,27 **** CREATE FOREIGN TABLE [ IF NOT EXISTS ] <replaceable 
class="PARAMETER">table_name
--- 22,28 ----
      <replaceable class="PARAMETER">column_name</replaceable> <replaceable 
class="PARAMETER">data_type</replaceable> [ OPTIONS ( <replaceable 
class="PARAMETER">option</replaceable> '<replaceable 
class="PARAMETER">value</replaceable>' [, ... ] ) ] [ COLLATE 
<replaceable>collation</replaceable> ] [ <replaceable 
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,164 **** CREATE FOREIGN TABLE [ IF NOT EXISTS ] <replaceable 
class="PARAMETER">table_name
--- 160,176 ----
     </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>
+ 
+    <varlistentry>
      <term><replaceable class="PARAMETER">server_name</replaceable></term>
      <listitem>
       <para>
*** a/src/backend/commands/analyze.c
--- b/src/backend/commands/analyze.c
***************
*** 1496,1501 **** acquire_inherited_sample_rows(Relation onerel, int elevel,
--- 1496,1508 ----
                /* We already got the needed lock */
                childrel = heap_open(childOID, NoLock);
  
+               /* Ignore foreignt tables */
+               if (childrel->rd_rel->relkind == RELKIND_FOREIGN_TABLE)
+               {
+                       heap_close(childrel, AccessShareLock);
+                       continue;
+               }
+ 
                /* Ignore if temp table of another backend */
                if (RELATION_IS_OTHER_TEMP(childrel))
                {
*** a/src/backend/commands/tablecmds.c
--- b/src/backend/commands/tablecmds.c
***************
*** 310,316 **** static AlteredTableInfo *ATGetQueueEntry(List **wqueue, 
Relation rel);
  static void ATSimplePermissions(Relation rel, int allowed_targets);
  static void ATWrongRelkindError(Relation rel, int allowed_targets);
  static void ATSimpleRecursion(List **wqueue, Relation rel,
!                                 AlterTableCmd *cmd, bool recurse, LOCKMODE 
lockmode);
  static void ATTypedTableRecursion(List **wqueue, Relation rel, AlterTableCmd 
*cmd,
                                          LOCKMODE lockmode);
  static List *find_typed_table_dependencies(Oid typeOid, const char *typeName,
--- 310,317 ----
  static void ATSimplePermissions(Relation rel, int allowed_targets);
  static void ATWrongRelkindError(Relation rel, int allowed_targets);
  static void ATSimpleRecursion(List **wqueue, Relation rel,
!                                 AlterTableCmd *cmd, bool recurse,
!                                 bool include_foreign, LOCKMODE lockmode);
  static void ATTypedTableRecursion(List **wqueue, Relation rel, AlterTableCmd 
*cmd,
                                          LOCKMODE lockmode);
  static List *find_typed_table_dependencies(Oid typeOid, const char *typeName,
***************
*** 465,474 **** DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId)
                ereport(ERROR,
                                (errcode(ERRCODE_INVALID_TABLE_DEFINITION),
                                 errmsg("ON COMMIT can only be used on 
temporary tables")));
!       if (stmt->constraints != NIL && relkind == RELKIND_FOREIGN_TABLE)
!               ereport(ERROR,
!                               (errcode(ERRCODE_WRONG_OBJECT_TYPE),
!                                errmsg("constraints are not supported on 
foreign tables")));
  
        /*
         * Look up the namespace in which we are supposed to create the 
relation,
--- 466,490 ----
                ereport(ERROR,
                                (errcode(ERRCODE_INVALID_TABLE_DEFINITION),
                                 errmsg("ON COMMIT can only be used on 
temporary tables")));
! /*
!  * Shouldn't this have been checked in parser?
!  */
!       if (relkind == RELKIND_FOREIGN_TABLE)
!       {
!               ListCell   *lc;
!               foreach(lc, stmt->constraints)
!               {
!                       NewConstraint      *nc = lfirst(lc);
! 
!                       if (nc->contype != CONSTR_CHECK &&
!                               nc->contype != CONSTR_DEFAULT &&
!                               nc->contype != CONSTR_NULL &&
!                               nc->contype != CONSTR_NOTNULL)
!                               ereport(ERROR,
!                                               
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
!                                                errmsg("only check constraints 
are supported on foreign tables")));
!               }
!       }
  
        /*
         * Look up the namespace in which we are supposed to create the 
relation,
***************
*** 3014,3037 **** ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
                         * rules.
                         */
                        ATSimplePermissions(rel, ATT_TABLE | ATT_VIEW | 
ATT_FOREIGN_TABLE);
!                       ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode);
                        /* No command-specific prep needed */
                        pass = cmd->def ? AT_PASS_ADD_CONSTR : AT_PASS_DROP;
                        break;
                case AT_DropNotNull:    /* ALTER COLUMN DROP NOT NULL */
                        ATSimplePermissions(rel, ATT_TABLE | ATT_FOREIGN_TABLE);
!                       ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode);
                        /* No command-specific prep needed */
                        pass = AT_PASS_DROP;
                        break;
                case AT_SetNotNull:             /* ALTER COLUMN SET NOT NULL */
                        ATSimplePermissions(rel, ATT_TABLE | ATT_FOREIGN_TABLE);
!                       ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode);
                        /* No command-specific prep needed */
                        pass = AT_PASS_ADD_CONSTR;
                        break;
                case AT_SetStatistics:  /* ALTER COLUMN SET STATISTICS */
!                       ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode);
                        /* Performs own permission checks */
                        ATPrepSetStatistics(rel, cmd->name, cmd->def, lockmode);
                        pass = AT_PASS_MISC;
--- 3030,3053 ----
                         * rules.
                         */
                        ATSimplePermissions(rel, ATT_TABLE | ATT_VIEW | 
ATT_FOREIGN_TABLE);
!                       ATSimpleRecursion(wqueue, rel, cmd, recurse, true, 
lockmode);
                        /* No command-specific prep needed */
                        pass = cmd->def ? AT_PASS_ADD_CONSTR : AT_PASS_DROP;
                        break;
                case AT_DropNotNull:    /* ALTER COLUMN DROP NOT NULL */
                        ATSimplePermissions(rel, ATT_TABLE | ATT_FOREIGN_TABLE);
!                       ATSimpleRecursion(wqueue, rel, cmd, recurse, true, 
lockmode);
                        /* No command-specific prep needed */
                        pass = AT_PASS_DROP;
                        break;
                case AT_SetNotNull:             /* ALTER COLUMN SET NOT NULL */
                        ATSimplePermissions(rel, ATT_TABLE | ATT_FOREIGN_TABLE);
!                       ATSimpleRecursion(wqueue, rel, cmd, recurse, true, 
lockmode);
                        /* No command-specific prep needed */
                        pass = AT_PASS_ADD_CONSTR;
                        break;
                case AT_SetStatistics:  /* ALTER COLUMN SET STATISTICS */
!                       ATSimpleRecursion(wqueue, rel, cmd, recurse, true, 
lockmode);
                        /* Performs own permission checks */
                        ATPrepSetStatistics(rel, cmd->name, cmd->def, lockmode);
                        pass = AT_PASS_MISC;
***************
*** 3044,3050 **** ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
                        break;
                case AT_SetStorage:             /* ALTER COLUMN SET STORAGE */
                        ATSimplePermissions(rel, ATT_TABLE | ATT_MATVIEW);
!                       ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode);
                        /* No command-specific prep needed */
                        pass = AT_PASS_MISC;
                        break;
--- 3060,3066 ----
                        break;
                case AT_SetStorage:             /* ALTER COLUMN SET STORAGE */
                        ATSimplePermissions(rel, ATT_TABLE | ATT_MATVIEW);
!                       ATSimpleRecursion(wqueue, rel, cmd, recurse, false, 
lockmode);
                        /* No command-specific prep needed */
                        pass = AT_PASS_MISC;
                        break;
***************
*** 3062,3068 **** ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
                        pass = AT_PASS_ADD_INDEX;
                        break;
                case AT_AddConstraint:  /* ADD CONSTRAINT */
!                       ATSimplePermissions(rel, ATT_TABLE);
                        /* Recursion occurs during execution phase */
                        /* No command-specific prep needed except saving 
recurse flag */
                        if (recurse)
--- 3078,3084 ----
                        pass = AT_PASS_ADD_INDEX;
                        break;
                case AT_AddConstraint:  /* ADD CONSTRAINT */
!                       ATSimplePermissions(rel, ATT_TABLE | ATT_FOREIGN_TABLE);
                        /* Recursion occurs during execution phase */
                        /* No command-specific prep needed except saving 
recurse flag */
                        if (recurse)
***************
*** 3076,3082 **** ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
                        pass = AT_PASS_ADD_CONSTR;
                        break;
                case AT_DropConstraint: /* DROP CONSTRAINT */
!                       ATSimplePermissions(rel, ATT_TABLE);
                        /* Recursion occurs during execution phase */
                        /* No command-specific prep needed except saving 
recurse flag */
                        if (recurse)
--- 3092,3098 ----
                        pass = AT_PASS_ADD_CONSTR;
                        break;
                case AT_DropConstraint: /* DROP CONSTRAINT */
!                       ATSimplePermissions(rel, ATT_TABLE | ATT_FOREIGN_TABLE);
                        /* Recursion occurs during execution phase */
                        /* No command-specific prep needed except saving 
recurse flag */
                        if (recurse)
***************
*** 3144,3156 **** ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
                        pass = AT_PASS_MISC;
                        break;
                case AT_AddInherit:             /* INHERIT */
!                       ATSimplePermissions(rel, ATT_TABLE);
                        /* This command never recurses */
                        ATPrepAddInherit(rel);
                        pass = AT_PASS_MISC;
                        break;
                case AT_AlterConstraint:                /* ALTER CONSTRAINT */
!                       ATSimplePermissions(rel, ATT_TABLE);
                        pass = AT_PASS_MISC;
                        break;
                case AT_ValidateConstraint:             /* VALIDATE CONSTRAINT 
*/
--- 3160,3178 ----
                        pass = AT_PASS_MISC;
                        break;
                case AT_AddInherit:             /* INHERIT */
!                       ATSimplePermissions(rel, ATT_TABLE | ATT_FOREIGN_TABLE);
                        /* This command never recurses */
                        ATPrepAddInherit(rel);
                        pass = AT_PASS_MISC;
                        break;
+               case AT_DropInherit:    /* NO INHERIT */
+                       ATSimplePermissions(rel, ATT_TABLE | ATT_FOREIGN_TABLE);
+                       /* This command never recurse */
+                       /* No command-specific prep needed */
+                       pass = AT_PASS_MISC;
+                       break;
                case AT_AlterConstraint:                /* ALTER CONSTRAINT */
!                       ATSimplePermissions(rel, ATT_TABLE | ATT_FOREIGN_TABLE);
                        pass = AT_PASS_MISC;
                        break;
                case AT_ValidateConstraint:             /* VALIDATE CONSTRAINT 
*/
***************
*** 3179,3185 **** ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
                case AT_EnableAlwaysRule:
                case AT_EnableReplicaRule:
                case AT_DisableRule:
-               case AT_DropInherit:    /* NO INHERIT */
                case AT_AddOf:                  /* OF */
                case AT_DropOf: /* NOT OF */
                        ATSimplePermissions(rel, ATT_TABLE);
--- 3201,3206 ----
***************
*** 4120,4126 **** ATWrongRelkindError(Relation rel, int allowed_targets)
   */
  static void
  ATSimpleRecursion(List **wqueue, Relation rel,
!                                 AlterTableCmd *cmd, bool recurse, LOCKMODE 
lockmode)
  {
        /*
         * Propagate to children if desired.  Non-table relations never have
--- 4141,4148 ----
   */
  static void
  ATSimpleRecursion(List **wqueue, Relation rel,
!                                 AlterTableCmd *cmd, bool recurse,
!                                 bool include_foreign, LOCKMODE lockmode)
  {
        /*
         * Propagate to children if desired.  Non-table relations never have
***************
*** 4148,4155 **** ATSimpleRecursion(List **wqueue, Relation rel,
                                continue;
                        /* find_all_inheritors already got lock */
                        childrel = relation_open(childrelid, NoLock);
!                       CheckTableNotInUse(childrel, "ALTER TABLE");
!                       ATPrepCmd(wqueue, childrel, cmd, false, true, lockmode);
                        relation_close(childrel, NoLock);
                }
        }
--- 4170,4181 ----
                                continue;
                        /* find_all_inheritors already got lock */
                        childrel = relation_open(childrelid, NoLock);
!                       if (childrel->rd_rel->relkind != RELKIND_FOREIGN_TABLE
!                               || include_foreign)
!                       {
!                               CheckTableNotInUse(childrel, "ALTER TABLE");
!                               ATPrepCmd(wqueue, childrel, cmd, false, true, 
lockmode);
!                       }
                        relation_close(childrel, NoLock);
                }
        }
***************
*** 4439,4445 **** ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, 
Relation rel,
  
        /* At top level, permission check was done in ATPrepCmd, else do it */
        if (recursing)
!               ATSimplePermissions(rel, ATT_TABLE);
  
        attrdesc = heap_open(AttributeRelationId, RowExclusiveLock);
  
--- 4465,4471 ----
  
        /* At top level, permission check was done in ATPrepCmd, else do it */
        if (recursing)
!               ATSimplePermissions(rel, ATT_TABLE | ATT_FOREIGN_TABLE);
  
        attrdesc = heap_open(AttributeRelationId, RowExclusiveLock);
  
***************
*** 5335,5341 **** ATExecDropColumn(List **wqueue, Relation rel, const char 
*colName,
  
        /* At top level, permission check was done in ATPrepCmd, else do it */
        if (recursing)
!               ATSimplePermissions(rel, ATT_TABLE);
  
        /*
         * get the number of the attribute
--- 5361,5367 ----
  
        /* At top level, permission check was done in ATPrepCmd, else do it */
        if (recursing)
!               ATSimplePermissions(rel, ATT_TABLE | ATT_FOREIGN_TABLE);
  
        /*
         * get the number of the attribute
***************
*** 5726,5732 **** ATAddCheckConstraint(List **wqueue, AlteredTableInfo *tab, 
Relation rel,
  
        /* At top level, permission check was done in ATPrepCmd, else do it */
        if (recursing)
!               ATSimplePermissions(rel, ATT_TABLE);
  
        /*
         * Call AddRelationNewConstraints to do the work, making sure it works 
on
--- 5752,5758 ----
  
        /* At top level, permission check was done in ATPrepCmd, else do it */
        if (recursing)
!               ATSimplePermissions(rel, ATT_TABLE | ATT_FOREIGN_TABLE);
  
        /*
         * Call AddRelationNewConstraints to do the work, making sure it works 
on
***************
*** 7215,7221 **** ATExecDropConstraint(Relation rel, const char *constrName,
  
        /* At top level, permission check was done in ATPrepCmd, else do it */
        if (recursing)
!               ATSimplePermissions(rel, ATT_TABLE);
  
        conrel = heap_open(ConstraintRelationId, RowExclusiveLock);
  
--- 7241,7247 ----
  
        /* At top level, permission check was done in ATPrepCmd, else do it */
        if (recursing)
!               ATSimplePermissions(rel, ATT_TABLE | ATT_FOREIGN_TABLE);
  
        conrel = heap_open(ConstraintRelationId, RowExclusiveLock);
  
***************
*** 7550,7556 **** ATPrepAlterColumnType(List **wqueue,
         * alter would put them out of step.
         */
        if (recurse)
!               ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode);
        else if (!recursing &&
                         find_inheritance_children(RelationGetRelid(rel), 
NoLock) != NIL)
                ereport(ERROR,
--- 7576,7582 ----
         * alter would put them out of step.
         */
        if (recurse)
!               ATSimpleRecursion(wqueue, rel, cmd, recurse, true, lockmode);
        else if (!recursing &&
                         find_inheritance_children(RelationGetRelid(rel), 
NoLock) != NIL)
                ereport(ERROR,
*** a/src/backend/optimizer/prep/prepunion.c
--- b/src/backend/optimizer/prep/prepunion.c
***************
*** 1337,1347 **** expand_inherited_rtentry(PlannerInfo *root, RangeTblEntry 
*rte, Index rti)
                /*
                 * Build an RTE for the child, and attach to query's rangetable 
list.
                 * We copy most fields of the parent's RTE, but replace 
relation OID,
!                * and set inh = false.  Also, set requiredPerms to zero since 
all
!                * required permissions checks are done on the original RTE.
                 */
                childrte = copyObject(rte);
                childrte->relid = childOID;
                childrte->inh = false;
                childrte->requiredPerms = 0;
                parse->rtable = lappend(parse->rtable, childrte);
--- 1337,1348 ----
                /*
                 * Build an RTE for the child, and attach to query's rangetable 
list.
                 * We copy most fields of the parent's RTE, but replace 
relation OID,
!                * relkind and set inh = false.  Also, set requiredPerms to 
zero since
!                * all required permissions checks are done on the original RTE.
                 */
                childrte = copyObject(rte);
                childrte->relid = childOID;
+               childrte->relkind = newrelation->rd_rel->relkind;
                childrte->inh = false;
                childrte->requiredPerms = 0;
                parse->rtable = lappend(parse->rtable, childrte);
*** a/src/backend/parser/gram.y
--- b/src/backend/parser/gram.y
***************
*** 4207,4238 **** AlterForeignServerStmt: ALTER SERVER name 
foreign_server_version alter_generic_o
  CreateForeignTableStmt:
                CREATE FOREIGN TABLE qualified_name
                        '(' OptTableElementList ')'
!                       SERVER name create_generic_options
                                {
                                        CreateForeignTableStmt *n = 
makeNode(CreateForeignTableStmt);
                                        $4->relpersistence = 
RELPERSISTENCE_PERMANENT;
                                        n->base.relation = $4;
                                        n->base.tableElts = $6;
!                                       n->base.inhRelations = NIL;
                                        n->base.if_not_exists = false;
                                        /* FDW-specific data */
!                                       n->servername = $9;
!                                       n->options = $10;
                                        $$ = (Node *) n;
                                }
                | CREATE FOREIGN TABLE IF_P NOT EXISTS qualified_name
                        '(' OptTableElementList ')'
!                       SERVER name create_generic_options
                                {
                                        CreateForeignTableStmt *n = 
makeNode(CreateForeignTableStmt);
                                        $7->relpersistence = 
RELPERSISTENCE_PERMANENT;
                                        n->base.relation = $7;
                                        n->base.tableElts = $9;
!                                       n->base.inhRelations = NIL;
                                        n->base.if_not_exists = true;
                                        /* FDW-specific data */
!                                       n->servername = $12;
!                                       n->options = $13;
                                        $$ = (Node *) n;
                                }
                ;
--- 4207,4238 ----
  CreateForeignTableStmt:
                CREATE FOREIGN TABLE qualified_name
                        '(' OptTableElementList ')'
!                       OptInherit SERVER name create_generic_options
                                {
                                        CreateForeignTableStmt *n = 
makeNode(CreateForeignTableStmt);
                                        $4->relpersistence = 
RELPERSISTENCE_PERMANENT;
                                        n->base.relation = $4;
                                        n->base.tableElts = $6;
!                                       n->base.inhRelations = $8;
                                        n->base.if_not_exists = false;
                                        /* FDW-specific data */
!                                       n->servername = $10;
!                                       n->options = $11;
                                        $$ = (Node *) n;
                                }
                | CREATE FOREIGN TABLE IF_P NOT EXISTS qualified_name
                        '(' OptTableElementList ')'
!                       OptInherit SERVER name create_generic_options
                                {
                                        CreateForeignTableStmt *n = 
makeNode(CreateForeignTableStmt);
                                        $7->relpersistence = 
RELPERSISTENCE_PERMANENT;
                                        n->base.relation = $7;
                                        n->base.tableElts = $9;
!                                       n->base.inhRelations = $11;
                                        n->base.if_not_exists = true;
                                        /* FDW-specific data */
!                                       n->servername = $13;
!                                       n->options = $14;
                                        $$ = (Node *) n;
                                }
                ;
*** a/src/backend/parser/parse_utilcmd.c
--- b/src/backend/parser/parse_utilcmd.c
***************
*** 515,526 **** transformColumnDefinition(CreateStmtContext *cxt, ColumnDef 
*column)
                                break;
  
                        case CONSTR_CHECK:
-                               if (cxt->isforeign)
-                                       ereport(ERROR,
-                                                       
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-                                       errmsg("constraints are not supported 
on foreign tables"),
-                                                        
parser_errposition(cxt->pstate,
-                                                                               
                constraint->location)));
                                cxt->ckconstraints = 
lappend(cxt->ckconstraints, constraint);
                                break;
  
--- 515,520 ----
***************
*** 605,614 **** transformColumnDefinition(CreateStmtContext *cxt, ColumnDef 
*column)
  static void
  transformTableConstraint(CreateStmtContext *cxt, Constraint *constraint)
  {
!       if (cxt->isforeign)
                ereport(ERROR,
                                (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
!                                errmsg("constraints are not supported on 
foreign tables"),
                                 parser_errposition(cxt->pstate,
                                                                        
constraint->location)));
  
--- 599,608 ----
  static void
  transformTableConstraint(CreateStmtContext *cxt, Constraint *constraint)
  {
!       if (cxt->isforeign && constraint->contype != CONSTR_CHECK)
                ereport(ERROR,
                                (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
!                                errmsg("only check constraints are supported 
on foreign tables"),
                                 parser_errposition(cxt->pstate,
                                                                        
constraint->location)));
  
*** a/src/test/regress/expected/foreign_data.out
--- b/src/test/regress/expected/foreign_data.out
***************
*** 750,765 **** CREATE TABLE use_ft1_column_type (x ft1);
  ALTER FOREIGN TABLE ft1 ALTER COLUMN c8 SET DATA TYPE integer;        -- ERROR
  ERROR:  cannot alter foreign table "ft1" because column 
"use_ft1_column_type.x" uses its row type
  DROP TABLE use_ft1_column_type;
! ALTER FOREIGN TABLE ft1 ADD CONSTRAINT ft1_c9_check CHECK (c9 < 0); -- ERROR
! ERROR:  constraints are not supported on foreign tables
! LINE 1: ALTER FOREIGN TABLE ft1 ADD CONSTRAINT ft1_c9_check CHECK (c...
!                                     ^
  ALTER FOREIGN TABLE ft1 DROP CONSTRAINT no_const;               -- ERROR
! ERROR:  "ft1" is not a table
  ALTER FOREIGN TABLE ft1 DROP CONSTRAINT IF EXISTS no_const;
! ERROR:  "ft1" is not a table
! ALTER FOREIGN TABLE ft1 DROP CONSTRAINT ft1_c1_check;
! ERROR:  "ft1" is not a table
  ALTER FOREIGN TABLE ft1 SET WITH OIDS;                          -- ERROR
  ERROR:  "ft1" is not a table
  ALTER FOREIGN TABLE ft1 OWNER TO regress_test_role;
--- 750,761 ----
  ALTER FOREIGN TABLE ft1 ALTER COLUMN c8 SET DATA TYPE integer;        -- ERROR
  ERROR:  cannot alter foreign table "ft1" because column 
"use_ft1_column_type.x" uses its row type
  DROP TABLE use_ft1_column_type;
! ALTER FOREIGN TABLE ft1 ADD CONSTRAINT ft1_c9_check CHECK (c9 < 0);
  ALTER FOREIGN TABLE ft1 DROP CONSTRAINT no_const;               -- ERROR
! ERROR:  constraint "no_const" of relation "ft1" does not exist
  ALTER FOREIGN TABLE ft1 DROP CONSTRAINT IF EXISTS no_const;
! NOTICE:  constraint "no_const" of relation "ft1" does not exist, skipping
! ALTER FOREIGN TABLE ft1 DROP CONSTRAINT ft1_c9_check;
  ALTER FOREIGN TABLE ft1 SET WITH OIDS;                          -- ERROR
  ERROR:  "ft1" is not a table
  ALTER FOREIGN TABLE ft1 OWNER TO regress_test_role;
*** a/src/test/regress/sql/foreign_data.sql
--- b/src/test/regress/sql/foreign_data.sql
***************
*** 314,323 **** ALTER FOREIGN TABLE ft1 ALTER COLUMN c8 SET STATISTICS -1;
  CREATE TABLE use_ft1_column_type (x ft1);
  ALTER FOREIGN TABLE ft1 ALTER COLUMN c8 SET DATA TYPE integer;        -- ERROR
  DROP TABLE use_ft1_column_type;
! ALTER FOREIGN TABLE ft1 ADD CONSTRAINT ft1_c9_check CHECK (c9 < 0); -- ERROR
  ALTER FOREIGN TABLE ft1 DROP CONSTRAINT no_const;               -- ERROR
  ALTER FOREIGN TABLE ft1 DROP CONSTRAINT IF EXISTS no_const;
! ALTER FOREIGN TABLE ft1 DROP CONSTRAINT ft1_c1_check;
  ALTER FOREIGN TABLE ft1 SET WITH OIDS;                          -- ERROR
  ALTER FOREIGN TABLE ft1 OWNER TO regress_test_role;
  ALTER FOREIGN TABLE ft1 OPTIONS (DROP delimiter, SET quote '~', ADD escape 
'@');
--- 314,323 ----
  CREATE TABLE use_ft1_column_type (x ft1);
  ALTER FOREIGN TABLE ft1 ALTER COLUMN c8 SET DATA TYPE integer;        -- ERROR
  DROP TABLE use_ft1_column_type;
! ALTER FOREIGN TABLE ft1 ADD CONSTRAINT ft1_c9_check CHECK (c9 < 0);
  ALTER FOREIGN TABLE ft1 DROP CONSTRAINT no_const;               -- ERROR
  ALTER FOREIGN TABLE ft1 DROP CONSTRAINT IF EXISTS no_const;
! ALTER FOREIGN TABLE ft1 DROP CONSTRAINT ft1_c9_check;
  ALTER FOREIGN TABLE ft1 SET WITH OIDS;                          -- ERROR
  ALTER FOREIGN TABLE ft1 OWNER TO regress_test_role;
  ALTER FOREIGN TABLE ft1 OPTIONS (DROP delimiter, SET quote '~', ADD escape 
'@');
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to