From 365145d11c28c63106f641995ed8c266b7e05184 Mon Sep 17 00:00:00 2001 From: amit Date: Tue, 27 Dec 2016 16:56:58 +0900 Subject: [PATCH 3/8] Fix RETURNING to work correctly after tuple-routing In ExecInsert(), do not switch back to the root partitioned table ResultRelInfo until after we finish ExecProcessReturning(), so that RETURNING projection is done using the partition's descriptor. For the projection to work correctly, we must initialize the same for each leaf partition during ModifyTableState initialization. With this commit, map_partition_varattnos() now accepts one more argument viz. target_varno. Previously, it assumed varno = 1 for its input expressions, which was fine since its usage was limited to partition constraints. To use it with expressions such as an INSERT statement's returning list, we must be prepared for varnos != 1 as in the change above. Reported by: n/a Patch by: Amit Langote Reports: n/a --- src/backend/catalog/partition.c | 8 ++++--- src/backend/commands/tablecmds.c | 1 + src/backend/executor/execMain.c | 4 ++-- src/backend/executor/nodeModifyTable.c | 43 +++++++++++++++++++++++++++------- src/include/catalog/partition.h | 3 ++- src/test/regress/expected/insert.out | 24 ++++++++++++++++++- src/test/regress/sql/insert.sql | 16 ++++++++++++- 7 files changed, 82 insertions(+), 17 deletions(-) diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c index e540fc16d0..c0a5f98f3c 100644 --- a/src/backend/catalog/partition.c +++ b/src/backend/catalog/partition.c @@ -883,7 +883,8 @@ get_qual_from_partbound(Relation rel, Relation parent, Node *bound) * different from the parent's. */ List * -map_partition_varattnos(List *expr, Relation partrel, Relation parent) +map_partition_varattnos(List *expr, int target_varno, + Relation partrel, Relation parent) { TupleDesc tupdesc = RelationGetDescr(parent); AttrNumber attno; @@ -908,7 +909,7 @@ map_partition_varattnos(List *expr, Relation partrel, Relation parent) } expr = (List *) map_variable_attnos((Node *) expr, - 1, 0, + target_varno, 0, part_attnos, tupdesc->natts, &found_whole_row); @@ -1540,8 +1541,9 @@ generate_partition_qual(Relation rel) * Change Vars to have partition's attnos instead of the parent's. * We do this after we concatenate the parent's quals, because * we want every Var in it to bear this relation's attnos. + * It's safe to assume varno = 1 here. */ - result = map_partition_varattnos(result, rel, parent); + result = map_partition_varattnos(result, 1, rel, parent); /* Save a copy in the relcache */ oldcxt = MemoryContextSwitchTo(CacheMemoryContext); diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 06e43cbb3a..18cac9ad2d 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -13454,6 +13454,7 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd) constr = linitial(partConstraint); my_constr = make_ands_implicit((Expr *) constr); tab->partition_constraint = map_partition_varattnos(my_constr, + 1, part_rel, rel); /* keep our lock until commit */ diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index 67e46729f3..8cb9691056 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -1279,10 +1279,10 @@ InitResultRelInfo(ResultRelInfo *resultRelInfo, /* * This is not our own partition constraint, but rather an ancestor's. * So any Vars in it bear the ancestor's attribute numbers. We must - * switch them to our own. + * switch them to our own. (dummy varno = 1) */ if (partition_check != NIL) - partition_check = map_partition_varattnos(partition_check, + partition_check = map_partition_varattnos(partition_check, 1, resultRelationDesc, partition_root); } diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 4692427e60..982f15d490 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -262,7 +262,8 @@ ExecInsert(ModifyTableState *mtstate, Relation resultRelationDesc; Oid newId; List *recheckIndexes = NIL; - TupleTableSlot *oldslot = slot; + TupleTableSlot *oldslot = slot, + *result = NULL; /* * get the heap tuple out of the tuple table slot, making sure we have a @@ -574,12 +575,6 @@ ExecInsert(ModifyTableState *mtstate, list_free(recheckIndexes); - if (saved_resultRelInfo) - { - resultRelInfo = saved_resultRelInfo; - estate->es_result_relation_info = resultRelInfo; - } - /* * Check any WITH CHECK OPTION constraints from parent views. We are * required to do this after testing all constraints and uniqueness @@ -597,9 +592,12 @@ ExecInsert(ModifyTableState *mtstate, /* Process RETURNING if present */ if (resultRelInfo->ri_projectReturning) - return ExecProcessReturning(resultRelInfo, slot, planSlot); + result = ExecProcessReturning(resultRelInfo, slot, planSlot); - return NULL; + if (saved_resultRelInfo) + estate->es_result_relation_info = saved_resultRelInfo; + + return result; } /* ---------------------------------------------------------------- @@ -1786,6 +1784,7 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) { TupleTableSlot *slot; ExprContext *econtext; + List *returningList; /* * Initialize result tuple slot and assign its rowtype using the first @@ -1818,6 +1817,32 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) resultRelInfo->ri_RelationDesc->rd_att); resultRelInfo++; } + + /* + * Build a projection for each leaf partition rel. Note that we + * didn't build the returningList 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 are handled above. + */ + resultRelInfo = mtstate->mt_partitions; + returningList = linitial(node->returningLists); + for (i = 0; i < mtstate->mt_num_partitions; i++) + { + Relation partrel = resultRelInfo->ri_RelationDesc; + List *rlist, + *rliststate; + + /* varno = node->nominalRelation */ + rlist = map_partition_varattnos(returningList, + node->nominalRelation, + partrel, rel); + rliststate = (List *) ExecInitExpr((Expr *) rlist, &mtstate->ps); + resultRelInfo->ri_projectReturning = + ExecBuildProjectionInfo(rliststate, econtext, slot, + resultRelInfo->ri_RelationDesc->rd_att); + resultRelInfo++; + } } else { diff --git a/src/include/catalog/partition.h b/src/include/catalog/partition.h index 537f0aad67..df7dcce331 100644 --- a/src/include/catalog/partition.h +++ b/src/include/catalog/partition.h @@ -77,7 +77,8 @@ extern bool partition_bounds_equal(PartitionKey key, extern void check_new_partition_bound(char *relname, Relation parent, Node *bound); extern Oid get_partition_parent(Oid relid); extern List *get_qual_from_partbound(Relation rel, Relation parent, Node *bound); -extern List *map_partition_varattnos(List *expr, Relation partrel, Relation parent); +extern List *map_partition_varattnos(List *expr, int target_varno, + Relation partrel, Relation parent); extern List *RelationGetPartitionQual(Relation rel); /* For tuple routing */ diff --git a/src/test/regress/expected/insert.out b/src/test/regress/expected/insert.out index b8e74caee8..81af3ef497 100644 --- a/src/test/regress/expected/insert.out +++ b/src/test/regress/expected/insert.out @@ -364,5 +364,27 @@ DETAIL: Failing row contains (1, 2). insert into p1 (a, b) values (2, 3); ERROR: new row for relation "p11" violates partition constraint DETAIL: Failing row contains (3, 2). +-- check that RETURNING works correctly with tuple-routing +alter table p drop constraint check_b; +create table p12 partition of p1 for values from (5) to (10); +create table p2 (b int not null, a int not null); +alter table p attach partition p2 for values from (1, 10) to (1, 20); +create table p3 partition of p for values from (1, 20) to (1, 30); +create table p4 (like p); +alter table p4 drop a; +alter table p4 add a int not null; +alter table p attach partition p4 for values from (1, 30) to (1, 40); +with ins (a, b, c) as + (insert into p (b, a) select s.a, 1 from generate_series(2, 39) s(a) returning tableoid::regclass, *) + select a, b, min(c), max(c) from ins group by a, b order by 1; + a | b | min | max +-----+---+-----+----- + p11 | 1 | 2 | 4 + p12 | 1 | 5 | 9 + p2 | 1 | 10 | 19 + p3 | 1 | 20 | 29 + p4 | 1 | 30 | 39 +(5 rows) + -- cleanup -drop table p, p1, p11; +drop table p, p1, p11, p12, p2, p3, p4; diff --git a/src/test/regress/sql/insert.sql b/src/test/regress/sql/insert.sql index 2154d01c56..454e1ce2e7 100644 --- a/src/test/regress/sql/insert.sql +++ b/src/test/regress/sql/insert.sql @@ -226,5 +226,19 @@ insert into p values (1, 2); -- selected by tuple-routing insert into p1 (a, b) values (2, 3); +-- check that RETURNING works correctly with tuple-routing +alter table p drop constraint check_b; +create table p12 partition of p1 for values from (5) to (10); +create table p2 (b int not null, a int not null); +alter table p attach partition p2 for values from (1, 10) to (1, 20); +create table p3 partition of p for values from (1, 20) to (1, 30); +create table p4 (like p); +alter table p4 drop a; +alter table p4 add a int not null; +alter table p attach partition p4 for values from (1, 30) to (1, 40); +with ins (a, b, c) as + (insert into p (b, a) select s.a, 1 from generate_series(2, 39) s(a) returning tableoid::regclass, *) + select a, b, min(c), max(c) from ins group by a, b order by 1; + -- cleanup -drop table p, p1, p11; +drop table p, p1, p11, p12, p2, p3, p4; -- 2.11.0