Peter Eisentraut <peter@eisentraut.org> writes:
> 0002: Add get_opfamily_member_for_cmptype(). This was called
> get_opmethod_member() in your patch set, but I think that name wasn't
> quite right. I also removed the opmethod argument, which was rarely
> used and is somewhat redundant.
Hm, that will throw an error if IndexAmTranslateCompareType fails.
Shouldn't it be made to return InvalidOid instead?
There are two failure modes. In one mode, the AM has a concept of equality, but there is no operator for the given type. In the other mode, the AM simply has no concept of equality.
In DefineIndex, the call:
if (stmt->unique && !stmt->iswithoutoverlaps)
{
idx_eqop = get_opfamily_member_for_cmptype(idx_opfamily,
idx_opcintype,
idx_opcintype,
COMPARE_EQ);
if (!idx_eqop)
ereport(ERROR,
errcode(ERRCODE_UNDEFINED_OBJECT),
errmsg("could not identify an equality operator for type %s", format_type_be(idx_opcintype)),
errdetail("There is no suitable operator in operator family \"%s\" for access method \"%s\".",
get_opfamily_name(idx_opfamily, false), get_am_name(get_opfamily_method(idx_opfamily))));
}
probably should error about COMPARE_EQ not having a strategy in the AM, rather than complaining later that an equality operator cannot be found for the type. So that one seems fine as it is now.
The other caller, refresh_by_match_merge, is similar:
op = get_opfamily_member_for_cmptype(opfamily, opcintype, opcintype, COMPARE_EQ);
if (!OidIsValid(op))
elog(ERROR, "missing equality operator for (%u,%u) in opfamily %u",
opcintype, opcintype, opfamily);
seems fine. There are no other callers as yet.
You have made me a bit paranoid that, in future, we might add additional callers who are not anticipating the error. Should we solve that with a more complete code comment, or do you think refactoring is in order?
