From 67e378316a9336f85d982f93dd8ddf7234dfeabb Mon Sep 17 00:00:00 2001 From: "Paul A. Jungwirth" Date: Thu, 13 Nov 2025 16:45:40 -0800 Subject: [PATCH v1] Improve comment and error handling in GetOperatorFromCompareType Despite the comment, we always ereport if we don't find a strategy number. Also we should avoid using uninitialized opfamily and opcintype values to build error messages. If get_opclass_opfamily_and_input_type fails, something is very wrong, since get_opclass_method just succeeded. Let's just fail. That reduces the paths through the function too, so reasoning is easier. I also inverted the conditional to reduce nesting. --- src/backend/commands/indexcmds.c | 49 +++++++++++++++++--------------- 1 file changed, 26 insertions(+), 23 deletions(-) diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index 5712fac3697..0298d6268db 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -2439,8 +2439,7 @@ GetDefaultOpClass(Oid type_id, Oid am_id) * Finds an operator from a CompareType. This is used for temporal index * constraints (and other temporal features) to look up equality and overlaps * operators. We ask an opclass support function to translate from the - * compare type to the internal strategy numbers. If the function isn't - * defined or it gives no result, we set *strat to InvalidStrategy. + * compare type to the internal strategy numbers. */ void GetOperatorFromCompareType(Oid opclass, Oid rhstype, CompareType cmptype, @@ -2456,30 +2455,34 @@ GetOperatorFromCompareType(Oid opclass, Oid rhstype, CompareType cmptype, *opid = InvalidOid; - if (get_opclass_opfamily_and_input_type(opclass, &opfamily, &opcintype)) - { + if (!get_opclass_opfamily_and_input_type(opclass, &opfamily, &opcintype)) /* - * Ask the index AM to translate to its internal stratnum + * Since get_opclass_method succeeded, we shouldn't ever fail here. + * If it happens, don't proceed, since opfamily and opcintype are uninitialized. */ - *strat = IndexAmTranslateCompareType(cmptype, amid, opfamily, true); - if (*strat == InvalidStrategy) - ereport(ERROR, - errcode(ERRCODE_UNDEFINED_OBJECT), - cmptype == COMPARE_EQ ? errmsg("could not identify an equality operator for type %s", format_type_be(opcintype)) : - cmptype == COMPARE_OVERLAP ? errmsg("could not identify an overlaps operator for type %s", format_type_be(opcintype)) : - cmptype == COMPARE_CONTAINED_BY ? errmsg("could not identify a contained-by operator for type %s", format_type_be(opcintype)) : 0, - errdetail("Could not translate compare type %d for operator family \"%s\" of access method \"%s\".", - cmptype, get_opfamily_name(opfamily, false), get_am_name(amid))); + elog(ERROR, "cache lookup failed for opclass %u", opclass); - /* - * We parameterize rhstype so foreign keys can ask for a <@ operator - * whose rhs matches the aggregate function. For example range_agg - * returns anymultirange. - */ - if (!OidIsValid(rhstype)) - rhstype = opcintype; - *opid = get_opfamily_member(opfamily, opcintype, rhstype, *strat); - } + /* + * Ask the index AM to translate to its internal stratnum + */ + *strat = IndexAmTranslateCompareType(cmptype, amid, opfamily, true); + if (*strat == InvalidStrategy) + ereport(ERROR, + errcode(ERRCODE_UNDEFINED_OBJECT), + cmptype == COMPARE_EQ ? errmsg("could not identify an equality operator for type %s", format_type_be(opcintype)) : + cmptype == COMPARE_OVERLAP ? errmsg("could not identify an overlaps operator for type %s", format_type_be(opcintype)) : + cmptype == COMPARE_CONTAINED_BY ? errmsg("could not identify a contained-by operator for type %s", format_type_be(opcintype)) : 0, + errdetail("Could not translate compare type %d for operator family \"%s\" of access method \"%s\".", + cmptype, get_opfamily_name(opfamily, false), get_am_name(amid))); + + /* + * We parameterize rhstype so foreign keys can ask for a <@ operator + * whose rhs matches the aggregate function. For example range_agg + * returns anymultirange. + */ + if (!OidIsValid(rhstype)) + rhstype = opcintype; + *opid = get_opfamily_member(opfamily, opcintype, rhstype, *strat); if (!OidIsValid(*opid)) ereport(ERROR, -- 2.45.0