Check the relevant index element in ON CONFLICT unique index inference.

ON CONFLICT unique index inference had a thinko that could affect cases
where the user-supplied inference clause required that an attribute
match a particular (user specified) collation and/or opclass.

infer_collation_opclass_match() has to check for opclass and/or
collation matches and that the attribute is in the list of attributes or
expressions known to be in the definition of the index under
consideration. The bug was that these two conditions weren't necessarily
evaluated for the same index attribute.

Author: Peter Geoghegan
Discussion: CAM3SWZR4uug=WvmGk7UgsqHn2MkEzy9YU-+8jKGO4JPhesyeWg@mail.gmail.com
Backpatch: 9.5, where ON CONFLICT was introduced

diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.c
index 662b97755ab281938efde861bb6900184ca44cd7..9442e5fa32b31a236901f376d86fa3bf34d5232d 100644 (file)
--- a/src/backend/optimizer/util/plancat.c
+++ b/src/backend/optimizer/util/plancat.c
@@ -52,7 +52,7 @@ get_relation_info_hook_type get_relation_info_hook = NULL;
 
 
 static bool infer_collation_opclass_match(InferenceElem *elem, Relation idxRel,
-                                                         Bitmapset *inferAttrs, List *idxExprs);
+                                                         List *idxExprs);
 static int32 get_rel_data_width(Relation rel, int32 *attr_widths);
 static List *get_relation_constraints(PlannerInfo *root,
                                                 Oid relationObjectId, RelOptInfo *rel,
@@ -616,8 +616,7 @@ infer_arbiter_indexes(PlannerInfo *root)
                         * this for both expressions and ordinary (non-expression)
                         * attributes appearing as inference elements.
                         */
-                       if (!infer_collation_opclass_match(elem, idxRel, inferAttrs,
-                                                                                          idxExprs))
+                       if (!infer_collation_opclass_match(elem, idxRel, idxExprs))
                                goto next;
 
                        /*
@@ -682,11 +681,10 @@ next:
  * infer_collation_opclass_match - ensure infer element opclass/collation match
  *
  * Given unique index inference element from inference specification, if
- * collation was specified, or if opclass (represented here as opfamily +
- * opcintype) was specified, verify that there is at least one matching
- * indexed attribute (occasionally, there may be more).  Skip this in the
- * common case where inference specification does not include collation or
- * opclass (instead matching everything, regardless of cataloged
+ * collation was specified, or if opclass was specified, verify that there is
+ * at least one matching indexed attribute (occasionally, there may be more).
+ * Skip this in the common case where inference specification does not include
+ * collation or opclass (instead matching everything, regardless of cataloged
  * collation/opclass of indexed attribute).
  *
  * At least historically, Postgres has not offered collations or opclasses
@@ -708,11 +706,12 @@ next:
  */
 static bool
 infer_collation_opclass_match(InferenceElem *elem, Relation idxRel,
-                                                         Bitmapset *inferAttrs, List *idxExprs)
+                                                         List *idxExprs)
 {
        AttrNumber      natt;
-       Oid                     inferopfamily = InvalidOid;             /* OID of att opfamily */
-       Oid                     inferopcinputtype = InvalidOid; /* OID of att opfamily */
+       Oid                     inferopfamily = InvalidOid;             /* OID of opclass opfamily */
+       Oid                     inferopcinputtype = InvalidOid; /* OID of opclass input type */
+       int                     nplain = 0;                                             /* # plain attrs observed */
 
        /*
         * If inference specification element lacks collation/opclass, then no
@@ -735,6 +734,10 @@ infer_collation_opclass_match(InferenceElem *elem, Relation idxRel,
                Oid                     opfamily = idxRel->rd_opfamily[natt - 1];
                Oid                     opcinputtype = idxRel->rd_opcintype[natt - 1];
                Oid                     collation = idxRel->rd_indcollation[natt - 1];
+               int                     attno = idxRel->rd_index->indkey.values[natt - 1];
+
+               if (attno != 0)
+                       nplain++;
 
                if (elem->inferopclass != InvalidOid &&
                        (inferopfamily != opfamily || inferopcinputtype != opcinputtype))
@@ -750,12 +753,23 @@ infer_collation_opclass_match(InferenceElem *elem, Relation idxRel,
                        continue;
                }
 
-               if ((IsA(elem->expr, Var) &&
-                        bms_is_member(((Var *) elem->expr)->varattno, inferAttrs)) ||
-                       list_member(idxExprs, elem->expr))
+               /* If one matching index att found, good enough -- return true */
+               if (IsA(elem->expr, Var))
                {
-                       /* Found one match - good enough */
-                       return true;
+                       if (((Var *) elem->expr)->varattno == attno)
+                               return true;
+               }
+               else if (attno == 0)
+               {
+                       Node       *nattExpr = list_nth(idxExprs, (natt - 1) - nplain);
+
+                       /*
+                        * Note that unlike routines like match_index_to_operand() we
+                        * don't need to care about RelabelType.  Neither the index
+                        * definition nor the inference clause should contain them.
+                        */
+                       if (equal(elem->expr, nattExpr))
+                               return true;
                }
        }
 

diff --git a/src/test/regress/expected/insert_conflict.out b/src/test/regress/expected/insert_conflict.out
index 325e88b572f0e67899b430af3391e75bcd586f14..09b67db43c186ead376ee49c46007d251bc66129 100644 (file)
--- a/src/test/regress/expected/insert_conflict.out
+++ b/src/test/regress/expected/insert_conflict.out
@@ -141,6 +141,24 @@ drop index collation_index_key;
 drop index both_index_key;
 drop index both_index_expr_key;
 --
+-- Make sure that cross matching of attribute opclass/collation does not occur
+--
+create unique index cross_match on insertconflicttest(lower(fruit) collate "C", upper(fruit) text_pattern_ops);
+-- fails:
+explain (costs off) insert into insertconflicttest values(0, 'Crowberry') on conflict (lower(fruit) text_pattern_ops, upper(fruit) collate "C") do nothing;
+ERROR:  there is no unique or exclusion constraint matching the ON CONFLICT specification
+-- works:
+explain (costs off) insert into insertconflicttest values(0, 'Crowberry') on conflict (lower(fruit) collate "C", upper(fruit) text_pattern_ops) do nothing;
+               QUERY PLAN                
+-----------------------------------------
+ Insert on insertconflicttest
+   Conflict Resolution: NOTHING
+   Conflict Arbiter Indexes: cross_match
+   ->  Result
+(4 rows)
+
+drop index cross_match;
+--
 -- Single key tests
 --
 create unique index key_index on insertconflicttest(key);

diff --git a/src/test/regress/sql/insert_conflict.sql b/src/test/regress/sql/insert_conflict.sql
index 7dd503221283995f676e5d751edaa130a160f2ac..e981e67fd27b922a572b04ed238e279918048799 100644 (file)
--- a/src/test/regress/sql/insert_conflict.sql
+++ b/src/test/regress/sql/insert_conflict.sql
@@ -57,6 +57,18 @@ drop index collation_index_key;
 drop index both_index_key;
 drop index both_index_expr_key;
 
+--
+-- Make sure that cross matching of attribute opclass/collation does not occur
+--
+create unique index cross_match on insertconflicttest(lower(fruit) collate "C", upper(fruit) text_pattern_ops);
+
+-- fails:
+explain (costs off) insert into insertconflicttest values(0, 'Crowberry') on conflict (lower(fruit) text_pattern_ops, upper(fruit) collate "C") do nothing;
+-- works:
+explain (costs off) insert into insertconflicttest values(0, 'Crowberry') on conflict (lower(fruit) collate "C", upper(fruit) text_pattern_ops) do nothing;
+
+drop index cross_match;
+
 --
 -- Single key tests
 --