From ba880146aedee6c979714ca3e232ecb783d2dc88 Mon Sep 17 00:00:00 2001 From: amit Date: Thu, 4 Apr 2019 18:51:31 +0900 Subject: [PATCH v1] Fix a bug in multi-level tuple routing In 34295b87fb, we attempted to rearrange slot handling such that each of the intermediate levels' partitioned tables gets a copy of the tuple being routed in their own dedicated slot, which is also cleared after routing through that parent is finished. The code was written such that the copied tuple is added to a given level's parent's slot only if tuple conversion is required between the previous level's parent and the current parent. A case was presented in the report of BUG #15733, where no conversion is required between two levels, so the slot for the next level's parent was left without a tuple to extract partition key from, resulting in this error: ERROR: cannot extract attribute from empty tuple slot Rearrange the code so that whether or not to put the tuple into a given parent's own slot is decoupled from whether tuple conversion is needed between the previous level's parent and current parent. This also adds the reported test case to regression tests. --- src/backend/executor/execPartition.c | 37 ++++++++++++++++++++++++------------ src/test/regress/expected/insert.out | 21 ++++++++++++++++++++ src/test/regress/sql/insert.sql | 15 +++++++++++++++ 3 files changed, 61 insertions(+), 12 deletions(-) diff --git a/src/backend/executor/execPartition.c b/src/backend/executor/execPartition.c index 011e3cff1a..11f1ec4bcb 100644 --- a/src/backend/executor/execPartition.c +++ b/src/backend/executor/execPartition.c @@ -253,16 +253,25 @@ ExecFindPartition(ResultRelInfo *resultRelInfo, PartitionDispatch *pd, partdesc = RelationGetPartitionDesc(rel); /* - * Convert the tuple to this parent's layout, if different from the - * current relation. + * Use the slot dedicated to this level's parent. All parents except + * the root have a dedicated slot. For the root parent, we just use + * the original input slot. */ - myslot = dispatch->tupslot; - if (myslot != NULL && map != NULL) + myslot = dispatch->tupslot == NULL ? slot : dispatch->tupslot; + + /* + * If this level's parent's tuple layout is different from the + * previous level's parent, convert the tuple and store it into its + * dedicated slot. + */ + if (myslot != slot) { - tuple = do_convert_tuple(tuple, map); + if (map != NULL) + tuple = do_convert_tuple(tuple, map); ExecStoreTuple(tuple, myslot, InvalidBuffer, true); - slot = myslot; } + else + Assert(map == NULL); /* * Extract partition key from tuple. Expression evaluation machinery @@ -272,8 +281,8 @@ ExecFindPartition(ResultRelInfo *resultRelInfo, PartitionDispatch *pd, * partitioning level has different tuple descriptor from the parent. * So update ecxt_scantuple accordingly. */ - ecxt->ecxt_scantuple = slot; - FormPartitionKeyDatum(dispatch, slot, estate, values, isnull); + ecxt->ecxt_scantuple = myslot; + FormPartitionKeyDatum(dispatch, myslot, estate, values, isnull); /* * Nothing for get_partition_for_tuple() to do if there are no @@ -309,10 +318,14 @@ ExecFindPartition(ResultRelInfo *resultRelInfo, PartitionDispatch *pd, dispatch = pd[-dispatch->indexes[cur_index]]; /* - * Release the dedicated slot, if it was used. Create a copy of - * the tuple first, for the next iteration. + * Make a copy of the tuple for the next level of routing. If + * this level's parent had a dedicated slot, we must clear its + * tuple too, which would be the copy we made in the last + * iteration. */ - if (slot == myslot) + if (myslot == slot) + tuple = ExecCopySlotTuple(myslot); + else { tuple = ExecCopySlotTuple(myslot); ExecClearTuple(myslot); @@ -321,7 +334,7 @@ ExecFindPartition(ResultRelInfo *resultRelInfo, PartitionDispatch *pd, } /* Release the tuple in the lowest parent's dedicated slot. */ - if (slot == myslot) + if (myslot != slot) ExecClearTuple(myslot); /* A partition was not found. */ diff --git a/src/test/regress/expected/insert.out b/src/test/regress/expected/insert.out index 5edf269367..1eaa9879f5 100644 --- a/src/test/regress/expected/insert.out +++ b/src/test/regress/expected/insert.out @@ -927,3 +927,24 @@ insert into returningwrtest values (2, 'foo') returning returningwrtest; (1 row) drop table returningwrtest; +-- BUG #15733 +create table multilevelparted (a int, b int, c int, d int) partition by list (a); +create table multilevelparted_1 partition of multilevelparted for values in (1) partition by list (b); +create table multilevelparted_1_1 partition of multilevelparted_1 for values in (1) partition by list (c); +create table multilevelparted_1_1_1 partition of multilevelparted_1_1 for values in (1); +insert into multilevelparted values (1, 1, 1, 1); +alter table multilevelparted drop d; +create table multilevelparted_2 partition of multilevelparted for values in (2) partition by list (b); +create table multilevelparted_2_1 partition of multilevelparted_2 for values in (1) partition by list (c); +create table multilevelparted_2_1_1 partition of multilevelparted_2_1 for values in (1); +insert into multilevelparted values (2, 1, 1); +insert into multilevelparted values (1, 1, 1); +select tableoid::regclass, * from multilevelparted order by a, b, c; + tableoid | a | b | c +------------------------+---+---+--- + multilevelparted_1_1_1 | 1 | 1 | 1 + multilevelparted_1_1_1 | 1 | 1 | 1 + multilevelparted_2_1_1 | 2 | 1 | 1 +(3 rows) + +drop table multilevelparted; diff --git a/src/test/regress/sql/insert.sql b/src/test/regress/sql/insert.sql index a7f659bc2b..a86f9500ee 100644 --- a/src/test/regress/sql/insert.sql +++ b/src/test/regress/sql/insert.sql @@ -569,3 +569,18 @@ alter table returningwrtest2 drop c; alter table returningwrtest attach partition returningwrtest2 for values in (2); insert into returningwrtest values (2, 'foo') returning returningwrtest; drop table returningwrtest; + +-- BUG #15733 +create table multilevelparted (a int, b int, c int, d int) partition by list (a); +create table multilevelparted_1 partition of multilevelparted for values in (1) partition by list (b); +create table multilevelparted_1_1 partition of multilevelparted_1 for values in (1) partition by list (c); +create table multilevelparted_1_1_1 partition of multilevelparted_1_1 for values in (1); +insert into multilevelparted values (1, 1, 1, 1); +alter table multilevelparted drop d; +create table multilevelparted_2 partition of multilevelparted for values in (2) partition by list (b); +create table multilevelparted_2_1 partition of multilevelparted_2 for values in (1) partition by list (c); +create table multilevelparted_2_1_1 partition of multilevelparted_2_1 for values in (1); +insert into multilevelparted values (2, 1, 1); +insert into multilevelparted values (1, 1, 1); +select tableoid::regclass, * from multilevelparted order by a, b, c; +drop table multilevelparted; -- 2.11.0