Fix assorted missing infrastructure for ON CONFLICT.

subquery_planner() failed to apply expression preprocessing to the
arbiterElems and arbiterWhere fields of an OnConflictExpr.  No doubt the
theory was that this wasn't necessary because we don't actually try to
execute those expressions; but that's wrong, because it results in failure
to match to index expressions or index predicates that are changed at all
by preprocessing.  Per bug #14132 from Reynold Smith.

Also add pullup_replace_vars processing for onConflictWhere.  Perhaps
it's impossible to have a subquery reference there, but I'm not exactly
convinced; and even if true today it's a failure waiting to happen.

Also add some comments to other places where one or another field of
OnConflictExpr is intentionally ignored, with explanation as to why it's
okay to do so.

Also, catalog/dependency.c failed to record any dependency on the named
constraint in ON CONFLICT ON CONSTRAINT, allowing such a constraint to
be dropped while rules exist that depend on it, and allowing pg_dump to
dump such a rule before the constraint it refers to.  The normal execution
path managed to error out reasonably for a dangling constraint reference,
but ruleutils.c dumped core; so in addition to fixing the omission, add
a protective check in ruleutils.c, since we can't retroactively add a
dependency in existing databases.

Back-patch to 9.5 where this code was introduced.

Report: <20160510190350.2608.48667@wrigleys.postgresql.org>

diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c
index a6180a64f2156658195701408f9b8fbbfc4c832f..04d78402903074f68465448d58c636252e68b852 100644 (file)
--- a/src/backend/catalog/dependency.c
+++ b/src/backend/catalog/dependency.c
@@ -1789,6 +1789,15 @@ find_expr_references_walker(Node *node,
        add_object_address(OCLASS_TYPE, cd->resulttype, 0,
                           context->addrs);
    }
+   else if (IsA(node, OnConflictExpr))
+   {
+       OnConflictExpr *onconflict = (OnConflictExpr *) node;
+
+       if (OidIsValid(onconflict->constraint))
+           add_object_address(OCLASS_CONSTRAINT, onconflict->constraint, 0,
+                              context->addrs);
+       /* fall through to examine arguments */
+   }
    else if (IsA(node, SortGroupClause))
    {
        SortGroupClause *sgc = (SortGroupClause *) node;

diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index 6770836dc894e81038e34fb6562a60518a1cc839..75d93c08db2f8b77a42b77094570b0dad066ef38 100644 (file)
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -77,6 +77,7 @@ create_upper_paths_hook_type create_upper_paths_hook = NULL;
 #define EXPRKIND_APPINFO       7
 #define EXPRKIND_PHV           8
 #define EXPRKIND_TABLESAMPLE   9
+#define EXPRKIND_ARBITER_ELEM  10
 
 /* Passthrough data for standard_qp_callback */
 typedef struct
@@ -620,13 +621,23 @@ subquery_planner(PlannerGlobal *glob, Query *parse,
 
    if (parse->onConflict)
    {
+       parse->onConflict->arbiterElems = (List *)
+           preprocess_expression(root,
+                                 (Node *) parse->onConflict->arbiterElems,
+                                 EXPRKIND_ARBITER_ELEM);
+       parse->onConflict->arbiterWhere =
+           preprocess_expression(root,
+                                 parse->onConflict->arbiterWhere,
+                                 EXPRKIND_QUAL);
        parse->onConflict->onConflictSet = (List *)
-           preprocess_expression(root, (Node *) parse->onConflict->onConflictSet,
+           preprocess_expression(root,
+                                 (Node *) parse->onConflict->onConflictSet,
                                  EXPRKIND_TARGET);
-
        parse->onConflict->onConflictWhere =
-           preprocess_expression(root, (Node *) parse->onConflict->onConflictWhere,
+           preprocess_expression(root,
+                                 parse->onConflict->onConflictWhere,
                                  EXPRKIND_QUAL);
+       /* exclRelTlist contains only Vars, so no preprocessing needed */
    }
 
    root->append_rel_list = (List *)

diff --git a/src/backend/optimizer/plan/subselect.c b/src/backend/optimizer/plan/subselect.c
index 1ff430229dd621e7df25ff143681c46577c916e2..0849b1d56346cd3fec5a8143e36fb234842ea2a6 100644 (file)
--- a/src/backend/optimizer/plan/subselect.c
+++ b/src/backend/optimizer/plan/subselect.c
@@ -2507,6 +2507,7 @@ finalize_plan(PlannerInfo *root, Plan *plan, Bitmapset *valid_params,
                                  &context);
                finalize_primnode((Node *) mtplan->onConflictWhere,
                                  &context);
+               /* exclRelTlist contains only Vars, doesn't need examination */
                foreach(l, mtplan->plans)
                {
                    context.paramids =

diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optimizer/prep/prepjointree.c
index 75577bce2ca02e56cb010d2161c85c8711967e0f..a334f15773ad3fbed7cc96e145729bc2ba0c57c2 100644 (file)
--- a/src/backend/optimizer/prep/prepjointree.c
+++ b/src/backend/optimizer/prep/prepjointree.c
@@ -1039,8 +1039,19 @@ pull_up_simple_subquery(PlannerInfo *root, Node *jtnode, RangeTblEntry *rte,
    parse->returningList = (List *)
        pullup_replace_vars((Node *) parse->returningList, &rvcontext);
    if (parse->onConflict)
+   {
        parse->onConflict->onConflictSet = (List *)
-           pullup_replace_vars((Node *) parse->onConflict->onConflictSet, &rvcontext);
+           pullup_replace_vars((Node *) parse->onConflict->onConflictSet,
+                               &rvcontext);
+       parse->onConflict->onConflictWhere =
+           pullup_replace_vars(parse->onConflict->onConflictWhere,
+                               &rvcontext);
+
+       /*
+        * We assume ON CONFLICT's arbiterElems, arbiterWhere, exclRelTlist
+        * can't contain any references to a subquery
+        */
+   }
    replace_vars_in_jointree((Node *) parse->jointree, &rvcontext,
                             lowest_nulling_outer_join);
    Assert(parse->setOperations == NULL);
@@ -1633,8 +1644,19 @@ pull_up_simple_values(PlannerInfo *root, Node *jtnode, RangeTblEntry *rte)
    parse->returningList = (List *)
        pullup_replace_vars((Node *) parse->returningList, &rvcontext);
    if (parse->onConflict)
+   {
        parse->onConflict->onConflictSet = (List *)
-           pullup_replace_vars((Node *) parse->onConflict->onConflictSet, &rvcontext);
+           pullup_replace_vars((Node *) parse->onConflict->onConflictSet,
+                               &rvcontext);
+       parse->onConflict->onConflictWhere =
+           pullup_replace_vars(parse->onConflict->onConflictWhere,
+                               &rvcontext);
+
+       /*
+        * We assume ON CONFLICT's arbiterElems, arbiterWhere, exclRelTlist
+        * can't contain any references to a subquery
+        */
+   }
    replace_vars_in_jointree((Node *) parse->jointree, &rvcontext, NULL);
    Assert(parse->setOperations == NULL);
    parse->havingQual = pullup_replace_vars(parse->havingQual, &rvcontext);

diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.c
index 9c11b09cbcc57be8100b4beb3876032659aa52c2..4f399a5b85471f39348a4e50b5bb9f0db1de4cab 100644 (file)
--- a/src/backend/optimizer/util/plancat.c
+++ b/src/backend/optimizer/util/plancat.c
@@ -623,7 +623,6 @@ infer_arbiter_indexes(PlannerInfo *root)
        Bitmapset  *indexedAttrs = NULL;
        List       *idxExprs;
        List       *predExprs;
-       List       *whereExplicit;
        AttrNumber  natt;
        ListCell   *el;
 
@@ -685,6 +684,7 @@ infer_arbiter_indexes(PlannerInfo *root)
        {
            int         attno = idxRel->rd_index->indkey.values[natt];
 
+           /* XXX broken */
            if (attno < 0)
                elog(ERROR, "system column in index");
 
@@ -745,13 +745,12 @@ infer_arbiter_indexes(PlannerInfo *root)
            goto next;
 
        /*
-        * Any user-supplied ON CONFLICT unique index inference WHERE clause
-        * need only be implied by the cataloged index definitions predicate.
+        * If it's a partial index, its predicate must be implied by the ON
+        * CONFLICT's WHERE clause.
         */
        predExprs = RelationGetIndexPredicate(idxRel);
-       whereExplicit = make_ands_implicit((Expr *) onconflict->arbiterWhere);
 
-       if (!predicate_implied_by(predExprs, whereExplicit))
+       if (!predicate_implied_by(predExprs, (List *) onconflict->arbiterWhere))
            goto next;
 
        results = lappend_oid(results, idxForm->indexrelid);

diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 6dfa6b9a3190fb5e0ba5a579fd2418854ec662a7..8cb3075e7856b45c6c6a98dd4d66bf5b839ed99a 100644 (file)
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -5570,12 +5570,15 @@ get_insert_query_def(Query *query, deparse_context *context)
                context->varprefix = save_varprefix;
            }
        }
-       else if (confl->constraint != InvalidOid)
+       else if (OidIsValid(confl->constraint))
        {
            char       *constraint = get_constraint_name(confl->constraint);
 
+           if (!constraint)
+               elog(ERROR, "cache lookup failed for constraint %u",
+                    confl->constraint);
            appendStringInfo(buf, " ON CONSTRAINT %s",
-                            quote_qualified_identifier(NULL, constraint));
+                            quote_identifier(constraint));
        }
 
        if (confl->action == ONCONFLICT_NOTHING)

diff --git a/src/test/regress/expected/insert_conflict.out b/src/test/regress/expected/insert_conflict.out
index f56cbd642dec667b34b179e6cef5c161a8e2371d..e5c8b4aaacb1ddc59b5cb6bb97e6129e3556e5b2 100644 (file)
--- a/src/test/regress/expected/insert_conflict.out
+++ b/src/test/regress/expected/insert_conflict.out
@@ -454,6 +454,21 @@ ERROR:  column excluded.oid does not exist
 LINE 1: ...values (1) on conflict (key) do update set data = excluded.o...
                                                              ^
 drop table syscolconflicttest;
+--
+-- Previous tests all managed to not test any expressions requiring
+-- planner preprocessing ...
+--
+create table insertconflict (a bigint, b bigint);
+create unique index insertconflicti1 on insertconflict(coalesce(a, 0));
+create unique index insertconflicti2 on insertconflict(b)
+  where coalesce(a, 1) > 0;
+insert into insertconflict values (1, 2)
+on conflict (coalesce(a, 0)) do nothing;
+insert into insertconflict values (1, 2)
+on conflict (b) where coalesce(a, 1) > 0 do nothing;
+insert into insertconflict values (1, 2)
+on conflict (b) where coalesce(a, 1) > 1 do nothing;
+drop table insertconflict;
 -- ******************************************************************
 -- *                                                                *
 -- * Test inheritance (example taken from tutorial)                 *

diff --git a/src/test/regress/sql/insert_conflict.sql b/src/test/regress/sql/insert_conflict.sql
index 8d846f51df4ae2873c832f1a33f1bd2873fc4582..94b1b0aadeebb2d37f961403c8e4e1a6f0a0449e 100644 (file)
--- a/src/test/regress/sql/insert_conflict.sql
+++ b/src/test/regress/sql/insert_conflict.sql
@@ -261,6 +261,28 @@ insert into syscolconflicttest values (1) on conflict (key) do update set data =
 insert into syscolconflicttest values (1) on conflict (key) do update set data = excluded.oid::text;
 drop table syscolconflicttest;
 
+--
+-- Previous tests all managed to not test any expressions requiring
+-- planner preprocessing ...
+--
+create table insertconflict (a bigint, b bigint);
+
+create unique index insertconflicti1 on insertconflict(coalesce(a, 0));
+
+create unique index insertconflicti2 on insertconflict(b)
+  where coalesce(a, 1) > 0;
+
+insert into insertconflict values (1, 2)
+on conflict (coalesce(a, 0)) do nothing;
+
+insert into insertconflict values (1, 2)
+on conflict (b) where coalesce(a, 1) > 0 do nothing;
+
+insert into insertconflict values (1, 2)
+on conflict (b) where coalesce(a, 1) > 1 do nothing;
+
+drop table insertconflict;
+
 
 -- ******************************************************************
 -- *                                                                *