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

diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index 9f13880d19a32dc9909f25cc6ab259330616b2da..862f18a92f27038588fd0a06cf2559d924a40056 100644 (file)
--- 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 af80aa4593689987c6a7bfde841096c2956b8167..89d95d3e949b12b2034330ea80bc43b510520300 100644 (file)
--- 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 a54a51e5c72d5cc03d0ab4840dd07668ef8b4f61..66d8633e3ec569635b7179961597a4658a3ca0b2 100644 (file)
--- 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 43691cd3357985b758ecdb5e2e847a7262e00d58..23d5778b821e03acc55fedecc812ab679f7ce773 100644 (file)
--- 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;