Fix partition tuple routing with dropped attributes
authorMichael Paquier
Mon, 8 Apr 2019 04:45:14 +0000 (13:45 +0900)
committerMichael Paquier
Mon, 8 Apr 2019 04:45:14 +0000 (13:45 +0900)
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
src/test/regress/expected/insert.out
src/test/regress/sql/insert.sql

index 011e3cff1ad0e098ea73de7cf076c1a1f8d3a9f0..8061c7e449d08ae46458a57f44a9451fd49f4f90 100644 (file)
@@ -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. */
index 5edf2693679921a59bbadae83bf9a6fc7c5835bb..13f53e36490b14611a619be1cf0705c2af6633de 100644 (file)
@@ -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));
index a7f659bc2b4103962b873562a7373558bd068d3f..4d1c92a54df990c4f9c5314f9f043295ed87d2ed 100644 (file)
@@ -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));