Hi hackers,

I'd like to propose adding inheritance support for foriegn tables.
David Fetter mentioned this feature last July, but it seems stalled.

    http://www.postgresql.org/message-id/20130719005601.ga5...@fetter.org

Supporting inheritance by foreign tables allows us to distribute query
to remote servers by using foreign tables as partition table of a
(perhaps ordinary) table.  For this purpose, I think that constraint
exclusion is necessary.

As result of extending Devid's patch for PoC, and AFAIS we need these changes:

1) Add INHERITS(rel, ...) clause to CREATE/ALTER FOREIGN TABLE
Apperantly we need to add new syntax to define parent table(s) of a
foreign table.  We have options about the position of INHERIT clause,
but I'd prefer before SERVER clause because having options specific to
foreign tables at the tail would be most extensible.

    a) CREATE FOREIGN TABLE child (...) INHERITS(p1, p2) SERVER server;
    b) CREATE FOREIGN TABLE child (...) SERVER server INHERITS(p1, p2);

2) Allow foreign tables to have CHECK constraints
Like NOT NULL, I think we don't need to enforce the check duroing
INSERT/UPDATE against foreign table.

3) Allow foreign table as a child node of Append
Currently prepunion.c assumes that children of Append have
RELKIND_RELATION as relkind always, so we need to set relkind of child
RTE explicitly.

Please see attached PoC patch.  I'll enhance implementation, tests and
document and submit the patch for the next CF.

Regards,
-- 
Shigeru HANADA
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 0b31f55..2f2dc88 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -465,10 +465,25 @@ 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")));
+/*
+ * 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,
@@ -1463,10 +1478,11 @@ MergeAttributes(List *schema, List *supers, char 
relpersistence,
                 */
                relation = heap_openrv(parent, ShareUpdateExclusiveLock);
 
-               if (relation->rd_rel->relkind != RELKIND_RELATION)
+               if (relation->rd_rel->relkind != RELKIND_RELATION &&
+                       relation->rd_rel->relkind != RELKIND_FOREIGN_TABLE)
                        ereport(ERROR,
                                        (errcode(ERRCODE_WRONG_OBJECT_TYPE),
-                                        errmsg("inherited relation \"%s\" is 
not a table",
+                                        errmsg("inherited relation \"%s\" is 
not a table or foreign table",
                                                        parent->relname)));
                /* Permanent rels cannot inherit from temporary ones */
                if (relpersistence != RELPERSISTENCE_TEMP &&
@@ -3043,7 +3059,7 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
                        pass = AT_PASS_ADD_INDEX;
                        break;
                case AT_AddConstraint:  /* ADD CONSTRAINT */
-                       ATSimplePermissions(rel, ATT_TABLE);
+                       ATSimplePermissions(rel, ATT_TABLE | ATT_FOREIGN_TABLE);
                        /* Recursion occurs during execution phase */
                        /* No command-specific prep needed except saving 
recurse flag */
                        if (recurse)
@@ -3057,7 +3073,7 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
                        pass = AT_PASS_ADD_CONSTR;
                        break;
                case AT_DropConstraint: /* DROP CONSTRAINT */
-                       ATSimplePermissions(rel, ATT_TABLE);
+                       ATSimplePermissions(rel, ATT_TABLE | ATT_FOREIGN_TABLE);
                        /* Recursion occurs during execution phase */
                        /* No command-specific prep needed except saving 
recurse flag */
                        if (recurse)
@@ -3125,13 +3141,13 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd 
*cmd,
                        pass = AT_PASS_MISC;
                        break;
                case AT_AddInherit:             /* INHERIT */
-                       ATSimplePermissions(rel, ATT_TABLE);
+                       ATSimplePermissions(rel, ATT_TABLE | ATT_FOREIGN_TABLE);
                        /* This command never recurses */
                        ATPrepAddInherit(rel);
                        pass = AT_PASS_MISC;
                        break;
                case AT_AlterConstraint:                /* ALTER CONSTRAINT */
-                       ATSimplePermissions(rel, ATT_TABLE);
+                       ATSimplePermissions(rel, ATT_TABLE | ATT_FOREIGN_TABLE);
                        pass = AT_PASS_MISC;
                        break;
                case AT_ValidateConstraint:             /* VALIDATE CONSTRAINT 
*/
diff --git a/src/backend/optimizer/prep/prepunion.c 
b/src/backend/optimizer/prep/prepunion.c
index e249628..0d76221 100644
--- a/src/backend/optimizer/prep/prepunion.c
+++ b/src/backend/optimizer/prep/prepunion.c
@@ -1337,11 +1337,12 @@ 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.
+                * 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);
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 11f6291..613a722 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -4191,32 +4191,32 @@ AlterForeignServerStmt: ALTER SERVER name 
foreign_server_version alter_generic_o
 CreateForeignTableStmt:
                CREATE FOREIGN TABLE qualified_name
                        '(' OptTableElementList ')'
-                       SERVER name create_generic_options
+                       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 = NIL;
+                                       n->base.inhRelations = $8;
                                        n->base.if_not_exists = false;
                                        /* FDW-specific data */
-                                       n->servername = $9;
-                                       n->options = $10;
+                                       n->servername = $10;
+                                       n->options = $11;
                                        $$ = (Node *) n;
                                }
                | CREATE FOREIGN TABLE IF_P NOT EXISTS qualified_name
                        '(' OptTableElementList ')'
-                       SERVER name create_generic_options
+                       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 = NIL;
+                                       n->base.inhRelations = $11;
                                        n->base.if_not_exists = true;
                                        /* FDW-specific data */
-                                       n->servername = $12;
-                                       n->options = $13;
+                                       n->servername = $13;
+                                       n->options = $14;
                                        $$ = (Node *) n;
                                }
                ;
diff --git a/src/backend/parser/parse_utilcmd.c 
b/src/backend/parser/parse_utilcmd.c
index 19d19e5f..8cf2998 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -515,12 +515,14 @@ transformColumnDefinition(CreateStmtContext *cxt, 
ColumnDef *column)
                                break;
 
                        case CONSTR_CHECK:
+#ifdef NOT_USED
                                if (cxt->isforeign)
                                        ereport(ERROR,
                                                        
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
                                        errmsg("constraints are not supported 
on foreign tables"),
                                                         
parser_errposition(cxt->pstate,
                                                                                
                constraint->location)));
+#endif
                                cxt->ckconstraints = 
lappend(cxt->ckconstraints, constraint);
                                break;
 
@@ -605,10 +607,10 @@ transformColumnDefinition(CreateStmtContext *cxt, 
ColumnDef *column)
 static void
 transformTableConstraint(CreateStmtContext *cxt, Constraint *constraint)
 {
-       if (cxt->isforeign)
+       if (cxt->isforeign && constraint->contype != CONSTR_CHECK)
                ereport(ERROR,
                                (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-                                errmsg("constraints are not supported on 
foreign tables"),
+                                errmsg("only check constraints are supported 
on foreign tables"),
                                 parser_errposition(cxt->pstate,
                                                                        
constraint->location)));
 
-- 
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