Allow table-qualified variable names in ON CONFLICT ... WHERE.

Previously you could only use unqualified variable names here.
While that's not a functional deficiency, since only the target
table can be referenced, it's a surprising inconsistency with the
rules for partial-index predicates, on which this syntax is
supposedly modeled.

The fix for that is no harder than passing addToRelNameSpace = true
to addNSItemToQuery.  However, it's really pretty bogus for
transformOnConflictArbiter and transformOnConflictClause to be
messing with the namespace item for the target table at all.
It's not theirs to manage, it results in duplicative creations of
namespace items, and transformOnConflictClause wasn't even doing
it quite correctly (that coding resulted in two nsitems for the
target table, since it hadn't cleaned out the existing one).
Hence, make transformInsertStmt responsible for setting up the
target nsitem once for both these clauses and RETURNING.

Also, arrange for ON CONFLICT ... UPDATE's "excluded" pseudo-relation
to be added to the rangetable before we run transformOnConflictArbiter.
This produces a more helpful HINT if someone writes "excluded.col"
in the arbiter expression.

Per bug #16958 from Lukas Eder.  Although I agree this is a bug,
the consequences are hardly severe, so no back-patch.

Discussion: https://postgr.es/m/16958-963f638020de271c@postgresql.org

4 files changed, 52 insertions, 48 deletions

diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index 9f13880d19a..862f18a92f2 100644
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -869,25 +869,27 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt)
 										   attr_num - FirstLowInvalidHeapAttributeNumber);
 	}
 
-	/* Process ON CONFLICT, if any. */
-	if (stmt->onConflictClause)
-		qry->onConflict = transformOnConflictClause(pstate,
-													stmt->onConflictClause);
-
 	/*
-	 * If we have a RETURNING clause, we need to add the target relation to
-	 * the query namespace before processing it, so that Var references in
-	 * RETURNING will work.  Also, remove any namespace entries added in a
+	 * If we have any clauses yet to process, set the query namespace to
+	 * contain only the target relation, removing any entries added in a
 	 * sub-SELECT or VALUES list.
 	 */
-	if (stmt->returningList)
+	if (stmt->onConflictClause || stmt->returningList)
 	{
 		pstate->p_namespace = NIL;
 		addNSItemToQuery(pstate, pstate->p_target_nsitem,
 						 false, true, true);
+	}
+
+	/* Process ON CONFLICT, if any. */
+	if (stmt->onConflictClause)
+		qry->onConflict = transformOnConflictClause(pstate,
+													stmt->onConflictClause);
+
+	/* Process RETURNING, if any. */
+	if (stmt->returningList)
 		qry->returningList = transformReturningList(pstate,
 													stmt->returningList);
-	}
 
 	/* done building the range table and jointree */
 	qry->rtable = pstate->p_rtable;
@@ -1014,6 +1016,7 @@ static OnConflictExpr *
 transformOnConflictClause(ParseState *pstate,
 						  OnConflictClause *onConflictClause)
 {
+	ParseNamespaceItem *exclNSItem = NULL;
 	List	   *arbiterElems;
 	Node	   *arbiterWhere;
 	Oid			arbiterConstraint;
@@ -1023,29 +1026,17 @@ transformOnConflictClause(ParseState *pstate,
 	List	   *exclRelTlist = NIL;
 	OnConflictExpr *result;
 
-	/* Process the arbiter clause, ON CONFLICT ON (...) */
-	transformOnConflictArbiter(pstate, onConflictClause, &arbiterElems,
-							   &arbiterWhere, &arbiterConstraint);
-
-	/* Process DO UPDATE */
+	/*
+	 * If this is ON CONFLICT ... UPDATE, first create the range table entry
+	 * for the EXCLUDED pseudo relation, so that that will be present while
+	 * processing arbiter expressions.  (You can't actually reference it from
+	 * there, but this provides a useful error message if you try.)
+	 */
 	if (onConflictClause->action == ONCONFLICT_UPDATE)
 	{
 		Relation	targetrel = pstate->p_target_relation;
-		ParseNamespaceItem *exclNSItem;
 		RangeTblEntry *exclRte;
 
-		/*
-		 * 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;
-
-		/*
-		 * Add range table entry for the EXCLUDED pseudo relation.  relkind is
-		 * set to composite to signal that we're not dealing with an actual
-		 * relation, and no permission checks are required on it.  (We'll
-		 * check the actual target relation, instead.)
-		 */
 		exclNSItem = addRangeTableEntryForRelation(pstate,
 												   targetrel,
 												   RowExclusiveLock,
@@ -1054,6 +1045,11 @@ transformOnConflictClause(ParseState *pstate,
 		exclRte = exclNSItem->p_rte;
 		exclRelIndex = exclNSItem->p_rtindex;
 
+		/*
+		 * relkind is set to composite to signal that we're not dealing with
+		 * an actual relation, and no permission checks are required on it.
+		 * (We'll check the actual target relation, instead.)
+		 */
 		exclRte->relkind = RELKIND_COMPOSITE_TYPE;
 		exclRte->requiredPerms = 0;
 		/* other permissions fields in exclRte are already empty */
@@ -1061,14 +1057,27 @@ transformOnConflictClause(ParseState *pstate,
 		/* Create EXCLUDED rel's targetlist for use by EXPLAIN */
 		exclRelTlist = BuildOnConflictExcludedTargetlist(targetrel,
 														 exclRelIndex);
+	}
+
+	/* Process the arbiter clause, ON CONFLICT ON (...) */
+	transformOnConflictArbiter(pstate, onConflictClause, &arbiterElems,
+							   &arbiterWhere, &arbiterConstraint);
+
+	/* Process DO UPDATE */
+	if (onConflictClause->action == ONCONFLICT_UPDATE)
+	{
+		/*
+		 * Expressions in the UPDATE targetlist need to be handled like UPDATE
+		 * not INSERT.  We don't need to save/restore this because all INSERT
+		 * expressions have been parsed already.
+		 */
+		pstate->p_is_insert = false;
 
 		/*
-		 * Add EXCLUDED and the target RTE to the namespace, so that they can
-		 * be used in the UPDATE subexpressions.
+		 * Add the EXCLUDED pseudo relation to the query namespace, making it
+		 * available in the UPDATE subexpressions.
 		 */
 		addNSItemToQuery(pstate, exclNSItem, false, true, true);
-		addNSItemToQuery(pstate, pstate->p_target_nsitem,
-						 false, true, true);
 
 		/*
 		 * Now transform the UPDATE subexpressions.
@@ -1079,6 +1088,14 @@ transformOnConflictClause(ParseState *pstate,
 		onConflictWhere = transformWhereClause(pstate,
 											   onConflictClause->whereClause,
 											   EXPR_KIND_WHERE, "WHERE");
+
+		/*
+		 * Remove the EXCLUDED pseudo relation from the query namespace, since
+		 * it's not supposed to be available in RETURNING.  (Maybe someday we
+		 * could allow that, and drop this step.)
+		 */
+		Assert((ParseNamespaceItem *) llast(pstate->p_namespace) == exclNSItem);
+		pstate->p_namespace = list_delete_last(pstate->p_namespace);
 	}
 
 	/* Finally, build ON CONFLICT DO [NOTHING | UPDATE] expression */
diff --git a/src/backend/parser/parse_clause.c b/src/backend/parser/parse_clause.c
index af80aa45936..89d95d3e949 100644
--- a/src/backend/parser/parse_clause.c
+++ b/src/backend/parser/parse_clause.c
@@ -3222,17 +3222,6 @@ transformOnConflictArbiter(ParseState *pstate,
 	/* ON CONFLICT DO NOTHING does not require an inference clause */
 	if (infer)
 	{
-		List	   *save_namespace;
-
-		/*
-		 * While we process the arbiter expressions, accept only non-qualified
-		 * references to the target table. Hide any other relations.
-		 */
-		save_namespace = pstate->p_namespace;
-		pstate->p_namespace = NIL;
-		addNSItemToQuery(pstate, pstate->p_target_nsitem,
-						 false, false, true);
-
 		if (infer->indexElems)
 			*arbiterExpr = resolve_unique_index_expr(pstate, infer,
 													 pstate->p_target_relation);
@@ -3245,8 +3234,6 @@ transformOnConflictArbiter(ParseState *pstate,
 			*arbiterWhere = transformExpr(pstate, infer->whereClause,
 										  EXPR_KIND_INDEX_PREDICATE);
 
-		pstate->p_namespace = save_namespace;
-
 		/*
 		 * If the arbiter is specified by constraint name, get the constraint
 		 * OID and mark the constrained columns as requiring SELECT privilege,
diff --git a/src/test/regress/expected/insert_conflict.out b/src/test/regress/expected/insert_conflict.out
index a54a51e5c72..66d8633e3ec 100644
--- a/src/test/regress/expected/insert_conflict.out
+++ b/src/test/regress/expected/insert_conflict.out
@@ -248,7 +248,7 @@ insert into insertconflicttest values (1, 'Apple') on conflict (keyy) do update
 ERROR:  column "keyy" does not exist
 LINE 1: ...nsertconflicttest values (1, 'Apple') on conflict (keyy) do ...
                                                              ^
-HINT:  Perhaps you meant to reference the column "insertconflicttest.key".
+HINT:  Perhaps you meant to reference the column "insertconflicttest.key" or the column "excluded.key".
 -- Have useful HINT for EXCLUDED.* RTE within UPDATE:
 insert into insertconflicttest values (1, 'Apple') on conflict (key) do update set fruit = excluded.fruitt;
 ERROR:  column excluded.fruitt does not exist
@@ -373,7 +373,7 @@ drop index fruit_index;
 create unique index partial_key_index on insertconflicttest(key) where fruit like '%berry';
 -- Succeeds
 insert into insertconflicttest values (23, 'Blackberry') on conflict (key) where fruit like '%berry' do update set fruit = excluded.fruit;
-insert into insertconflicttest values (23, 'Blackberry') on conflict (key) where fruit like '%berry' and fruit = 'inconsequential' do nothing;
+insert into insertconflicttest as t values (23, 'Blackberry') on conflict (key) where fruit like '%berry' and t.fruit = 'inconsequential' do nothing;
 -- fails
 insert into insertconflicttest values (23, 'Blackberry') on conflict (key) do update set fruit = excluded.fruit;
 ERROR:  there is no unique or exclusion constraint matching the ON CONFLICT specification
diff --git a/src/test/regress/sql/insert_conflict.sql b/src/test/regress/sql/insert_conflict.sql
index 43691cd3357..23d5778b821 100644
--- a/src/test/regress/sql/insert_conflict.sql
+++ b/src/test/regress/sql/insert_conflict.sql
@@ -214,7 +214,7 @@ create unique index partial_key_index on insertconflicttest(key) where fruit lik
 
 -- Succeeds
 insert into insertconflicttest values (23, 'Blackberry') on conflict (key) where fruit like '%berry' do update set fruit = excluded.fruit;
-insert into insertconflicttest values (23, 'Blackberry') on conflict (key) where fruit like '%berry' and fruit = 'inconsequential' do nothing;
+insert into insertconflicttest as t values (23, 'Blackberry') on conflict (key) where fruit like '%berry' and t.fruit = 'inconsequential' do nothing;
 
 -- fails
 insert into insertconflicttest values (23, 'Blackberry') on conflict (key) do update set fruit = excluded.fruit;