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