From 7dc9b222d8145ca0f44599e0034efec50cb4683c Mon Sep 17 00:00:00 2001 From: amit Date: Fri, 5 Apr 2019 11:33:27 +0900 Subject: [PATCH v3 2/2] 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 | 8 ++++---- 2 files changed, 29 insertions(+), 16 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 476db316ef..e221803285 100644 --- a/src/test/regress/expected/insert.out +++ b/src/test/regress/expected/insert.out @@ -607,11 +607,11 @@ create table mlparted5bc partition of mlparted5 for values in ('b', 'c') partiti create table mlparted5b partition of mlparted5bc for values in ('b'); truncate mlparted; insert into mlparted values (1, 42, 'b'); -ERROR: cannot extract attribute from empty tuple slot select * from mlparted5b; - c | a | b ----+---+--- -(0 rows) + c | a | b +---+---+---- + b | 1 | 42 +(1 row) drop table mlparted5; alter table mlparted drop constraint check_b; -- 2.11.0