From 719954a22d7a1fdf75fdcc10a51ea9fdf2d0ed38 Mon Sep 17 00:00:00 2001 From: amit Date: Fri, 6 Jan 2017 15:33:02 +0900 Subject: [PATCH 4/8] Fix some issues with views and partitioned tables Automatically updatable views failed to handle partitioned tables. Once that's fixed, WITH CHECK OPTIONS wouldn't work correctly without the WCO expressions having been suitably converted for each partition (think applying map_partition_varattnos to Vars in the WCO expressions just like with partition constraint expressions). Reported by: n/a Patch by: Amit Langote Reports: n/a --- src/backend/executor/execMain.c | 49 +++++++++++++++++++++++++++ src/backend/executor/nodeModifyTable.c | 40 ++++++++++++++++++++++ src/backend/rewrite/rewriteHandler.c | 3 +- src/test/regress/expected/updatable_views.out | 24 +++++++++++++ src/test/regress/sql/updatable_views.sql | 19 +++++++++++ 5 files changed, 134 insertions(+), 1 deletion(-) diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index 8cb9691056..04c026f3f8 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -1963,6 +1963,55 @@ ExecWithCheckOptions(WCOKind kind, ResultRelInfo *resultRelInfo, Bitmapset *insertedCols; Bitmapset *updatedCols; + /* + * In case where the tuple is routed, it's been converted + * to the partition's rowtype, which might differ from the + * root table's. We must convert it back to the root table's + * type so that it's shown correctly in the error message. + */ + if (resultRelInfo->ri_PartitionRoot) + { + HeapTuple tuple = ExecFetchSlotTuple(slot); + TupleDesc old_tupdesc = RelationGetDescr(rel); + TupleConversionMap *map; + + rel = resultRelInfo->ri_PartitionRoot; + tupdesc = RelationGetDescr(rel); + /* a reverse map */ + map = convert_tuples_by_name(old_tupdesc, tupdesc, false, + gettext_noop("could not convert row type")); + if (map != NULL) + { + tuple = do_convert_tuple(tuple, map); + ExecStoreTuple(tuple, slot, InvalidBuffer, false); + } + } + + /* + * In the case where the tuple is routed, it's been converted to + * the partition's rowtype, which might differ from the root + * table's. We must convert it back to the root table's type so + * that val_desc shown error message matches the input tuple. + */ + if (resultRelInfo->ri_PartitionRoot) + { + HeapTuple tuple = ExecFetchSlotTuple(slot); + TupleDesc old_tupdesc = RelationGetDescr(rel); + TupleConversionMap *map; + + rel = resultRelInfo->ri_PartitionRoot; + tupdesc = RelationGetDescr(rel); + /* a reverse map */ + map = convert_tuples_by_name(old_tupdesc, tupdesc, false, + gettext_noop("could not convert row type")); + if (map != NULL) + { + tuple = do_convert_tuple(tuple, map); + ExecSetSlotDescriptor(slot, tupdesc); + ExecStoreTuple(tuple, slot, InvalidBuffer, false); + } + } + switch (wco->kind) { /* diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 982f15d490..57edfeab95 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -1778,6 +1778,46 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) } /* + * Build WITH CHECK OPTION constraints for each leaf partition rel. + * Note that we didn't build the withCheckOptionList for each partition + * within the planner, but simple translation of the varattnos for each + * partition will suffice. This only occurs for the INSERT case; + * UPDATE/DELETE cases are handled above. + */ + if (node->withCheckOptionLists != NIL && mtstate->mt_num_partitions > 0) + { + List *wcoList; + + Assert(operation == CMD_INSERT); + resultRelInfo = mtstate->mt_partitions; + wcoList = linitial(node->withCheckOptionLists); + for (i = 0; i < mtstate->mt_num_partitions; i++) + { + Relation partrel = resultRelInfo->ri_RelationDesc; + List *mapped_wcoList; + List *wcoExprs = NIL; + ListCell *ll; + + /* varno = node->nominalRelation */ + mapped_wcoList = map_partition_varattnos(wcoList, + node->nominalRelation, + partrel, rel); + foreach(ll, mapped_wcoList) + { + WithCheckOption *wco = (WithCheckOption *) lfirst(ll); + ExprState *wcoExpr = ExecInitExpr((Expr *) wco->qual, + mtstate->mt_plans[i]); + + wcoExprs = lappend(wcoExprs, wcoExpr); + } + + resultRelInfo->ri_WithCheckOptions = mapped_wcoList; + resultRelInfo->ri_WithCheckOptionExprs = wcoExprs; + resultRelInfo++; + } + } + + /* * Initialize RETURNING projections if needed. */ if (node->returningLists) diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c index d1ff3b20b6..d3e44fb135 100644 --- a/src/backend/rewrite/rewriteHandler.c +++ b/src/backend/rewrite/rewriteHandler.c @@ -2249,7 +2249,8 @@ view_query_is_auto_updatable(Query *viewquery, bool check_cols) if (base_rte->rtekind != RTE_RELATION || (base_rte->relkind != RELKIND_RELATION && base_rte->relkind != RELKIND_FOREIGN_TABLE && - base_rte->relkind != RELKIND_VIEW)) + base_rte->relkind != RELKIND_VIEW && + base_rte->relkind != RELKIND_PARTITIONED_TABLE)) return gettext_noop("Views that do not select from a single table or view are not automatically updatable."); if (base_rte->tablesample) diff --git a/src/test/regress/expected/updatable_views.out b/src/test/regress/expected/updatable_views.out index 2da3c069e1..b49fd58ac9 100644 --- a/src/test/regress/expected/updatable_views.out +++ b/src/test/regress/expected/updatable_views.out @@ -2367,3 +2367,27 @@ ERROR: new row violates check option for view "v1" DETAIL: Failing row contains (-1, invalid). DROP VIEW v1; DROP TABLE t1; +-- check that an auto-updatable view on a partitioned table works correctly +create table p (a int, b int) partition by range (a, b); +create table p1 (b int not null, a int not null) partition by range (b); +create table p11 (like p1); +alter table p11 drop a; +alter table p11 add a int; +alter table p11 drop a; +alter table p11 add a int not null; +alter table p1 attach partition p11 for values from (2) to (5); +alter table p attach partition p1 for values from (1, 2) to (1, 10); +create view pv as select * from p; +insert into pv values (1, 2); +select tableoid::regclass, * from p; + tableoid | a | b +----------+---+--- + p11 | 1 | 2 +(1 row) + +create view pv_wco as select * from p where a = 0 with check option; +insert into pv_wco values (1, 2); +ERROR: new row violates check option for view "pv_wco" +DETAIL: Failing row contains (1, 2). +drop view pv, pv_wco; +drop table p, p1, p11; diff --git a/src/test/regress/sql/updatable_views.sql b/src/test/regress/sql/updatable_views.sql index ffc64d2de9..3c19edc8f7 100644 --- a/src/test/regress/sql/updatable_views.sql +++ b/src/test/regress/sql/updatable_views.sql @@ -1112,3 +1112,22 @@ INSERT INTO v1 VALUES (-1, 'invalid'); -- should fail DROP VIEW v1; DROP TABLE t1; + +-- check that an auto-updatable view on a partitioned table works correctly +create table p (a int, b int) partition by range (a, b); +create table p1 (b int not null, a int not null) partition by range (b); +create table p11 (like p1); +alter table p11 drop a; +alter table p11 add a int; +alter table p11 drop a; +alter table p11 add a int not null; +alter table p1 attach partition p11 for values from (2) to (5); +alter table p attach partition p1 for values from (1, 2) to (1, 10); + +create view pv as select * from p; +insert into pv values (1, 2); +select tableoid::regclass, * from p; +create view pv_wco as select * from p where a = 0 with check option; +insert into pv_wco values (1, 2); +drop view pv, pv_wco; +drop table p, p1, p11; -- 2.11.0