On Sun, May 24, 2015 at 6:17 PM, Peter Geoghegan <p...@heroku.com> wrote:
> Attached patch adds such a pfree() call.

Attached, revised version incorporates this small fix, while adding an
additional big fix, and a number of small style tweaks.

This is mainly concerned with fixing the bug I was trying to fix when
I spotted the minor pfree() issue:

postgres=# insert into upsert (key, val) values('Foo', 'Bar') on
conflict (key) do update set val = excluded.val where excluded.* is
not null;
ERROR:  XX000: variable not found in subplan target lists
LOCATION:  fix_join_expr_mutator, setrefs.c:2003
postgres=# insert into upsert (key, val) values(('Foo', 'Bar') on
conflict (key) do update set val = excluded.val where excluded.ctid is
not null;
ERROR:  XX000: variable not found in subplan target lists
LOCATION:  fix_join_expr_mutator, setrefs.c:2003

The first query shown should clearly finish processing by the
optimizer without raising this error message (execution should work
correctly too, of course). The second query shown should fail with a
user visible error message about referencing the excluded
pseudo-relation's ctid column not making sense.

The basic problem is that there wasn't much thought put into how the
excluded pseudo-relation's "reltargetlist" is generated -- it
currently comes from a call to expandRelAttrs() during parse analysis,
which, on its own, doesn't allow whole row Vars to work.

One approach to fixing this is to follow the example of RETURNING
lists with references to more than one relation:
preprocess_targetlist() handles this by calling pull_var_clause() and
making new TargetEntry entries for Vars found to not be referencing
the target (and not already in the targetlist for some other reason).
Another approach, preferred by Andres, is to have query_planner() do
more. I understand that the idea there is to make excluded.* closer to
a regular table, in that it can be expected to have a baserel, and
within the executor we have something closer to a bona-fide scan
reltargetlist, that we can expect to have all Vars appear in. This
should be enough to make the reltargetlist have the appropriate Vars
more or less in the regular course of planning, including excluded.*
whole row Vars.  To make this work we could call
add_vars_to_targetlist(), and call add_base_rels_to_query()  and then
build_base_rel_tlists() within query_planner() (while moving a few
other things around).

However, the ordering dependencies within query_planner() seemed quite
complicated to me, and I didn't want to modify such an important
routine just to fix this bug. RETURNING seemed like a perfectly good
precedent to follow, so that's what I did. Now, it might have been
that  I misunderstood Andres when we discussed this problem on
Jabber/IM, but ISTM that the second idea doesn't have much advantage
over the first (I'm sure that Andres will be able to explain what he'd
like to do better here -- it was a quick conversation). I did
prototype the second approach, and the code footprint of what I have
here (the first approach) seems lower than it would have to be with
the second. Besides, I didn't see a convenient choke point to reject
system column references with the second approach. Attached patch
fixes the bug using the first approach. Tests were added demonstrating
that the cases above are fixed.

A second attached patch fixes another, largely independent bug. I
noticed array assignment with ON CONFLICT DO UPDATE was broken. See
commit message for full details.

Thoughts?
-- 
Peter Geoghegan
From 72076f565b142debe39eb1e5a6cac27100b76fdb Mon Sep 17 00:00:00 2001
From: Peter Geoghegan <peter.geoghega...@gmail.com>
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

From 39c85acd7b9c11d6b7c6612459bb4fb8f36ef45b Mon Sep 17 00:00:00 2001
From: Peter Geoghegan <peter.geoghega...@gmail.com>
Date: Mon, 25 May 2015 01:08:30 -0700
Subject: [PATCH 1/2] Fix bug with whole row Vars in excluded targetlist

Follow the approach taken with RETURNING clauses containing references
to multiple relations;  pull up Vars referencing non-target relations
(limited here to the ON CONFLICT DO UPDATE excluded pseudo-relation)
that appear in ON CONFLICT DO UPDATE, and add the Vars to the excluded
relation targetlist as resjunk.

Have setrefs.c infrastructure avoid changing the varattno of whole row
Var resjunk attributes from ON CONFLICT related fix_join_expr() calls.
Failing to do so would now cause the executor to misidentify the Var as
a scalar Var rather than a whole row Var (it specifically recognizes
"varattno == InvalidAttrNumber" as representing the whole row case).

setrefs.c is also changed to call build_tlist_index_other_vars() rather
than build_tlist_index();  this isn't important for correctness, but it
seems preferable to make the code more consistent with the well
established set_returning_clause_references() case.

In passing, make a few minor stylistic tweaks, and reject Vars
referencing the excluded.* pseudo relation in an invalid manner.  These
system column Vars are never useful or meaningful, since, of course,
excluded is not a real table.
---
 src/backend/executor/execMain.c               |  2 +
 src/backend/executor/nodeModifyTable.c        |  9 +++-
 src/backend/optimizer/plan/planner.c          | 14 +++--
 src/backend/optimizer/plan/setrefs.c          | 38 +++++++++++---
 src/backend/optimizer/prep/preptlist.c        | 74 +++++++++++++++++++++++++--
 src/include/optimizer/prep.h                  |  4 +-
 src/test/regress/expected/insert_conflict.out | 39 ++++++++++++++
 src/test/regress/sql/insert_conflict.sql      | 22 ++++++++
 8 files changed, 179 insertions(+), 23 deletions(-)

diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index a1561ce..f2c0c64 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -1246,6 +1246,8 @@ InitResultRelInfo(ResultRelInfo *resultRelInfo,
 	resultRelInfo->ri_ConstraintExprs = NULL;
 	resultRelInfo->ri_junkFilter = NULL;
 	resultRelInfo->ri_projectReturning = NULL;
+	resultRelInfo->ri_onConflictSetProj = NULL;
+	resultRelInfo->ri_onConflictSetWhere = NIL;
 }
 
 /*
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index 874ca6a..30a092b 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -1505,13 +1505,18 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
 	mtstate->resultRelInfo = estate->es_result_relations + node->resultRelIndex;
 	mtstate->mt_arowmarks = (List **) palloc0(sizeof(List *) * nplans);
 	mtstate->mt_nplans = nplans;
-	mtstate->mt_onconflict = node->onConflictAction;
-	mtstate->mt_arbiterindexes = node->arbiterIndexes;
 
 	/* set up epqstate with dummy subplan data for the moment */
 	EvalPlanQualInit(&mtstate->mt_epqstate, estate, NULL, NIL, node->epqParam);
 	mtstate->fireBSTriggers = true;
 
+	/* initialize ON CONFLICT data */
+	mtstate->mt_onconflict = node->onConflictAction;
+	mtstate->mt_arbiterindexes = node->arbiterIndexes;
+	mtstate->mt_existing = NULL;
+	mtstate->mt_excludedtlist = NIL;
+	mtstate->mt_conflproj = NULL;
+
 	/*
 	 * call ExecInitNode on each of the plans to be executed and save the
 	 * results into the array "mt_plans".  This is also a convenient place to
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index 8afde2b..4ce6ee85 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -488,7 +488,7 @@ subquery_planner(PlannerGlobal *glob, Query *parse,
 								  EXPRKIND_TARGET);
 
 		parse->onConflict->onConflictWhere =
-			preprocess_expression(root, (Node *) parse->onConflict->onConflictWhere,
+			preprocess_expression(root, parse->onConflict->onConflictWhere,
 								  EXPRKIND_QUAL);
 	}
 
@@ -1273,7 +1273,6 @@ grouping_planner(PlannerInfo *root, double tuple_fraction)
 		bool		use_hashed_grouping = false;
 		WindowFuncLists *wflists = NULL;
 		List	   *activeWindows = NIL;
-		OnConflictExpr *onconfl;
 		int			maxref = 0;
 		int		   *tleref_to_colnum_map;
 		List	   *rollup_lists = NIL;
@@ -1364,12 +1363,11 @@ grouping_planner(PlannerInfo *root, double tuple_fraction)
 		/* Preprocess targetlist */
 		tlist = preprocess_targetlist(root, tlist);
 
-		onconfl = parse->onConflict;
-		if (onconfl)
-			onconfl->onConflictSet =
-				preprocess_onconflict_targetlist(onconfl->onConflictSet,
-												 parse->resultRelation,
-												 parse->rtable);
+		/* Preprocess targetlist for ON CONFLICT DO UPDATE */
+		if (parse->onConflict)
+			preprocess_onconflict_targetlist(parse->onConflict,
+											 parse->resultRelation,
+											 parse->rtable);
 
 		/*
 		 * Expand any rangetable entries that have security barrier quals.
diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
index a7f65dd..fa683ab 100644
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -33,6 +33,7 @@ typedef struct
 	Index		varno;			/* RT index of Var */
 	AttrNumber	varattno;		/* attr number of Var */
 	AttrNumber	resno;			/* TLE position of Var */
+	bool		resjunk;		/* TLE is resjunk? */
 } tlist_vinfo;
 
 typedef struct
@@ -41,6 +42,7 @@ typedef struct
 	int			num_vars;		/* number of plain Var tlist entries */
 	bool		has_ph_vars;	/* are there PlaceHolderVar entries? */
 	bool		has_non_vars;	/* are there other entries? */
+	bool		change_resjunk;	/* change varattno of resjunk entries? */
 	tlist_vinfo vars[FLEXIBLE_ARRAY_MEMBER];	/* has num_vars entries */
 } indexed_tlist;
 
@@ -106,6 +108,8 @@ static void set_join_references(PlannerInfo *root, Join *join, int rtoffset);
 static void set_upper_references(PlannerInfo *root, Plan *plan, int rtoffset);
 static void set_dummy_tlist_references(Plan *plan, int rtoffset);
 static indexed_tlist *build_tlist_index(List *tlist);
+static indexed_tlist *build_tlist_index_other_vars(List *tlist,
+										 Index ignore_rel);
 static Var *search_indexed_tlist_for_var(Var *var,
 							 indexed_tlist *itlist,
 							 Index newvarno,
@@ -762,7 +766,15 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset)
 				{
 					indexed_tlist *itlist;
 
-					itlist = build_tlist_index(splan->exclRelTlist);
+					itlist = build_tlist_index_other_vars(splan->exclRelTlist,
+														  linitial_int(splan->resultRelations));
+
+					/*
+					 * Don't allow fix_join_expr to change the varattno of
+					 * resjunk TLE's Vars.  This can occur due to whole row Var
+					 * excluded.* references, but for no other reason.
+					 */
+					itlist->change_resjunk = false;
 
 					splan->onConflictSet =
 						fix_join_expr(root, splan->onConflictSet,
@@ -775,6 +787,7 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset)
 									  NULL, itlist,
 									  linitial_int(splan->resultRelations),
 									  rtoffset);
+					pfree(itlist);
 
 					splan->exclRelTlist =
 						fix_scan_list(root, splan->exclRelTlist, rtoffset);
@@ -1722,6 +1735,7 @@ build_tlist_index(List *tlist)
 	itlist->tlist = tlist;
 	itlist->has_ph_vars = false;
 	itlist->has_non_vars = false;
+	itlist->change_resjunk = true;
 
 	/* Find the Vars and fill in the index array */
 	vinfo = itlist->vars;
@@ -1736,6 +1750,7 @@ build_tlist_index(List *tlist)
 			vinfo->varno = var->varno;
 			vinfo->varattno = var->varattno;
 			vinfo->resno = tle->resno;
+			vinfo->resjunk = tle->resjunk;
 			vinfo++;
 		}
 		else if (tle->expr && IsA(tle->expr, PlaceHolderVar))
@@ -1772,6 +1787,7 @@ build_tlist_index_other_vars(List *tlist, Index ignore_rel)
 	itlist->tlist = tlist;
 	itlist->has_ph_vars = false;
 	itlist->has_non_vars = false;
+	itlist->change_resjunk = true;
 
 	/* Find the desired Vars and fill in the index array */
 	vinfo = itlist->vars;
@@ -1788,6 +1804,7 @@ build_tlist_index_other_vars(List *tlist, Index ignore_rel)
 				vinfo->varno = var->varno;
 				vinfo->varattno = var->varattno;
 				vinfo->resno = tle->resno;
+				vinfo->resjunk = tle->resjunk;
 				vinfo++;
 			}
 		}
@@ -1827,7 +1844,11 @@ search_indexed_tlist_for_var(Var *var, indexed_tlist *itlist,
 			Var		   *newvar = copyVar(var);
 
 			newvar->varno = newvarno;
-			newvar->varattno = vinfo->resno;
+			if (itlist->change_resjunk || !vinfo->resjunk)
+				newvar->varattno = vinfo->resno;
+			else
+				Assert(newvar->varattno == InvalidAttrNumber);
+
 			if (newvar->varnoold > 0)
 				newvar->varnoold += rtoffset;
 			return newvar;
@@ -1917,10 +1938,13 @@ search_indexed_tlist_for_sortgroupref(Node *node,
  *
  * This is used in two different scenarios: a normal join clause, where all
  * the Vars in the clause *must* be replaced by OUTER_VAR or INNER_VAR
- * references; and a RETURNING clause, which may contain both Vars of the
- * target relation and Vars of other relations.  In the latter case we want
- * to replace the other-relation Vars by OUTER_VAR references, while leaving
- * target Vars alone.
+ * references; and a RETURNING/ON CONFLICT DO UPDATE SET/ON CONFLICT DO UPDATE
+ * WHERE clause, which may contain both Vars of the target relation and Vars of
+ * other relations.  In the latter case we want to replace the other-relation
+ * Vars by OUTER_VAR references (or INNER_VAR references for ON CONFLICT),
+ * while leaving target Vars alone.  Also, for ON CONFLICT, caller has us avoid
+ * incrementing resjunk TLE Vars' varattno -- this is for the benefit of
+ * excluded.* whole row Vars.
  *
  * For a normal join, acceptable_rel should be zero so that any failure to
  * match a Var will be reported as an error.  For the RETURNING case, pass
@@ -1978,7 +2002,7 @@ fix_join_expr_mutator(Node *node, fix_join_expr_context *context)
 				return (Node *) newvar;
 		}
 
-		/* Then in the outer */
+		/* Then in the inner */
 		if (context->inner_itlist)
 		{
 			newvar = search_indexed_tlist_for_var(var,
diff --git a/src/backend/optimizer/prep/preptlist.c b/src/backend/optimizer/prep/preptlist.c
index 6b0c689..e6a5933 100644
--- a/src/backend/optimizer/prep/preptlist.c
+++ b/src/backend/optimizer/prep/preptlist.c
@@ -38,6 +38,8 @@
 
 static List *expand_targetlist(List *tlist, int command_type,
 				  Index result_relation, List *range_table);
+static void pull_var_targetlist_clause(OnConflictExpr *onconfl, List* clause,
+				  int result_relation);
 
 
 /*
@@ -185,12 +187,28 @@ preprocess_targetlist(PlannerInfo *root, List *tlist)
  * preprocess_onconflict_targetlist
  *	  Process ON CONFLICT SET targetlist.
  *
- *	  Returns the new targetlist.
+ *	  Modifies passed ON CONFLICT expression in-place.
  */
-List *
-preprocess_onconflict_targetlist(List *tlist, int result_relation, List *range_table)
+void
+preprocess_onconflict_targetlist(OnConflictExpr *onconfl, int result_relation,
+								 List *range_table)
 {
-	return expand_targetlist(tlist, CMD_UPDATE, result_relation, range_table);
+	List	   *tlist;
+
+	tlist = expand_targetlist(onconfl->onConflictSet, CMD_UPDATE,
+							  result_relation, range_table);
+
+	/*
+	 * Perform similar pull up process to that perfomred by
+	 * preprocess_targetlist() for multi-relation RETURNING lists.  This is
+	 * only necessary for the benefit of excluded.* whole row Vars, and to
+	 * enforce that excluded.* system column references are disallowed.
+	 */
+	pull_var_targetlist_clause(onconfl, tlist, result_relation);
+	pull_var_targetlist_clause(onconfl, (List *) onconfl->onConflictWhere,
+							   result_relation);
+
+	onconfl->onConflictSet = tlist;
 }
 
 
@@ -376,6 +394,54 @@ expand_targetlist(List *tlist, int command_type,
 	return new_tlist;
 }
 
+/*
+ * pull_var_targetlist_clause
+ *	  Given an ON CONFLICT clause as generated by the parser and a result
+ *	  relation, add resjunk entries for any Vars used that are not
+ *	  associated with the target and are not excluded.* non-resjunk
+ *	  TLE Vars.
+ */
+static void
+pull_var_targetlist_clause(OnConflictExpr *onconfl, List* clause,
+						   int result_relation)
+{
+	List	   *vars;
+	ListCell   *l;
+
+	vars = pull_var_clause((Node *) clause,
+						   PVC_RECURSE_AGGREGATES,
+						   PVC_INCLUDE_PLACEHOLDERS);
+	foreach(l, vars)
+	{
+		Var		   *var = (Var *) lfirst(l);
+		TargetEntry *tle;
+
+		if (IsA(var, Var) && var->varno == result_relation)
+			continue;		/* don't need it */
+
+		if (IsA(var, Var) && var->varno == onconfl->exclRelIndex)
+		{
+			if (var->varattno < 0)
+				ereport(ERROR,
+						(errcode(ERRCODE_INVALID_COLUMN_REFERENCE),
+						 errmsg("system columns from excluded table cannot be referenced")));
+			else if (var->varattno > 0)		/* don't need it */
+				continue;
+		}
+
+		if (tlist_member((Node *) var, onconfl->exclRelTlist))
+			continue;		/* already got it */
+
+		tle = makeTargetEntry((Expr *) var,
+						  list_length(clause) + 1,
+							  NULL,
+							  true);
+
+		onconfl->exclRelTlist = lappend(onconfl->exclRelTlist, tle);
+	}
+
+	list_free(vars);
+}
 
 /*
  * Locate PlanRowMark for given RT index, or return NULL if none
diff --git a/src/include/optimizer/prep.h b/src/include/optimizer/prep.h
index 7b8c0a9..8d61c7c 100644
--- a/src/include/optimizer/prep.h
+++ b/src/include/optimizer/prep.h
@@ -45,8 +45,8 @@ extern void expand_security_quals(PlannerInfo *root, List *tlist);
  */
 extern List *preprocess_targetlist(PlannerInfo *root, List *tlist);
 
-extern List *preprocess_onconflict_targetlist(List *tlist,
-								 int result_relation, List *range_table);
+extern void preprocess_onconflict_targetlist(OnConflictExpr *onconfl,
+						int result_relation, List *range_table);
 
 extern PlanRowMark *get_plan_rowmark(List *rowmarks, Index rtindex);
 
diff --git a/src/test/regress/expected/insert_conflict.out b/src/test/regress/expected/insert_conflict.out
index eca9690..442d9fc 100644
--- a/src/test/regress/expected/insert_conflict.out
+++ b/src/test/regress/expected/insert_conflict.out
@@ -363,6 +363,45 @@ ERROR:  there is no unique or exclusion constraint matching the ON CONFLICT spec
 insert into insertconflicttest values (23, 'Blackberry') on conflict (fruit) where fruit like '%berry' do update set fruit = excluded.fruit;
 ERROR:  there is no unique or exclusion constraint matching the ON CONFLICT specification
 drop index partial_key_index;
+--
+-- Test resjunk in excluded.* pseudo-relation
+--
+create unique index plain on insertconflicttest(key);
+-- Succeeds, updates existing row:
+insert into insertconflicttest as i values (23, 'Jackfruit') on conflict (key) do update set fruit = excluded.fruit
+  where i.* != excluded.* returning *;
+ key |   fruit   
+-----+-----------
+  23 | Jackfruit
+(1 row)
+
+-- No update this time, though:
+insert into insertconflicttest as i values (23, 'Jackfruit') on conflict (key) do update set fruit = excluded.fruit
+  where i.* != excluded.* returning *;
+ key | fruit 
+-----+-------
+(0 rows)
+
+-- Predicate changed to require match rather than non-match, so updates once more:
+insert into insertconflicttest as i values (23, 'Jackfruit') on conflict (key) do update set fruit = excluded.fruit
+  where i.* = excluded.* returning *;
+ key |   fruit   
+-----+-----------
+  23 | Jackfruit
+(1 row)
+
+-- Assign:
+insert into insertconflicttest as i values (23, 'Avocado') on conflict (key) do update set fruit = excluded.*::text
+  returning *;
+ key |    fruit     
+-----+--------------
+  23 | (23,Avocado)
+(1 row)
+
+-- Forbidden, fails:
+insert into insertconflicttest values (23, 'Blackberry') on conflict (key) do update set fruit = excluded.ctid;
+ERROR:  system columns from excluded table cannot be referenced
+drop index plain;
 -- Cleanup
 drop table insertconflicttest;
 -- ******************************************************************
diff --git a/src/test/regress/sql/insert_conflict.sql b/src/test/regress/sql/insert_conflict.sql
index a0bdd7f..a2b6aba 100644
--- a/src/test/regress/sql/insert_conflict.sql
+++ b/src/test/regress/sql/insert_conflict.sql
@@ -212,6 +212,28 @@ insert into insertconflicttest values (23, 'Blackberry') on conflict (fruit) whe
 
 drop index partial_key_index;
 
+--
+-- Test resjunk in excluded.* pseudo-relation
+--
+create unique index plain on insertconflicttest(key);
+
+-- Succeeds, updates existing row:
+insert into insertconflicttest as i values (23, 'Jackfruit') on conflict (key) do update set fruit = excluded.fruit
+  where i.* != excluded.* returning *;
+-- No update this time, though:
+insert into insertconflicttest as i values (23, 'Jackfruit') on conflict (key) do update set fruit = excluded.fruit
+  where i.* != excluded.* returning *;
+-- Predicate changed to require match rather than non-match, so updates once more:
+insert into insertconflicttest as i values (23, 'Jackfruit') on conflict (key) do update set fruit = excluded.fruit
+  where i.* = excluded.* returning *;
+-- Assign:
+insert into insertconflicttest as i values (23, 'Avocado') on conflict (key) do update set fruit = excluded.*::text
+  returning *;
+-- Forbidden, fails:
+insert into insertconflicttest values (23, 'Blackberry') on conflict (key) do update set fruit = excluded.ctid;
+
+drop index plain;
+
 -- Cleanup
 drop table insertconflicttest;
 
-- 
1.9.1

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to