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
* 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,
* 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()
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
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)
{
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);
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
extern Node *coerce_to_common_type(ParseState *pstate, Node *node,
Oid targetTypeId,
const char *context);
+extern bool verify_common_type(Oid common_type, List *exprs);
extern bool check_generic_type_consistency(const Oid *actual_arg_types,
const Oid *declared_arg_types,
{0,1,2,3}
(1 row)
+SELECT array_agg(x) || array_agg(x) FROM (VALUES (ROW(1,2)), (ROW(3,4))) v(x);
+ ?column?
+-----------------------------------
+ {"(1,2)","(3,4)","(1,2)","(3,4)"}
+(1 row)
+
+SELECT ROW(1,2) || array_agg(x) FROM (VALUES (ROW(3,4)), (ROW(5,6))) v(x);
+ ?column?
+---------------------------
+ {"(1,2)","(3,4)","(5,6)"}
+(1 row)
+
SELECT * FROM array_op_test WHERE i @> '{32}' ORDER BY seqno;
seqno | i | t
-------+---------------------------------+------------------------------------------------------------------------------------------------------------------------------------
(3 rows)
rollback;
+--
+-- Ordinarily, IN/NOT IN can be converted to a ScalarArrayOpExpr
+-- with a suitably-chosen array type.
+--
+explain (verbose, costs off)
+select random() IN (1, 4, 8.0);
+ QUERY PLAN
+------------------------------------------------------------
+ Result
+ Output: (random() = ANY ('{1,4,8}'::double precision[]))
+(2 rows)
+
+explain (verbose, costs off)
+select random()::int IN (1, 4, 8.0);
+ QUERY PLAN
+---------------------------------------------------------------------------
+ Result
+ Output: (((random())::integer)::numeric = ANY ('{1,4,8.0}'::numeric[]))
+(2 rows)
+
+-- However, if there's not a common supertype for the IN elements,
+-- we should instead try to produce "x = v1 OR x = v2 OR ...".
+-- In most cases that'll fail for lack of all the requisite = operators,
+-- but it can succeed sometimes. So this should complain about lack of
+-- an = operator, not about cast failure.
+select '(0,0)'::point in ('(0,0,0,0)'::box, point(0,0));
+ERROR: operator does not exist: point = box
+LINE 1: select '(0,0)'::point in ('(0,0,0,0)'::box, point(0,0));
+ ^
+HINT: No operator matches the given name and argument types. You might need to add explicit type casts.
SELECT ARRAY[[1,2],[3,4]] || ARRAY[5,6] AS "{{1,2},{3,4},{5,6}}";
SELECT ARRAY[0,0] || ARRAY[1,1] || ARRAY[2,2] AS "{0,0,1,1,2,2}";
SELECT 0 || ARRAY[1,2] || 3 AS "{0,1,2,3}";
+SELECT array_agg(x) || array_agg(x) FROM (VALUES (ROW(1,2)), (ROW(3,4))) v(x);
+SELECT ROW(1,2) || array_agg(x) FROM (VALUES (ROW(3,4)), (ROW(5,6))) v(x);
SELECT * FROM array_op_test WHERE i @> '{32}' ORDER BY seqno;
SELECT * FROM array_op_test WHERE i && '{32}' ORDER BY seqno;
where f1::bpchar = 'foo';
rollback;
+
+
+--
+-- Ordinarily, IN/NOT IN can be converted to a ScalarArrayOpExpr
+-- with a suitably-chosen array type.
+--
+explain (verbose, costs off)
+select random() IN (1, 4, 8.0);
+explain (verbose, costs off)
+select random()::int IN (1, 4, 8.0);
+-- However, if there's not a common supertype for the IN elements,
+-- we should instead try to produce "x = v1 OR x = v2 OR ...".
+-- In most cases that'll fail for lack of all the requisite = operators,
+-- but it can succeed sometimes. So this should complain about lack of
+-- an = operator, not about cast failure.
+select '(0,0)'::point in ('(0,0,0,0)'::box, point(0,0));
projects / postgresql.git / commitdiff