From 6b0208ebc436b33bd80ce264299b4b1b8d59b68a Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Mon, 8 Apr 2019 13:45:14 +0900 Subject: [PATCH] Fix partition tuple routing with dropped attributes When trying to insert a tuple into a partitioned table, the routing to the correct partition has been messed up by mixing when a tuple needs to be stored in an intermediate parent's slot and when a tuple needs to be converted because of attribute changes between the immediate parent relation and the parent relation one level above that (the grandparent). This could trigger errors like the following: ERROR: cannot extract attribute from empty tuple slot SQL state: XX000 This was not detected because regression tests with dropped attributes only included tests with two levels of partitioning, and this can be triggered with three levels or more. This fixes bug #15733, which has been introduced by 34295b8. The bug happens only on REL_11_STABLE and HEAD gains the regression tests added for this bug. Reported-by: Petr Fedorov Author: Amit Langote, Michael Paquier Discussion: https://postgr.es/m/15733-7692379e310b80ec@postgresql.org --- src/backend/executor/execPartition.c | 39 ++++++++++++++++---------- src/test/regress/expected/insert.out | 41 ++++++++++++++++++++++++++++ src/test/regress/sql/insert.sql | 28 +++++++++++++++++++ 3 files changed, 93 insertions(+), 15 deletions(-) diff --git a/src/backend/executor/execPartition.c b/src/backend/executor/execPartition.c index 011e3cff1a..8061c7e449 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 the tuple layout of this level's parent 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,19 +318,19 @@ 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) - { - tuple = ExecCopySlotTuple(myslot); + tuple = ExecCopySlotTuple(myslot); + if (myslot != slot) ExecClearTuple(myslot); - } } } /* 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..13f53e3649 100644 --- a/src/test/regress/expected/insert.out +++ b/src/test/regress/expected/insert.out @@ -629,6 +629,47 @@ select tableoid::regclass, * from mlparted_def; mlparted_defd | 70 | 100 | (4 rows) +-- Check multi-level tuple routing with attributes dropped from the +-- top-most parent. First remove the last attribute. +alter table mlparted add d int, add e int; +alter table mlparted drop e; +create table mlparted5 partition of mlparted + for values from (1, 40) to (1, 50) partition by range (c); +create table mlparted5_ab partition of mlparted5 + for values from ('a') to ('c') partition by list (c); +create table mlparted5_a partition of mlparted5_ab for values in ('a'); +create table mlparted5_b (d int, b int, c text, a int); +alter table mlparted5_ab attach partition mlparted5_b for values in ('b'); +truncate mlparted; +insert into mlparted values (1, 2, 'a', 1); +insert into mlparted values (1, 40, 'a', 1); -- goes to mlparted5_a +insert into mlparted values (1, 45, 'b', 1); -- goes to mlparted5_b +select tableoid::regclass, * from mlparted order by a, b, c, d; + tableoid | a | b | c | d +-------------+---+----+---+--- + mlparted11 | 1 | 2 | a | 1 + mlparted5_a | 1 | 40 | a | 1 + mlparted5_b | 1 | 45 | b | 1 +(3 rows) + +alter table mlparted drop d; +truncate mlparted; +-- Remove the before last attribute. +alter table mlparted add e int, add d int; +alter table mlparted drop e; +insert into mlparted values (1, 2, 'a', 1); +insert into mlparted values (1, 40, 'a', 1); -- goes to mlparted5_a +insert into mlparted values (1, 45, 'b', 1); -- goes to mlparted5_b +select tableoid::regclass, * from mlparted order by a, b, c, d; + tableoid | a | b | c | d +-------------+---+----+---+--- + mlparted11 | 1 | 2 | a | 1 + mlparted5_a | 1 | 40 | a | 1 + mlparted5_b | 1 | 45 | b | 1 +(3 rows) + +alter table mlparted drop d; +drop table mlparted5; -- check that message shown after failure to find a partition shows the -- appropriate key description (or none) in various situations create table key_desc (a int, b int) partition by list ((a+0)); diff --git a/src/test/regress/sql/insert.sql b/src/test/regress/sql/insert.sql index a7f659bc2b..4d1c92a54d 100644 --- a/src/test/regress/sql/insert.sql +++ b/src/test/regress/sql/insert.sql @@ -401,6 +401,34 @@ insert into mlparted values (70, 100); select tableoid::regclass, * from mlparted_def; +-- Check multi-level tuple routing with attributes dropped from the +-- top-most parent. First remove the last attribute. +alter table mlparted add d int, add e int; +alter table mlparted drop e; +create table mlparted5 partition of mlparted + for values from (1, 40) to (1, 50) partition by range (c); +create table mlparted5_ab partition of mlparted5 + for values from ('a') to ('c') partition by list (c); +create table mlparted5_a partition of mlparted5_ab for values in ('a'); +create table mlparted5_b (d int, b int, c text, a int); +alter table mlparted5_ab attach partition mlparted5_b for values in ('b'); +truncate mlparted; +insert into mlparted values (1, 2, 'a', 1); +insert into mlparted values (1, 40, 'a', 1); -- goes to mlparted5_a +insert into mlparted values (1, 45, 'b', 1); -- goes to mlparted5_b +select tableoid::regclass, * from mlparted order by a, b, c, d; +alter table mlparted drop d; +truncate mlparted; +-- Remove the before last attribute. +alter table mlparted add e int, add d int; +alter table mlparted drop e; +insert into mlparted values (1, 2, 'a', 1); +insert into mlparted values (1, 40, 'a', 1); -- goes to mlparted5_a +insert into mlparted values (1, 45, 'b', 1); -- goes to mlparted5_b +select tableoid::regclass, * from mlparted order by a, b, c, d; +alter table mlparted drop d; +drop table mlparted5; + -- check that message shown after failure to find a partition shows the -- appropriate key description (or none) in various situations create table key_desc (a int, b int) partition by list ((a+0));