Re: BUG #17387: Working in PG13 but not in PGH14: array_agg(RECORD) - Mailing list pgsql-bugs

From Tom Lane
Subject Re: BUG #17387: Working in PG13 but not in PGH14: array_agg(RECORD)
Date
Msg-id 2856894.1643413680@sss.pgh.pa.us
Whole thread Raw
In response to Re: BUG #17387: Working in PG13 but not in PGH14: array_agg(RECORD)  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: BUG #17387: Working in PG13 but not in PGH14: array_agg(RECORD)  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-bugs
I wrote:
> So it seems like this is specific to type record[] somehow.

Ah, no, I found it: the callers of select_common_type_from_oids
assume that its result is guaranteed valid, which is not so.
We need to explicitly check can_coerce_type.  This oversight
allows check_generic_type_consistency to succeed when it should
not, which in turn allows us to decide that record and record[]
are OK as matches to all three of those operators.

This apparently escaped notice before because we've only tested
cases in which incompatible arguments were of different typcategory.
record and record[] are both of category 'P', which might be a
dumb idea.  But this would be a bug anyway.

We need something like the attached, but I'm going to nose
around for other oversights.

            regards, tom lane

diff --git a/src/backend/parser/parse_coerce.c b/src/backend/parser/parse_coerce.c
index d674e31405..aa4cfaa52e 100644
--- a/src/backend/parser/parse_coerce.c
+++ b/src/backend/parser/parse_coerce.c
@@ -1298,6 +1298,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 of 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,
@@ -1426,6 +1430,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 of the inputs are coercible
+ * to the selected type; caller must check that (see verify_common_type).
+ *
  * 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()
@@ -1548,6 +1556,27 @@ 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.
+ *
+ * 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.
+ */
+static bool
+verify_common_type(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;
+}
+
 /*
  * select_common_typmod()
  *        Determine the common typmod of a list of input expressions.
@@ -1917,7 +1946,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(anycompatible_typeid,
+                                n_anycompatible_args,
+                                anycompatible_actual_types))
+            return false;

         if (have_anycompatible_nonarray)
         {
@@ -2494,6 +2529,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(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);

pgsql-bugs by date:

Previous
From: Tom Lane
Date:
Subject: Re: BUG #17387: Working in PG13 but not in PGH14: array_agg(RECORD)
Next
From: Noah Misch
Date:
Subject: Re: BUG #17386: btree index corruption after reindex concurrently on write heavy table