From 6bebd4f4bf72191fff11724eeeb279e02e086e60 Mon Sep 17 00:00:00 2001 From: amit Date: Fri, 5 Apr 2019 11:33:27 +0900 Subject: [PATCH v2 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 | 12 ++++++------ 2 files changed, 31 insertions(+), 18 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 2752946e02..1482cd28a3 100644 --- a/src/test/regress/expected/insert.out +++ b/src/test/regress/expected/insert.out @@ -640,14 +640,14 @@ alter table mlparted5ab attach partition mlparted5b for values in ('b'); truncate mlparted; insert into mlparted values (1, 2, 'a', 1); insert into mlparted values (1, 40, 'a', 1); -ERROR: cannot extract attribute from empty tuple slot insert into mlparted values (1, 45, 'b', 1); -ERROR: cannot extract attribute from empty tuple slot select tableoid::regclass, * from mlparted order by a, b, c, d; - tableoid | a | b | c | d -------------+---+---+---+--- - mlparted11 | 1 | 2 | a | 1 -(1 row) + tableoid | a | b | c | d +------------+---+----+---+--- + mlparted11 | 1 | 2 | a | 1 + mlparted5a | 1 | 40 | a | 1 + mlparted5b | 1 | 45 | b | 1 +(3 rows) drop table mlparted5; -- check that message shown after failure to find a partition shows the -- 2.11.0