Commit 159cff58 authored by Andres Freund's avatar Andres Freund

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
parent faab14ec
...@@ -52,7 +52,7 @@ get_relation_info_hook_type get_relation_info_hook = NULL; ...@@ -52,7 +52,7 @@ get_relation_info_hook_type get_relation_info_hook = NULL;
static bool infer_collation_opclass_match(InferenceElem *elem, Relation idxRel, 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 int32 get_rel_data_width(Relation rel, int32 *attr_widths);
static List *get_relation_constraints(PlannerInfo *root, static List *get_relation_constraints(PlannerInfo *root,
Oid relationObjectId, RelOptInfo *rel, Oid relationObjectId, RelOptInfo *rel,
...@@ -616,8 +616,7 @@ infer_arbiter_indexes(PlannerInfo *root) ...@@ -616,8 +616,7 @@ infer_arbiter_indexes(PlannerInfo *root)
* this for both expressions and ordinary (non-expression) * this for both expressions and ordinary (non-expression)
* attributes appearing as inference elements. * attributes appearing as inference elements.
*/ */
if (!infer_collation_opclass_match(elem, idxRel, inferAttrs, if (!infer_collation_opclass_match(elem, idxRel, idxExprs))
idxExprs))
goto next; goto next;
/* /*
...@@ -682,11 +681,10 @@ next: ...@@ -682,11 +681,10 @@ next:
* infer_collation_opclass_match - ensure infer element opclass/collation match * infer_collation_opclass_match - ensure infer element opclass/collation match
* *
* Given unique index inference element from inference specification, if * Given unique index inference element from inference specification, if
* collation was specified, or if opclass (represented here as opfamily + * collation was specified, or if opclass was specified, verify that there is
* opcintype) was specified, verify that there is at least one matching * at least one matching indexed attribute (occasionally, there may be more).
* indexed attribute (occasionally, there may be more). Skip this in the * Skip this in the common case where inference specification does not include
* common case where inference specification does not include collation or * collation or opclass (instead matching everything, regardless of cataloged
* opclass (instead matching everything, regardless of cataloged
* collation/opclass of indexed attribute). * collation/opclass of indexed attribute).
* *
* At least historically, Postgres has not offered collations or opclasses * At least historically, Postgres has not offered collations or opclasses
...@@ -708,11 +706,12 @@ next: ...@@ -708,11 +706,12 @@ next:
*/ */
static bool static bool
infer_collation_opclass_match(InferenceElem *elem, Relation idxRel, infer_collation_opclass_match(InferenceElem *elem, Relation idxRel,
Bitmapset *inferAttrs, List *idxExprs) List *idxExprs)
{ {
AttrNumber natt; AttrNumber natt;
Oid inferopfamily = InvalidOid; /* OID of att opfamily */ Oid inferopfamily = InvalidOid; /* OID of opclass opfamily */
Oid inferopcinputtype = InvalidOid; /* OID of att opfamily */ Oid inferopcinputtype = InvalidOid; /* OID of opclass input type */
int nplain = 0; /* # plain attrs observed */
/* /*
* If inference specification element lacks collation/opclass, then no * If inference specification element lacks collation/opclass, then no
...@@ -735,6 +734,10 @@ infer_collation_opclass_match(InferenceElem *elem, Relation idxRel, ...@@ -735,6 +734,10 @@ infer_collation_opclass_match(InferenceElem *elem, Relation idxRel,
Oid opfamily = idxRel->rd_opfamily[natt - 1]; Oid opfamily = idxRel->rd_opfamily[natt - 1];
Oid opcinputtype = idxRel->rd_opcintype[natt - 1]; Oid opcinputtype = idxRel->rd_opcintype[natt - 1];
Oid collation = idxRel->rd_indcollation[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 && if (elem->inferopclass != InvalidOid &&
(inferopfamily != opfamily || inferopcinputtype != opcinputtype)) (inferopfamily != opfamily || inferopcinputtype != opcinputtype))
...@@ -750,12 +753,23 @@ infer_collation_opclass_match(InferenceElem *elem, Relation idxRel, ...@@ -750,12 +753,23 @@ infer_collation_opclass_match(InferenceElem *elem, Relation idxRel,
continue; continue;
} }
if ((IsA(elem->expr, Var) && /* If one matching index att found, good enough -- return true */
bms_is_member(((Var *) elem->expr)->varattno, inferAttrs)) || if (IsA(elem->expr, Var))
list_member(idxExprs, elem->expr))
{ {
/* Found one match - good enough */ if (((Var *) elem->expr)->varattno == attno)
return true; 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;
} }
} }
......
...@@ -141,6 +141,24 @@ drop index collation_index_key; ...@@ -141,6 +141,24 @@ drop index collation_index_key;
drop index both_index_key; drop index both_index_key;
drop index both_index_expr_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 -- Single key tests
-- --
create unique index key_index on insertconflicttest(key); create unique index key_index on insertconflicttest(key);
......
...@@ -57,6 +57,18 @@ drop index collation_index_key; ...@@ -57,6 +57,18 @@ drop index collation_index_key;
drop index both_index_key; drop index both_index_key;
drop index both_index_expr_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 -- Single key tests
-- --
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment