From 72076f565b142debe39eb1e5a6cac27100b76fdb Mon Sep 17 00:00:00 2001 From: Peter Geoghegan Date: Thu, 28 May 2015 00:18:19 -0700 Subject: [PATCH 2/2] Fix bug with assigned-to expressions with indirection Handling of assigned-to expressions with indirection (as used with UPDATE targetlists, where, for example, assigned-to expressions allow array elements to be directly assigned to) could previously become confused. The problem was that ParseState was consulted to determine if an INSERT-appropriate or UPDATE-appropriate behavior should be used, and so for INSERT statements with ON CONFLICT DO UPDATE, an INSERT-targetlist-applicable codepath could incorrectly be taken. To fix, allow ParseState to reflect that an individual statement can be both p_is_insert and p_is_update at the same time. --- src/backend/parser/analyze.c | 3 +++ src/backend/parser/parse_target.c | 3 ++- src/include/parser/parse_node.h | 2 +- src/test/regress/expected/arrays.out | 21 +++++++++++++++++++++ src/test/regress/sql/arrays.sql | 13 +++++++++++++ 5 files changed, 40 insertions(+), 2 deletions(-) diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c index fc463fa..be474dc 100644 --- a/src/backend/parser/analyze.c +++ b/src/backend/parser/analyze.c @@ -891,6 +891,9 @@ transformOnConflictClause(ParseState *pstate, /* Process DO UPDATE */ if (onConflictClause->action == ONCONFLICT_UPDATE) { + /* p_is_update must be set here, after INSERT targetlist processing */ + pstate->p_is_update = true; + exclRte = addRangeTableEntryForRelation(pstate, pstate->p_target_relation, makeAlias("excluded", NIL), diff --git a/src/backend/parser/parse_target.c b/src/backend/parser/parse_target.c index 1b3fcd6..ebde013 100644 --- a/src/backend/parser/parse_target.c +++ b/src/backend/parser/parse_target.c @@ -469,13 +469,14 @@ transformAssignedExpr(ParseState *pstate, { Node *colVar; - if (pstate->p_is_insert) + if (!pstate->p_is_update) { /* * The command is INSERT INTO table (col.something) ... so there * is not really a source value to work with. Insert a NULL * constant as the source value. */ + Assert(pstate->p_is_insert); colVar = (Node *) makeNullConst(attrtype, attrtypmod, attrcollation); } diff --git a/src/include/parser/parse_node.h b/src/include/parser/parse_node.h index 3103b71..e0f5641 100644 --- a/src/include/parser/parse_node.h +++ b/src/include/parser/parse_node.h @@ -152,7 +152,7 @@ struct ParseState bool p_hasSubLinks; bool p_hasModifyingCTE; bool p_is_insert; - bool p_is_update; + bool p_is_update; /* not mutually exclusive with p_is_insert */ 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 5f1532f..73fb5a2 100644 --- 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 562134b..b1dd651 100644 --- 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; -- 1.9.1