From 5ad70564f46a5fc782191eb8010d90aaca9a762e Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sat, 29 Jan 2022 11:41:12 -0500 Subject: Fix failure to validate the result of select_common_type(). Although select_common_type() has a failure-return convention, an apparent successful return just provides a type OID that *might* work as a common supertype; we've not validated that the required casts actually exist. In the mainstream use-cases that doesn't matter, because we'll proceed to invoke coerce_to_common_type() on each input, which will fail appropriately if the proposed common type doesn't actually work. However, a few callers didn't read the (nonexistent) fine print, and thought that if they got back a nonzero OID then the coercions were sure to work. This affects in particular the recently-added "anycompatible" polymorphic types; we might think that a function/operator using such types matches cases it really doesn't. A likely end result of that is unexpected "ambiguous operator" errors, as for example in bug #17387 from James Inform. Another, much older, case is that the parser might try to transform an "x IN (list)" construct to a ScalarArrayOpExpr even when the list elements don't actually have a common supertype. It doesn't seem desirable to add more checking to select_common_type itself, as that'd just slow down the mainstream use-cases. Instead, write a separate function verify_common_type that performs the missing checks, and add a call to that where necessary. Likewise add verify_common_type_from_oids to go with select_common_type_from_oids. Back-patch to v13 where the "anycompatible" types came in. (The symptom complained of in bug #17387 doesn't appear till v14, but that's just because we didn't get around to converting || to use anycompatible till then.) In principle the "x IN (list)" fix could go back all the way, but I'm not currently convinced that it makes much difference in real-world cases, so I won't bother for now. Discussion: https://postgr.es/m/17387-5dfe54b988444963@postgresql.org --- src/backend/parser/parse_coerce.c | 66 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 65 insertions(+), 1 deletion(-) (limited to 'src/backend/parser/parse_coerce.c') diff --git a/src/backend/parser/parse_coerce.c b/src/backend/parser/parse_coerce.c index 70466153d9d..e914f8ac8be 100644 --- a/src/backend/parser/parse_coerce.c +++ b/src/backend/parser/parse_coerce.c @@ -1295,6 +1295,10 @@ parser_coercion_errposition(ParseState *pstate, * rather than throwing an error on failure. * 'which_expr': if not NULL, receives a pointer to the particular input * expression from which the result type was taken. + * + * Caution: "failure" just means that there were inputs of different type + * categories. It is not guaranteed that all the inputs are coercible to the + * selected type; caller must check that (see verify_common_type). */ Oid select_common_type(ParseState *pstate, List *exprs, const char *context, @@ -1423,6 +1427,10 @@ select_common_type(ParseState *pstate, List *exprs, const char *context, * earlier entries in the array have some preference over later ones. * On failure, return InvalidOid if noerror is true, else throw an error. * + * Caution: "failure" just means that there were inputs of different type + * categories. It is not guaranteed that all the inputs are coercible to the + * selected type; caller must check that (see verify_common_type_from_oids). + * * Note: neither caller will pass any UNKNOWNOID entries, so the tests * for that in this function are dead code. However, they don't cost much, * and it seems better to keep this logic as close to select_common_type() @@ -1545,6 +1553,48 @@ coerce_to_common_type(ParseState *pstate, Node *node, return node; } +/* + * verify_common_type() + * Verify that all input types can be coerced to a proposed common type. + * Return true if so, false if not all coercions are possible. + * + * Most callers of select_common_type() don't need to do this explicitly + * because the checks will happen while trying to convert input expressions + * to the right type, e.g. in coerce_to_common_type(). However, if a separate + * check step is needed to validate the applicability of the common type, call + * this. + */ +bool +verify_common_type(Oid common_type, List *exprs) +{ + ListCell *lc; + + foreach(lc, exprs) + { + Node *nexpr = (Node *) lfirst(lc); + Oid ntype = exprType(nexpr); + + if (!can_coerce_type(1, &ntype, &common_type, COERCION_IMPLICIT)) + return false; + } + return true; +} + +/* + * verify_common_type_from_oids() + * As above, but work from an array of type OIDs. + */ +static bool +verify_common_type_from_oids(Oid common_type, int nargs, const Oid *typeids) +{ + for (int i = 0; i < nargs; i++) + { + if (!can_coerce_type(1, &typeids[i], &common_type, COERCION_IMPLICIT)) + return false; + } + return true; +} + /* * check_generic_type_consistency() * Are the actual arguments potentially compatible with a @@ -1791,7 +1841,13 @@ check_generic_type_consistency(const Oid *actual_arg_types, true); if (!OidIsValid(anycompatible_typeid)) - return false; /* there's no common supertype */ + return false; /* there's definitely no common supertype */ + + /* We have to verify that the selected type actually works */ + if (!verify_common_type_from_oids(anycompatible_typeid, + n_anycompatible_args, + anycompatible_actual_types)) + return false; if (have_anycompatible_nonarray) { @@ -2222,6 +2278,14 @@ enforce_generic_type_consistency(const Oid *actual_arg_types, anycompatible_actual_types, false); + /* We have to verify that the selected type actually works */ + if (!verify_common_type_from_oids(anycompatible_typeid, + n_anycompatible_args, + anycompatible_actual_types)) + ereport(ERROR, + (errcode(ERRCODE_DATATYPE_MISMATCH), + errmsg("arguments of anycompatible family cannot be cast to a common type"))); + if (have_anycompatible_array) { anycompatible_array_typeid = get_array_type(anycompatible_typeid); -- cgit v1.2.3