mirror of
https://github.com/postgres/postgres.git
synced 2025-06-29 10:41:53 +03:00
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
This commit is contained in:
@ -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);
|
||||
|
@ -1283,6 +1283,11 @@ transformAExprIn(ParseState *pstate, A_Expr *a)
|
||||
allexprs = list_concat(list_make1(lexpr), rnonvars);
|
||||
scalar_type = select_common_type(pstate, allexprs, NULL, NULL);
|
||||
|
||||
/* We have to verify that the selected type actually works */
|
||||
if (OidIsValid(scalar_type) &&
|
||||
!verify_common_type(scalar_type, allexprs))
|
||||
scalar_type = InvalidOid;
|
||||
|
||||
/*
|
||||
* Do we have an array type to use? Aside from the case where there
|
||||
* isn't one, we don't risk using ScalarArrayOpExpr when the common
|
||||
|
Reference in New Issue
Block a user