Fix ON CONFLICT bugs that manifest when used in rules.

Specifically the tlist and rti of the pseudo "excluded" relation weren't
properly treated by expression_tree_walker, which lead to errors when
excluded was referenced inside a rule because the varnos where not
properly adjusted.  Similar omissions in OffsetVarNodes and
expression_tree_mutator had less impact, but should obviously be fixed
nonetheless.

A couple tests of for ON CONFLICT UPDATE into INSERT rule bearing
relations have been added.

In passing I updated a couple comments.

diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index aec415109468d5ff6eebffcf23fcf8875f07a47d..89f1f57ae3dcce4ac684196db4bb7188a0838f7e 100644 (file)
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -1675,6 +1675,7 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
        ExecSetSlotDescriptor(mtstate->mt_existing,
                              resultRelInfo->ri_RelationDesc->rd_att);
 
+       /* carried forward solely for the benefit of explain */
        mtstate->mt_excludedtlist = node->exclRelTlist;
 
        /* create target slot for UPDATE SET projection */

diff --git a/src/backend/nodes/nodeFuncs.c b/src/backend/nodes/nodeFuncs.c
index 4135f9c3cfc17f662034e60de4a09647a03824fe..eac02159236806223877b350aa3ce1fb42ffbf1f 100644 (file)
--- a/src/backend/nodes/nodeFuncs.c
+++ b/src/backend/nodes/nodeFuncs.c
@@ -1922,6 +1922,8 @@ expression_tree_walker(Node *node,
                    return true;
                if (walker(onconflict->onConflictWhere, context))
                    return true;
+               if (walker(onconflict->exclRelTlist, context))
+                   return true;
            }
            break;
        case T_JoinExpr:
@@ -2642,6 +2644,7 @@ expression_tree_mutator(Node *node,
                MUTATE(newnode->arbiterWhere, oc->arbiterWhere, Node *);
                MUTATE(newnode->onConflictSet, oc->onConflictSet, List *);
                MUTATE(newnode->onConflictWhere, oc->onConflictWhere, Node *);
+               MUTATE(newnode->exclRelTlist, oc->exclRelTlist, List *);
 
                return (Node *) newnode;
            }

diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
index fac51c9147470556bf23a063fc375e5f52f715ce..517409d28a07d7133186e22b6846f5ad3f24fbd9 100644 (file)
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -740,9 +740,9 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset)
 
                /*
                 * We treat ModifyTable with ON CONFLICT as a form of 'pseudo
-                * join', where the inner side is the EXLUDED tuple. Therefore
-                * use fix_join_expr to setup the relevant variables to
-                * INNER_VAR. We explicitly don't create any OUTER_VARs as
+                * join', where the inner side is the EXCLUDED tuple.
+                * Therefore use fix_join_expr to setup the relevant variables
+                * to INNER_VAR. We explicitly don't create any OUTER_VARs as
                 * those are already used by RETURNING and it seems better to
                 * be non-conflicting.
                 */
@@ -763,6 +763,9 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset)
                                      NULL, itlist,
                                      linitial_int(splan->resultRelations),
                                      rtoffset);
+
+                   splan->exclRelTlist =
+                       fix_scan_list(root, splan->exclRelTlist, rtoffset);
                }
 
                splan->nominalRelation += rtoffset;

diff --git a/src/backend/rewrite/rewriteManip.c b/src/backend/rewrite/rewriteManip.c
index df457080fea689621335fa4adb8a6486481bc03a..a9c6e626ba787d7a9e1f4ed5d4c126e3e1fc7516 100644 (file)
--- a/src/backend/rewrite/rewriteManip.c
+++ b/src/backend/rewrite/rewriteManip.c
@@ -426,9 +426,9 @@ OffsetVarNodes(Node *node, int offset, int sublevels_up)
        /*
         * If we are starting at a Query, and sublevels_up is zero, then we
         * must also fix rangetable indexes in the Query itself --- namely
-        * resultRelation and rowMarks entries.  sublevels_up cannot be zero
-        * when recursing into a subquery, so there's no need to have the same
-        * logic inside OffsetVarNodes_walker.
+        * resultRelation, exclRelIndex and rowMarks entries.  sublevels_up
+        * cannot be zero when recursing into a subquery, so there's no need
+        * to have the same logic inside OffsetVarNodes_walker.
         */
        if (sublevels_up == 0)
        {
@@ -436,6 +436,10 @@ OffsetVarNodes(Node *node, int offset, int sublevels_up)
 
            if (qry->resultRelation)
                qry->resultRelation += offset;
+
+           if (qry->onConflict && qry->onConflict->exclRelIndex)
+               qry->onConflict->exclRelIndex += offset;
+
            foreach(l, qry->rowMarks)
            {
                RowMarkClause *rc = (RowMarkClause *) lfirst(l);
@@ -617,6 +621,11 @@ ChangeVarNodes(Node *node, int rt_index, int new_index, int sublevels_up)
 
            if (qry->resultRelation == rt_index)
                qry->resultRelation = new_index;
+
+           /* this is unlikely to ever be used, but ... */
+           if (qry->onConflict && qry->onConflict->exclRelIndex == rt_index)
+               qry->onConflict->exclRelIndex = new_index;
+
            foreach(l, qry->rowMarks)
            {
                RowMarkClause *rc = (RowMarkClause *) lfirst(l);

diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index a379a7279c5dad46c00b197f813c6a1a2c589742..cb18bb931a4870b1487842de9b03f398b1b8f242 100644 (file)
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -2817,25 +2817,112 @@ CREATE RULE hat_upsert AS ON INSERT TO hats
     INSERT INTO hat_data VALUES (
            NEW.hat_name,
            NEW.hat_color)
-        ON CONFLICT (hat_name) DO UPDATE SET hat_color = 'Orange' RETURNING *;
+        ON CONFLICT (hat_name)
+        DO UPDATE
+           SET hat_name = hat_data.hat_name, hat_color = excluded.hat_color
+           WHERE excluded.hat_color <>  'forbidden'
+        RETURNING *;
 -- Works (does upsert)
-INSERT INTO hats VALUES ('h7', 'black') RETURNING *;
+INSERT INTO hats VALUES ('h8', 'black') RETURNING *;
+  hat_name  | hat_color  
+------------+------------
+ h8         | black     
+(1 row)
+
+SELECT * FROM hat_data WHERE hat_name = 'h8';
+  hat_name  | hat_color  
+------------+------------
+ h8         | black     
+(1 row)
+
+INSERT INTO hats VALUES ('h8', 'white') RETURNING *;
+  hat_name  | hat_color  
+------------+------------
+ h8         | white     
+(1 row)
+
+SELECT * FROM hat_data WHERE hat_name = 'h8';
+  hat_name  | hat_color  
+------------+------------
+ h8         | white     
+(1 row)
+
+INSERT INTO hats VALUES ('h8', 'forbidden') RETURNING *;
+ hat_name | hat_color 
+----------+-----------
+(0 rows)
+
+SELECT * FROM hat_data WHERE hat_name = 'h8';
   hat_name  | hat_color  
 ------------+------------
- h7         | Orange    
+ h8         | white     
 (1 row)
 
 SELECT tablename, rulename, definition FROM pg_rules
    WHERE tablename = 'hats';
- tablename |  rulename  |                                          definition                                           
------------+------------+-----------------------------------------------------------------------------------------------
- hats      | hat_upsert | CREATE RULE hat_upsert AS                                                                    +
-           |            |     ON INSERT TO hats DO INSTEAD  INSERT INTO hat_data (hat_name, hat_color)                 +
-           |            |   VALUES (new.hat_name, new.hat_color) ON CONFLICT DO UPDATE SET hat_color = 'Orange'::bpchar+
-           |            |   RETURNING hat_data.hat_name,                                                               +
+ tablename |  rulename  |                                                          definition                                                           
+-----------+------------+-------------------------------------------------------------------------------------------------------------------------------
+ hats      | hat_upsert | CREATE RULE hat_upsert AS                                                                                                    +
+           |            |     ON INSERT TO hats DO INSTEAD  INSERT INTO hat_data (hat_name, hat_color)                                                 +
+           |            |   VALUES (new.hat_name, new.hat_color) ON CONFLICT DO UPDATE SET hat_name = hat_data.hat_name, hat_color = excluded.hat_color+
+           |            |   WHERE (excluded.hat_color <> 'forbidden'::bpchar)                                                                          +
+           |            |   RETURNING hat_data.hat_name,                                                                                               +
            |            |     hat_data.hat_color;
 (1 row)
 
+-- ensure explain works for on insert conflict rules
+explain (costs off) INSERT INTO hats VALUES ('h8', 'forbidden') RETURNING *;
+                           QUERY PLAN                           
+----------------------------------------------------------------
+ Insert on hat_data
+   Conflict Resolution: UPDATE
+   Conflict Arbiter Indexes: hat_data_pkey
+   Conflict Filter: (excluded.hat_color <> 'forbidden'::bpchar)
+   ->  Result
+(5 rows)
+
+-- ensure upserting into a rule, with a CTE (different offsets!) works
+WITH data(hat_name, hat_color) AS (
+    VALUES ('h8', 'green'),
+        ('h9', 'blue'),
+        ('h7', 'forbidden')
+)
+INSERT INTO hats
+    SELECT * FROM data
+RETURNING *;
+  hat_name  | hat_color  
+------------+------------
+ h8         | green     
+ h9         | blue      
+(2 rows)
+
+EXPLAIN (costs off) WITH data(hat_name, hat_color) AS (
+    VALUES ('h8', 'green'),
+        ('h9', 'blue'),
+        ('h7', 'forbidden')
+)
+INSERT INTO hats
+    SELECT * FROM data
+RETURNING *;
+                           QUERY PLAN                           
+----------------------------------------------------------------
+ Insert on hat_data
+   Conflict Resolution: UPDATE
+   Conflict Arbiter Indexes: hat_data_pkey
+   Conflict Filter: (excluded.hat_color <> 'forbidden'::bpchar)
+   CTE data
+     ->  Values Scan on "*VALUES*"
+   ->  CTE Scan on data
+(7 rows)
+
+SELECT * FROM hat_data WHERE hat_name IN ('h8', 'h9', 'h7') ORDER BY hat_name;
+  hat_name  | hat_color  
+------------+------------
+ h7         | black     
+ h8         | green     
+ h9         | blue      
+(3 rows)
+
 DROP RULE hat_upsert ON hats;
 drop table hats;
 drop table hat_data;

diff --git a/src/test/regress/sql/rules.sql b/src/test/regress/sql/rules.sql
index 6f1a1b84e79b9754821673b0d8622d2b4df9eafd..1a81155bf1b729691428876f17631a5429d82917 100644 (file)
--- a/src/test/regress/sql/rules.sql
+++ b/src/test/regress/sql/rules.sql
@@ -1074,12 +1074,43 @@ CREATE RULE hat_upsert AS ON INSERT TO hats
     INSERT INTO hat_data VALUES (
            NEW.hat_name,
            NEW.hat_color)
-        ON CONFLICT (hat_name) DO UPDATE SET hat_color = 'Orange' RETURNING *;
+        ON CONFLICT (hat_name)
+        DO UPDATE
+           SET hat_name = hat_data.hat_name, hat_color = excluded.hat_color
+           WHERE excluded.hat_color <>  'forbidden'
+        RETURNING *;
 
 -- Works (does upsert)
-INSERT INTO hats VALUES ('h7', 'black') RETURNING *;
+INSERT INTO hats VALUES ('h8', 'black') RETURNING *;
+SELECT * FROM hat_data WHERE hat_name = 'h8';
+INSERT INTO hats VALUES ('h8', 'white') RETURNING *;
+SELECT * FROM hat_data WHERE hat_name = 'h8';
+INSERT INTO hats VALUES ('h8', 'forbidden') RETURNING *;
+SELECT * FROM hat_data WHERE hat_name = 'h8';
 SELECT tablename, rulename, definition FROM pg_rules
    WHERE tablename = 'hats';
+-- ensure explain works for on insert conflict rules
+explain (costs off) INSERT INTO hats VALUES ('h8', 'forbidden') RETURNING *;
+
+-- ensure upserting into a rule, with a CTE (different offsets!) works
+WITH data(hat_name, hat_color) AS (
+    VALUES ('h8', 'green'),
+        ('h9', 'blue'),
+        ('h7', 'forbidden')
+)
+INSERT INTO hats
+    SELECT * FROM data
+RETURNING *;
+EXPLAIN (costs off) WITH data(hat_name, hat_color) AS (
+    VALUES ('h8', 'green'),
+        ('h9', 'blue'),
+        ('h7', 'forbidden')
+)
+INSERT INTO hats
+    SELECT * FROM data
+RETURNING *;
+SELECT * FROM hat_data WHERE hat_name IN ('h8', 'h9', 'h7') ORDER BY hat_name;
+
 DROP RULE hat_upsert ON hats;
 
 drop table hats;