Commit cb37c291 authored by Tom Lane's avatar Tom Lane

Fix index matching for operators with mixed collatable/noncollatable inputs.

If an indexable operator for a non-collatable indexed datatype has a
collatable right-hand input type, any OpExpr for it will be marked with a
nonzero inputcollid (since having one collatable input is sufficient to
make that happen).  However, an index on a non-collatable column certainly
doesn't have any collation.  This caused us to fail to match such operators
to their indexes, because indxpath.c required an exact match of index
collation and clause collation.  It seems correct to allow a match when the
index is collation-less regardless of the clause's inputcollid: an operator
with both noncollatable and collatable inputs could perhaps depend on the
collation of the collatable input, but it could hardly expect the index for
the noncollatable input to have that same collation.

Per bug #6232 from Pierre Ducroquet.  His example is specifically about
"hstore ? text" but the problem seems quite generic.
parent 5e595842
...@@ -41,6 +41,9 @@ ...@@ -41,6 +41,9 @@
#define IsBooleanOpfamily(opfamily) \ #define IsBooleanOpfamily(opfamily) \
((opfamily) == BOOL_BTREE_FAM_OID || (opfamily) == BOOL_HASH_FAM_OID) ((opfamily) == BOOL_BTREE_FAM_OID || (opfamily) == BOOL_HASH_FAM_OID)
#define IndexCollMatchesExprColl(idxcollation, exprcollation) \
((idxcollation) == InvalidOid || (idxcollation) == (exprcollation))
/* Whether to use ScalarArrayOpExpr to build index qualifications */ /* Whether to use ScalarArrayOpExpr to build index qualifications */
typedef enum typedef enum
{ {
...@@ -1181,6 +1184,13 @@ group_clauses_by_indexkey(IndexOptInfo *index, ...@@ -1181,6 +1184,13 @@ group_clauses_by_indexkey(IndexOptInfo *index,
* We do not actually do the commuting here, but we check whether a * We do not actually do the commuting here, but we check whether a
* suitable commutator operator is available. * suitable commutator operator is available.
* *
* If the index has a collation, the clause must have the same collation.
* For collation-less indexes, we assume it doesn't matter; this is
* necessary for cases like "hstore ? text", wherein hstore's operators
* don't care about collation but the clause will get marked with a
* collation anyway because of the text argument. (This logic is
* embodied in the macro IndexCollMatchesExprColl.)
*
* It is also possible to match RowCompareExpr clauses to indexes (but * It is also possible to match RowCompareExpr clauses to indexes (but
* currently, only btree indexes handle this). In this routine we will * currently, only btree indexes handle this). In this routine we will
* report a match if the first column of the row comparison matches the * report a match if the first column of the row comparison matches the
...@@ -1303,7 +1313,7 @@ match_clause_to_indexcol(IndexOptInfo *index, ...@@ -1303,7 +1313,7 @@ match_clause_to_indexcol(IndexOptInfo *index,
bms_is_subset(right_relids, outer_relids) && bms_is_subset(right_relids, outer_relids) &&
!contain_volatile_functions(rightop)) !contain_volatile_functions(rightop))
{ {
if (idxcollation == expr_coll && if (IndexCollMatchesExprColl(idxcollation, expr_coll) &&
is_indexable_operator(expr_op, opfamily, true)) is_indexable_operator(expr_op, opfamily, true))
return true; return true;
...@@ -1322,7 +1332,7 @@ match_clause_to_indexcol(IndexOptInfo *index, ...@@ -1322,7 +1332,7 @@ match_clause_to_indexcol(IndexOptInfo *index,
bms_is_subset(left_relids, outer_relids) && bms_is_subset(left_relids, outer_relids) &&
!contain_volatile_functions(leftop)) !contain_volatile_functions(leftop))
{ {
if (idxcollation == expr_coll && if (IndexCollMatchesExprColl(idxcollation, expr_coll) &&
is_indexable_operator(expr_op, opfamily, false)) is_indexable_operator(expr_op, opfamily, false))
return true; return true;
...@@ -1398,8 +1408,8 @@ match_rowcompare_to_indexcol(IndexOptInfo *index, ...@@ -1398,8 +1408,8 @@ match_rowcompare_to_indexcol(IndexOptInfo *index,
expr_op = linitial_oid(clause->opnos); expr_op = linitial_oid(clause->opnos);
expr_coll = linitial_oid(clause->inputcollids); expr_coll = linitial_oid(clause->inputcollids);
/* Collations must match */ /* Collations must match, if relevant */
if (expr_coll != idxcollation) if (!IndexCollMatchesExprColl(idxcollation, expr_coll))
return false; return false;
/* /*
...@@ -1571,7 +1581,7 @@ match_clause_to_ordering_op(IndexOptInfo *index, ...@@ -1571,7 +1581,7 @@ match_clause_to_ordering_op(IndexOptInfo *index,
/* /*
* We can forget the whole thing right away if wrong collation. * We can forget the whole thing right away if wrong collation.
*/ */
if (expr_coll != idxcollation) if (!IndexCollMatchesExprColl(idxcollation, expr_coll))
return NULL; return NULL;
/* /*
...@@ -1862,7 +1872,7 @@ eclass_matches_any_index(EquivalenceClass *ec, EquivalenceMember *em, ...@@ -1862,7 +1872,7 @@ eclass_matches_any_index(EquivalenceClass *ec, EquivalenceMember *em,
*/ */
if ((index->relam != BTREE_AM_OID || if ((index->relam != BTREE_AM_OID ||
list_member_oid(ec->ec_opfamilies, curFamily)) && list_member_oid(ec->ec_opfamilies, curFamily)) &&
ec->ec_collation == curCollation && IndexCollMatchesExprColl(curCollation, ec->ec_collation) &&
match_index_to_operand((Node *) em->em_expr, indexcol, index)) match_index_to_operand((Node *) em->em_expr, indexcol, index))
return true; return true;
} }
...@@ -2967,7 +2977,8 @@ expand_indexqual_rowcompare(RestrictInfo *rinfo, ...@@ -2967,7 +2977,8 @@ expand_indexqual_rowcompare(RestrictInfo *rinfo,
break; break;
/* Does collation match? */ /* Does collation match? */
if (lfirst_oid(collids_cell) != index->indexcollations[i]) if (!IndexCollMatchesExprColl(index->indexcollations[i],
lfirst_oid(collids_cell)))
break; break;
/* Add opfamily and datatypes to lists */ /* Add opfamily and datatypes to lists */
......
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