Fix bug around assignment expressions containing indirections.

Handling of assigned-to expressions with indirection (e.g. set f1[1] =
3) was broken for ON CONFLICT DO UPDATE.  The problem was that
ParseState was consulted to determine if an INSERT-appropriate or
UPDATE-appropriate behavior should be used when transforming expressions
with indirections. When the wrong path was taken the old row was
substituted with NULL, leading to wrong results..

To fix remove p_is_update and only use p_is_insert to decide how to
transform the assignment expression, and uset p_is_insert while parsing
the on conflict statement. This isn't particularly pretty, but it's not
any worse than before.

Author: Peter Geoghegan, slightly edited by me
Discussion: CAM3SWZS8RPvA=KFxADZWw3wAHnnbxMxDzkEC6fNaFc7zSm411w@mail.gmail.com
Backpatch: 9.5, where the feature was introduced

diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index fc463faa6be6459440025a59c7c8f8c1ac636a56..a0dfbf900a924e3c834271275221ec71b9edbfed 100644 (file)
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -891,6 +891,12 @@ transformOnConflictClause(ParseState *pstate,
        /* Process DO UPDATE */
        if (onConflictClause->action == ONCONFLICT_UPDATE)
        {
+               /*
+                * All INSERT expressions have been parsed, get ready for potentially
+                * existing SET statements that need to be processed like an UPDATE.
+                */
+               pstate->p_is_insert = false;
+
                exclRte = addRangeTableEntryForRelation(pstate,
                                                                                                pstate->p_target_relation,
                                                                                                makeAlias("excluded", NIL),
@@ -1999,7 +2005,7 @@ transformUpdateStmt(ParseState *pstate, UpdateStmt *stmt)
        Node       *qual;
 
        qry->commandType = CMD_UPDATE;
-       pstate->p_is_update = true;
+       pstate->p_is_insert = false;
 
        /* process the WITH clause independently of all else */
        if (stmt->withClause)

diff --git a/src/include/parser/parse_node.h b/src/include/parser/parse_node.h
index 3103b71594471110aacae495150f33b4cb0a36db..7ecaffc0dc37d3e8b9310513c6424e87a96fe061 100644 (file)
--- a/src/include/parser/parse_node.h
+++ b/src/include/parser/parse_node.h
@@ -152,7 +152,6 @@ struct ParseState
        bool            p_hasSubLinks;
        bool            p_hasModifyingCTE;
        bool            p_is_insert;
-       bool            p_is_update;
        bool            p_locked_from_parent;
        Relation        p_target_relation;
        RangeTblEntry *p_target_rangetblentry;

diff --git a/src/test/regress/expected/arrays.out b/src/test/regress/expected/arrays.out
index 5f1532f2371e94bb68ab7707cfe62dedc8c7b23e..73fb5a248b47cf181422158c8c6a3bca351c72ca 100644 (file)
--- a/src/test/regress/expected/arrays.out
+++ b/src/test/regress/expected/arrays.out
@@ -1116,6 +1116,27 @@ select * from arr_tbl where f1 >= '{1,2,3}' and f1 < '{1,5,3}';
  {1,2,10}
 (2 rows)
 
+-- test ON CONFLICT DO UPDATE with arrays
+create temp table arr_pk_tbl (pk int4 primary key, f1 int[]);
+insert into arr_pk_tbl values (1, '{1,2,3}');
+insert into arr_pk_tbl values (1, '{3,4,5}') on conflict (pk)
+  do update set f1[1] = excluded.f1[1], f1[3] = excluded.f1[3]
+  returning pk, f1;
+ pk |   f1    
+----+---------
+  1 | {3,2,5}
+(1 row)
+
+insert into arr_pk_tbl(pk, f1[1:2]) values (1, '{6,7,8}') on conflict (pk)
+  do update set f1[1] = excluded.f1[1],
+    f1[2] = excluded.f1[2],
+    f1[3] = excluded.f1[3]
+  returning pk, f1;
+ pk |     f1     
+----+------------
+  1 | {6,7,NULL}
+(1 row)
+
 -- note: if above selects don't produce the expected tuple order,
 -- then you didn't get an indexscan plan, and something is busted.
 reset enable_seqscan;

diff --git a/src/test/regress/sql/arrays.sql b/src/test/regress/sql/arrays.sql
index 562134b2863ae13dd6ae6666e48f49464fde8a58..b1dd65144050b97a1a2d81b667a38ca112d5c192 100644 (file)
--- a/src/test/regress/sql/arrays.sql
+++ b/src/test/regress/sql/arrays.sql
@@ -306,6 +306,19 @@ set enable_seqscan to off;
 set enable_bitmapscan to off;
 select * from arr_tbl where f1 > '{1,2,3}' and f1 <= '{1,5,3}';
 select * from arr_tbl where f1 >= '{1,2,3}' and f1 < '{1,5,3}';
+
+-- test ON CONFLICT DO UPDATE with arrays
+create temp table arr_pk_tbl (pk int4 primary key, f1 int[]);
+insert into arr_pk_tbl values (1, '{1,2,3}');
+insert into arr_pk_tbl values (1, '{3,4,5}') on conflict (pk)
+  do update set f1[1] = excluded.f1[1], f1[3] = excluded.f1[3]
+  returning pk, f1;
+insert into arr_pk_tbl(pk, f1[1:2]) values (1, '{6,7,8}') on conflict (pk)
+  do update set f1[1] = excluded.f1[1],
+    f1[2] = excluded.f1[2],
+    f1[3] = excluded.f1[3]
+  returning pk, f1;
+
 -- note: if above selects don't produce the expected tuple order,
 -- then you didn't get an indexscan plan, and something is busted.
 reset enable_seqscan;