Ignore whole-rows in INSERT/CONFLICT with partitioned tables

We had an Assert() preventing whole-row expressions from being used in
the SET clause of INSERT ON CONFLICT, but it seems unnecessary, given
some tests, so remove it.  Add a new test to exercise the case.

Still at ExecInitPartitionInfo, we used map_partition_varattnos (which
constructs an attribute map, then calls map_variable_attnos) using
the same two relations many times in different expressions and with
different parameters.  Constructing the map over and over is a waste.
To avoid this repeated work, construct the map once, and use
map_variable_attnos() directly instead.

Author: Amit Langote, per comments by me (Álvaro)
Discussion: https://postgr.es/m/20180326142016[email protected]

diff --git a/src/backend/executor/execPartition.c b/src/backend/executor/execPartition.c
index 218645d43b887f1d2e6a2071a06ed81a4f473220..23a74bc3d98cbb5cbcd21346202d0f34b3b53bbb 100644 (file)
--- a/src/backend/executor/execPartition.c
+++ b/src/backend/executor/execPartition.c
@@ -24,6 +24,7 @@
 #include "nodes/makefuncs.h"
 #include "partitioning/partbounds.h"
 #include "partitioning/partprune.h"
+#include "rewrite/rewriteManip.h"
 #include "utils/lsyscache.h"
 #include "utils/partcache.h"
 #include "utils/rel.h"
@@ -307,8 +308,12 @@ ExecInitPartitionInfo(ModifyTableState *mtstate,
    ModifyTable *node = (ModifyTable *) mtstate->ps.plan;
    Relation    rootrel = resultRelInfo->ri_RelationDesc,
                partrel;
+   Relation    firstResultRel = mtstate->resultRelInfo[0].ri_RelationDesc;
    ResultRelInfo *leaf_part_rri;
    MemoryContext oldContext;
+   AttrNumber *part_attnos = NULL;
+   bool        found_whole_row;
+   bool        equalTupdescs;
 
    /*
     * We locked all the partitions in ExecSetupPartitionTupleRouting
@@ -356,6 +361,10 @@ ExecInitPartitionInfo(ModifyTableState *mtstate,
                        (node != NULL &&
                         node->onConflictAction != ONCONFLICT_NONE));
 
+   /* if tuple descs are identical, we don't need to map the attrs */
+   equalTupdescs = equalTupleDescs(RelationGetDescr(partrel),
+                                   RelationGetDescr(firstResultRel));
+
    /*
     * Build WITH CHECK OPTION constraints for the partition.  Note that we
     * didn't build the withCheckOptionList for partitions within the planner,
@@ -369,7 +378,6 @@ ExecInitPartitionInfo(ModifyTableState *mtstate,
        List       *wcoExprs = NIL;
        ListCell   *ll;
        int         firstVarno = mtstate->resultRelInfo[0].ri_RangeTableIndex;
-       Relation    firstResultRel = mtstate->resultRelInfo[0].ri_RelationDesc;
 
        /*
         * In the case of INSERT on a partitioned table, there is only one
@@ -397,8 +405,22 @@ ExecInitPartitionInfo(ModifyTableState *mtstate,
        /*
         * Convert Vars in it to contain this partition's attribute numbers.
         */
-       wcoList = map_partition_varattnos(wcoList, firstVarno,
-                                         partrel, firstResultRel, NULL);
+       if (!equalTupdescs)
+       {
+           part_attnos =
+               convert_tuples_by_name_map(RelationGetDescr(partrel),
+                                          RelationGetDescr(firstResultRel),
+                                          gettext_noop("could not convert row type"));
+           wcoList = (List *)
+               map_variable_attnos((Node *) wcoList,
+                                   firstVarno, 0,
+                                   part_attnos,
+                                   RelationGetDescr(firstResultRel)->natts,
+                                   RelationGetForm(partrel)->reltype,
+                                   &found_whole_row);
+           /* We ignore the value of found_whole_row. */
+       }
+
        foreach(ll, wcoList)
        {
            WithCheckOption *wco = castNode(WithCheckOption, lfirst(ll));
@@ -425,7 +447,6 @@ ExecInitPartitionInfo(ModifyTableState *mtstate,
        ExprContext *econtext;
        List       *returningList;
        int         firstVarno = mtstate->resultRelInfo[0].ri_RangeTableIndex;
-       Relation    firstResultRel = mtstate->resultRelInfo[0].ri_RelationDesc;
 
        /* See the comment above for WCO lists. */
        Assert((node->operation == CMD_INSERT &&
@@ -443,12 +464,26 @@ ExecInitPartitionInfo(ModifyTableState *mtstate,
         */
        returningList = linitial(node->returningLists);
 
-       /*
-        * Convert Vars in it to contain this partition's attribute numbers.
-        */
-       returningList = map_partition_varattnos(returningList, firstVarno,
-                                               partrel, firstResultRel,
-                                               NULL);
+       if (!equalTupdescs)
+       {
+           /*
+            * Convert Vars in it to contain this partition's attribute numbers.
+            */
+           if (part_attnos == NULL)
+               part_attnos =
+                   convert_tuples_by_name_map(RelationGetDescr(partrel),
+                                              RelationGetDescr(firstResultRel),
+                                              gettext_noop("could not convert row type"));
+           returningList = (List *)
+               map_variable_attnos((Node *) returningList,
+                                   firstVarno, 0,
+                                   part_attnos,
+                                   RelationGetDescr(firstResultRel)->natts,
+                                   RelationGetForm(partrel)->reltype,
+                                   &found_whole_row);
+           /* We ignore the value of found_whole_row. */
+       }
+
        leaf_part_rri->ri_returningList = returningList;
 
        /*
@@ -473,7 +508,6 @@ ExecInitPartitionInfo(ModifyTableState *mtstate,
    {
        TupleConversionMap *map = proute->parent_child_tupconv_maps[partidx];
        int         firstVarno = mtstate->resultRelInfo[0].ri_RangeTableIndex;
-       Relation    firstResultRel = mtstate->resultRelInfo[0].ri_RelationDesc;
        TupleDesc   partrelDesc = RelationGetDescr(partrel);
        ExprContext *econtext = mtstate->ps.ps_ExprContext;
        ListCell   *lc;
@@ -549,17 +583,33 @@ ExecInitPartitionInfo(ModifyTableState *mtstate,
                 * target relation (firstVarno).
                 */
                onconflset = (List *) copyObject((Node *) node->onConflictSet);
-               onconflset =
-                   map_partition_varattnos(onconflset, INNER_VAR, partrel,
-                                           firstResultRel, &found_whole_row);
-               Assert(!found_whole_row);
-               onconflset =
-                   map_partition_varattnos(onconflset, firstVarno, partrel,
-                                           firstResultRel, &found_whole_row);
-               Assert(!found_whole_row);
-
-               /* Finally, adjust this tlist to match the partition. */
-               onconflset = adjust_partition_tlist(onconflset, map);
+               if (!equalTupdescs)
+               {
+                   if (part_attnos == NULL)
+                       part_attnos =
+                           convert_tuples_by_name_map(RelationGetDescr(partrel),
+                                                      RelationGetDescr(firstResultRel),
+                                                      gettext_noop("could not convert row type"));
+                   onconflset = (List *)
+                       map_variable_attnos((Node *) onconflset,
+                                           INNER_VAR, 0,
+                                           part_attnos,
+                                           RelationGetDescr(firstResultRel)->natts,
+                                           RelationGetForm(partrel)->reltype,
+                                           &found_whole_row);
+                   /* We ignore the value of found_whole_row. */
+                   onconflset = (List *)
+                       map_variable_attnos((Node *) onconflset,
+                                           firstVarno, 0,
+                                           part_attnos,
+                                           RelationGetDescr(firstResultRel)->natts,
+                                           RelationGetForm(partrel)->reltype,
+                                           &found_whole_row);
+                   /* We ignore the value of found_whole_row. */
+
+                   /* Finally, adjust this tlist to match the partition. */
+                   onconflset = adjust_partition_tlist(onconflset, map);
+               }
 
                /*
                 * Build UPDATE SET's projection info.  The user of this
@@ -587,14 +637,25 @@ ExecInitPartitionInfo(ModifyTableState *mtstate,
                    List       *clause;
 
                    clause = copyObject((List *) node->onConflictWhere);
-                   clause = map_partition_varattnos(clause, INNER_VAR,
-                                                    partrel, firstResultRel,
-                                                    &found_whole_row);
-                   Assert(!found_whole_row);
-                   clause = map_partition_varattnos(clause, firstVarno,
-                                                    partrel, firstResultRel,
-                                                    &found_whole_row);
-                   Assert(!found_whole_row);
+                   if (!equalTupdescs)
+                   {
+                       clause = (List *)
+                           map_variable_attnos((Node *) clause,
+                                               INNER_VAR, 0,
+                                               part_attnos,
+                                               RelationGetDescr(firstResultRel)->natts,
+                                               RelationGetForm(partrel)->reltype,
+                                               &found_whole_row);
+                       /* We ignore the value of found_whole_row. */
+                       clause = (List *)
+                           map_variable_attnos((Node *) clause,
+                                               firstVarno, 0,
+                                               part_attnos,
+                                               RelationGetDescr(firstResultRel)->natts,
+                                               RelationGetForm(partrel)->reltype,
+                                               &found_whole_row);
+                       /* We ignore the value of found_whole_row. */
+                   }
                    leaf_part_rri->ri_onConflict->oc_WhereClause =
                        ExecInitQual((List *) clause, &mtstate->ps);
                }

diff --git a/src/test/regress/expected/insert_conflict.out b/src/test/regress/expected/insert_conflict.out
index 2d7061fa1b0002d586c0ec40c7c4d7fdf168772e..b27ad25d378abad634328520f7f25172832fce1c 100644 (file)
--- a/src/test/regress/expected/insert_conflict.out
+++ b/src/test/regress/expected/insert_conflict.out
@@ -884,4 +884,20 @@ insert into parted_conflict values (40, 'forty');
 insert into parted_conflict_1 values (40, 'cuarenta')
   on conflict (a) do update set b = excluded.b;
 ERROR:  there is no unique or exclusion constraint matching the ON CONFLICT specification
+-- test whole-row Vars in ON CONFLICT expressions
+create unique index on parted_conflict (a, b);
+alter table parted_conflict add c int;
+truncate parted_conflict;
+insert into parted_conflict values (50, 'cincuenta', 1);
+insert into parted_conflict values (50, 'cincuenta', 2)
+  on conflict (a, b) do update set (a, b, c) = row(excluded.*)
+  where parted_conflict = (50, text 'cincuenta', 1) and
+        excluded = (50, text 'cincuenta', 2);
+-- should see (50, 'cincuenta', 2)
+select * from parted_conflict order by a;
+ a  |     b     | c 
+----+-----------+---
+ 50 | cincuenta | 2
+(1 row)
+
 drop table parted_conflict;

diff --git a/src/test/regress/sql/insert_conflict.sql b/src/test/regress/sql/insert_conflict.sql
index 6c50fd61eb69f29f87ff4190baba6451be57de58..c677d70fb7b6c5b200da2df841a77bee0fa7edce 100644 (file)
--- a/src/test/regress/sql/insert_conflict.sql
+++ b/src/test/regress/sql/insert_conflict.sql
@@ -559,3 +559,21 @@ insert into parted_conflict values (40, 'forty');
 insert into parted_conflict_1 values (40, 'cuarenta')
   on conflict (a) do update set b = excluded.b;
 drop table parted_conflict;
+
+-- test whole-row Vars in ON CONFLICT expressions
+create table parted_conflict (a int, b text, c int) partition by range (a);
+create table parted_conflict_1 (drp text, c int, a int, b text);
+alter table parted_conflict_1 drop column drp;
+create unique index on parted_conflict (a, b);
+alter table parted_conflict attach partition parted_conflict_1 for values from (0) to (1000);
+truncate parted_conflict;
+insert into parted_conflict values (50, 'cincuenta', 1);
+insert into parted_conflict values (50, 'cincuenta', 2)
+  on conflict (a, b) do update set (a, b, c) = row(excluded.*)
+  where parted_conflict = (50, text 'cincuenta', 1) and
+        excluded = (50, text 'cincuenta', 2);
+
+-- should see (50, 'cincuenta', 2)
+select * from parted_conflict order by a;
+
+drop table parted_conflict;