Re: Domains versus polymorphic functions, redux - Mailing list pgsql-hackers

From Tom Lane
Subject Re: Domains versus polymorphic functions, redux
Date
Msg-id 23554.1307485736@sss.pgh.pa.us
Whole thread Raw
In response to Re: Domains versus polymorphic functions, redux  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: Domains versus polymorphic functions, redux
Re: Domains versus polymorphic functions, redux
List pgsql-hackers
I wrote:
> Anyway, I think we're out of time to do anything about the issue for
> 9.1.  I think what we'd better do is force a downcast in the context
> of matching to an ANYARRAY parameter, and leave the other cases to
> revisit later.

Attached is a draft patch to do the above.  It's only lightly tested,
and could use some regression test additions, but it seems to fix
Regina's complaint.

Note that I changed coerce_type's behavior for both ANYARRAY and ANYENUM
targets, but the latter behavioral change is unreachable since the other
routines in parse_coerce.c will not match a domain-over-enum to ANYENUM.
I am half tempted to extend the patch so they will, which would allow
cases like this to work:

regression=#  create type color as enum('red','green','blue');
CREATE TYPE
regression=# select enum_first('green'::color);
 enum_first
------------
 red
(1 row)

regression=# create domain dcolor as color;
CREATE DOMAIN
regression=# select enum_first('green'::dcolor);
ERROR:  function enum_first(dcolor) does not exist
LINE 1: select enum_first('green'::dcolor);
               ^
HINT:  No function matches the given name and argument types. You might need to add explicit type casts.

I'm unsure though if there's any support for this further adventure,
since it wouldn't be fixing a 9.1 regression.

Comments?

            regards, tom lane

diff --git a/src/backend/parser/parse_coerce.c b/src/backend/parser/parse_coerce.c
index 0418972517eee4df52dbdc8f7807aa8fa528a674..e0727f12285d73b3ce48b53ba91cd5d8d9fc87e4 100644
*** a/src/backend/parser/parse_coerce.c
--- b/src/backend/parser/parse_coerce.c
*************** coerce_type(ParseState *pstate, Node *no
*** 143,151 ****
      }
      if (targetTypeId == ANYOID ||
          targetTypeId == ANYELEMENTOID ||
!         targetTypeId == ANYNONARRAYOID ||
!         (targetTypeId == ANYARRAYOID && inputTypeId != UNKNOWNOID) ||
!         (targetTypeId == ANYENUMOID && inputTypeId != UNKNOWNOID))
      {
          /*
           * Assume can_coerce_type verified that implicit coercion is okay.
--- 143,149 ----
      }
      if (targetTypeId == ANYOID ||
          targetTypeId == ANYELEMENTOID ||
!         targetTypeId == ANYNONARRAYOID)
      {
          /*
           * Assume can_coerce_type verified that implicit coercion is okay.
*************** coerce_type(ParseState *pstate, Node *no
*** 154,168 ****
           * it's OK to treat an UNKNOWN constant as a valid input for a
           * function accepting ANY, ANYELEMENT, or ANYNONARRAY.    This should be
           * all right, since an UNKNOWN value is still a perfectly valid Datum.
-          * However an UNKNOWN value is definitely *not* an array, and so we
-          * mustn't accept it for ANYARRAY.  (Instead, we will call anyarray_in
-          * below, which will produce an error.)  Likewise, UNKNOWN input is no
-          * good for ANYENUM.
           *
!          * NB: we do NOT want a RelabelType here.
           */
          return node;
      }
      if (inputTypeId == UNKNOWNOID && IsA(node, Const))
      {
          /*
--- 152,195 ----
           * it's OK to treat an UNKNOWN constant as a valid input for a
           * function accepting ANY, ANYELEMENT, or ANYNONARRAY.    This should be
           * all right, since an UNKNOWN value is still a perfectly valid Datum.
           *
!          * NB: we do NOT want a RelabelType here: the exposed type of the
!          * function argument must be its actual type, not the polymorphic
!          * pseudotype.
           */
          return node;
      }
+     if (targetTypeId == ANYARRAYOID ||
+         targetTypeId == ANYENUMOID)
+     {
+         /*
+          * Assume can_coerce_type verified that implicit coercion is okay.
+          *
+          * These cases are unlike the ones above because the exposed type of
+          * the argument must be an actual array or enum type.  In particular
+          * the argument must *not* be an UNKNOWN constant.  If it is, we just
+          * fall through; below, we'll call anyarray_in or anyenum_in, which
+          * will produce an error.  Also, if what we have is a domain over
+          * array or enum, we have to relabel it to its base type.
+          */
+         if (inputTypeId != UNKNOWNOID)
+         {
+             Oid            baseTypeId = getBaseType(inputTypeId);
+
+             if (baseTypeId != inputTypeId)
+             {
+                 RelabelType *r = makeRelabelType((Expr *) node,
+                                                  baseTypeId, -1,
+                                                  InvalidOid,
+                                                  cformat);
+
+                 r->location = location;
+                 return (Node *) r;
+             }
+             /* Not a domain type, so return it as-is */
+             return node;
+         }
+     }
      if (inputTypeId == UNKNOWNOID && IsA(node, Const))
      {
          /*
*************** coerce_to_common_type(ParseState *pstate
*** 1257,1262 ****
--- 1284,1294 ----
   *      (This is a no-op if used in combination with ANYARRAY or ANYENUM, but
   *      is an extra restriction if not.)
   *
+  * Domains over arrays match ANYARRAY, and are immediately flattened to their
+  * base type.  (Thus, for example, we will consider it a match if one ANYARRAY
+  * argument is a domain over int4[] while another one is just int4[].)  Also
+  * notice that such a domain does *not* match ANYNONARRAY.
+  *
   * If we have UNKNOWN input (ie, an untyped literal) for any polymorphic
   * argument, assume it is okay.
   *
*************** check_generic_type_consistency(Oid *actu
*** 1309,1314 ****
--- 1341,1347 ----
          {
              if (actual_type == UNKNOWNOID)
                  continue;
+             actual_type = getBaseType(actual_type);        /* flatten domains */
              if (OidIsValid(array_typeid) && actual_type != array_typeid)
                  return false;
              array_typeid = actual_type;
*************** check_generic_type_consistency(Oid *actu
*** 1346,1353 ****

      if (have_anynonarray)
      {
!         /* require the element type to not be an array */
!         if (type_is_array(elem_typeid))
              return false;
      }

--- 1379,1386 ----

      if (have_anynonarray)
      {
!         /* require the element type to not be an array or domain over array */
!         if (type_is_array_domain(elem_typeid))
              return false;
      }

*************** check_generic_type_consistency(Oid *actu
*** 1406,1411 ****
--- 1439,1448 ----
   *      (This is a no-op if used in combination with ANYARRAY or ANYENUM, but
   *      is an extra restriction if not.)
   *
+  * Domains over arrays match ANYARRAY arguments, and are immediately flattened
+  * to their base type.  (In particular, if the return type is also ANYARRAY,
+  * we'll set it to the base type not the domain type.)
+  *
   * When allow_poly is false, we are not expecting any of the actual_arg_types
   * to be polymorphic, and we should not return a polymorphic result type
   * either.    When allow_poly is true, it is okay to have polymorphic "actual"
*************** enforce_generic_type_consistency(Oid *ac
*** 1485,1490 ****
--- 1522,1528 ----
              }
              if (allow_poly && decl_type == actual_type)
                  continue;        /* no new information here */
+             actual_type = getBaseType(actual_type);        /* flatten domains */
              if (OidIsValid(array_typeid) && actual_type != array_typeid)
                  ereport(ERROR,
                          (errcode(ERRCODE_DATATYPE_MISMATCH),
*************** enforce_generic_type_consistency(Oid *ac
*** 1557,1564 ****

      if (have_anynonarray && elem_typeid != ANYELEMENTOID)
      {
!         /* require the element type to not be an array */
!         if (type_is_array(elem_typeid))
              ereport(ERROR,
                      (errcode(ERRCODE_DATATYPE_MISMATCH),
                     errmsg("type matched to anynonarray is an array type: %s",
--- 1595,1602 ----

      if (have_anynonarray && elem_typeid != ANYELEMENTOID)
      {
!         /* require the element type to not be an array or domain over array */
!         if (type_is_array_domain(elem_typeid))
              ereport(ERROR,
                      (errcode(ERRCODE_DATATYPE_MISMATCH),
                     errmsg("type matched to anynonarray is an array type: %s",
*************** resolve_generic_type(Oid declared_type,
*** 1655,1669 ****
      {
          if (context_declared_type == ANYARRAYOID)
          {
!             /* Use actual type, but it must be an array */
!             Oid            array_typelem = get_element_type(context_actual_type);

              if (!OidIsValid(array_typelem))
                  ereport(ERROR,
                          (errcode(ERRCODE_DATATYPE_MISMATCH),
                           errmsg("argument declared \"anyarray\" is not an array but type %s",
!                                 format_type_be(context_actual_type))));
!             return context_actual_type;
          }
          else if (context_declared_type == ANYELEMENTOID ||
                   context_declared_type == ANYNONARRAYOID ||
--- 1693,1711 ----
      {
          if (context_declared_type == ANYARRAYOID)
          {
!             /*
!              * Use actual type, but it must be an array; or if it's a domain
!              * over array, use the base array type.
!              */
!             Oid            context_base_type = getBaseType(context_actual_type);
!             Oid            array_typelem = get_element_type(context_base_type);

              if (!OidIsValid(array_typelem))
                  ereport(ERROR,
                          (errcode(ERRCODE_DATATYPE_MISMATCH),
                           errmsg("argument declared \"anyarray\" is not an array but type %s",
!                                 format_type_be(context_base_type))));
!             return context_base_type;
          }
          else if (context_declared_type == ANYELEMENTOID ||
                   context_declared_type == ANYNONARRAYOID ||
*************** resolve_generic_type(Oid declared_type,
*** 1687,1699 ****
          if (context_declared_type == ANYARRAYOID)
          {
              /* Use the element type corresponding to actual type */
!             Oid            array_typelem = get_element_type(context_actual_type);

              if (!OidIsValid(array_typelem))
                  ereport(ERROR,
                          (errcode(ERRCODE_DATATYPE_MISMATCH),
                           errmsg("argument declared \"anyarray\" is not an array but type %s",
!                                 format_type_be(context_actual_type))));
              return array_typelem;
          }
          else if (context_declared_type == ANYELEMENTOID ||
--- 1729,1742 ----
          if (context_declared_type == ANYARRAYOID)
          {
              /* Use the element type corresponding to actual type */
!             Oid            context_base_type = getBaseType(context_actual_type);
!             Oid            array_typelem = get_element_type(context_base_type);

              if (!OidIsValid(array_typelem))
                  ereport(ERROR,
                          (errcode(ERRCODE_DATATYPE_MISMATCH),
                           errmsg("argument declared \"anyarray\" is not an array but type %s",
!                                 format_type_be(context_base_type))));
              return array_typelem;
          }
          else if (context_declared_type == ANYELEMENTOID ||
*************** IsBinaryCoercible(Oid srctype, Oid targe
*** 1796,1807 ****

      /* Also accept any array type as coercible to ANYARRAY */
      if (targettype == ANYARRAYOID)
!         if (type_is_array(srctype))
              return true;

      /* Also accept any non-array type as coercible to ANYNONARRAY */
      if (targettype == ANYNONARRAYOID)
!         if (!type_is_array(srctype))
              return true;

      /* Also accept any enum type as coercible to ANYENUM */
--- 1839,1850 ----

      /* Also accept any array type as coercible to ANYARRAY */
      if (targettype == ANYARRAYOID)
!         if (type_is_array_domain(srctype))
              return true;

      /* Also accept any non-array type as coercible to ANYNONARRAY */
      if (targettype == ANYNONARRAYOID)
!         if (!type_is_array_domain(srctype))
              return true;

      /* Also accept any enum type as coercible to ANYENUM */

pgsql-hackers by date:

Previous
From: Robert Haas
Date:
Subject: Re: heap vacuum & cleanup locks
Next
From: Brar Piening
Date:
Subject: Re: smallserial / serial2