Proposed patch to adjust UNION/CASE/etc type resolution rules - Mailing list pgsql-patches

The attached proposed patch addresses the recent gripe that there is no
way for UNION (or related constructs) to return a domain type, because
select_common_type() always smashes all its inputs to base types:
http://archives.postgresql.org/pgsql-performance/2007-11/msg00278.php

The minimum-impact way to fix that seems to be to add an initial test to
see whether all the given types are exactly the same non-unknown type.
If so, return that; otherwise proceed with the existing algorithm.  This
approach has an additional small advantage of making select_common_type()
a bit faster for trivial cases, since it can skip catalog lookups.

Note that this method will not be intelligent about cases involving
domains over domains --- you'll get the base type, even if one of the
domains is a super-domain of all the others.  But it's hard to see how
to handle that efficiently, and it's not clear that there's much call
for it anyway.

The patch also removes some tests for InvalidOid, which have clearly been
dead code for quite some time now, because getBaseType() would fail on
that input.

Also, clarify the manual's not-very-precise description of the existing
algorithm's behavior.

As I mentioned earlier today, I think we should address this for 8.3,
because fooling with the resolution rules seems like something that
should only happen in major releases.

Comments?

            regards, tom lane

Index: doc/src/sgml/typeconv.sgml
===================================================================
RCS file: /cvsroot/pgsql/doc/src/sgml/typeconv.sgml,v
retrieving revision 1.52
diff -c -r1.52 typeconv.sgml
*** doc/src/sgml/typeconv.sgml    5 Jun 2007 21:31:04 -0000    1.52
--- doc/src/sgml/typeconv.sgml    25 Nov 2007 01:33:03 -0000
***************
*** 845,853 ****

  <step performance="required">
  <para>
  If all inputs are of type <type>unknown</type>, resolve as type
  <type>text</type> (the preferred type of the string category).
! Otherwise, ignore the <type>unknown</type> inputs while choosing the result type.
  </para>
  </step>

--- 845,861 ----

  <step performance="required">
  <para>
+ If all inputs are of the same type, and it is not <type>unknown</type>,
+ resolve as that type.  Otherwise, replace any domain types in the list with
+ their underlying base types.
+ </para>
+ </step>
+
+ <step performance="required">
+ <para>
  If all inputs are of type <type>unknown</type>, resolve as type
  <type>text</type> (the preferred type of the string category).
! Otherwise, the <type>unknown</type> inputs will be ignored.
  </para>
  </step>

***************
*** 860,873 ****
  <step performance="required">
  <para>
  Choose the first non-unknown input type which is a preferred type in
! that category or allows all the non-unknown inputs to be implicitly
! converted to it.
  </para>
  </step>

  <step performance="required">
  <para>
! Convert all inputs to the selected type.
  </para>
  </step>
  </procedure>
--- 868,890 ----
  <step performance="required">
  <para>
  Choose the first non-unknown input type which is a preferred type in
! that category, if there is one.
! </para>
! </step>
!
! <step performance="required">
! <para>
! Otherwise, choose the last non-unknown input type that allows all the
! preceding non-unknown inputs to be implicitly converted to it.  (There
! always is such a type, since at least the first type in the list must
! satisfy this condition.)
  </para>
  </step>

  <step performance="required">
  <para>
! Convert all inputs to the selected type.  Fail if there is not a
! conversion from a given input to the selected type.
  </para>
  </step>
  </procedure>
Index: src/backend/parser/parse_coerce.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/parser/parse_coerce.c,v
retrieving revision 2.158
diff -c -r2.158 parse_coerce.c
*** src/backend/parser/parse_coerce.c    15 Nov 2007 21:14:37 -0000    2.158
--- src/backend/parser/parse_coerce.c    25 Nov 2007 01:33:04 -0000
***************
*** 958,964 ****
   *        This is used for determining the output type of CASE and UNION
   *        constructs.
   *
!  * typeids is a nonempty list of type OIDs.  Note that earlier items
   * in the list will be preferred if there is doubt.
   * 'context' is a phrase to use in the error message if we fail to select
   * a usable type.
--- 958,964 ----
   *        This is used for determining the output type of CASE and UNION
   *        constructs.
   *
!  * 'typeids' is a nonempty list of type OIDs.  Note that earlier items
   * in the list will be preferred if there is doubt.
   * 'context' is a phrase to use in the error message if we fail to select
   * a usable type.
***************
*** 971,977 ****
      ListCell   *type_item;

      Assert(typeids != NIL);
!     ptype = getBaseType(linitial_oid(typeids));
      pcategory = TypeCategory(ptype);

      for_each_cell(type_item, lnext(list_head(typeids)))
--- 971,998 ----
      ListCell   *type_item;

      Assert(typeids != NIL);
!     ptype = linitial_oid(typeids);
!
!     /*
!      * If all input types are valid and exactly the same, just pick that type.
!      * This is the only way that we will resolve the result as being a domain
!      * type; otherwise domains are smashed to their base types for comparison.
!      */
!     if (ptype != UNKNOWNOID)
!     {
!         for_each_cell(type_item, lnext(list_head(typeids)))
!         {
!             Oid        ntype = lfirst_oid(type_item);
!
!             if (ntype != ptype)
!                 break;
!         }
!         if (type_item == NULL)            /* got to the end of the list? */
!             return ptype;
!     }
!
!     /* Nope, so set up for the full algorithm */
!     ptype = getBaseType(ptype);
      pcategory = TypeCategory(ptype);

      for_each_cell(type_item, lnext(list_head(typeids)))
***************
*** 979,989 ****
          Oid            ntype = getBaseType(lfirst_oid(type_item));

          /* move on to next one if no new information... */
!         if ((ntype != InvalidOid) && (ntype != UNKNOWNOID) && (ntype != ptype))
          {
!             if ((ptype == InvalidOid) || ptype == UNKNOWNOID)
              {
!                 /* so far, only nulls so take anything... */
                  ptype = ntype;
                  pcategory = TypeCategory(ptype);
              }
--- 1000,1010 ----
          Oid            ntype = getBaseType(lfirst_oid(type_item));

          /* move on to next one if no new information... */
!         if (ntype != UNKNOWNOID && ntype != ptype)
          {
!             if (ptype == UNKNOWNOID)
              {
!                 /* so far, only unknowns so take anything... */
                  ptype = ntype;
                  pcategory = TypeCategory(ptype);
              }
Index: src/test/regress/expected/domain.out
===================================================================
RCS file: /cvsroot/pgsql/src/test/regress/expected/domain.out,v
retrieving revision 1.42
diff -c -r1.42 domain.out
*** src/test/regress/expected/domain.out    29 Oct 2007 19:40:40 -0000    1.42
--- src/test/regress/expected/domain.out    25 Nov 2007 01:33:04 -0000
***************
*** 69,74 ****
--- 69,93 ----
             |
  (3 rows)

+ -- check that union/case/coalesce type resolution handles domains properly
+ select coalesce(4::domainint4, 7) is of (int4) as t;
+  t
+ ---
+  t
+ (1 row)
+
+ select coalesce(4::domainint4, 7) is of (domainint4) as f;
+  f
+ ---
+  f
+ (1 row)
+
+ select coalesce(4::domainint4, 7::domainint4) is of (domainint4) as t;
+  t
+ ---
+  t
+ (1 row)
+
  drop table basictest;
  drop domain domainvarchar restrict;
  drop domain domainnumeric restrict;
Index: src/test/regress/sql/domain.sql
===================================================================
RCS file: /cvsroot/pgsql/src/test/regress/sql/domain.sql,v
retrieving revision 1.25
diff -c -r1.25 domain.sql
*** src/test/regress/sql/domain.sql    29 Oct 2007 19:40:40 -0000    1.25
--- src/test/regress/sql/domain.sql    25 Nov 2007 01:33:04 -0000
***************
*** 58,63 ****
--- 58,68 ----
  select testtext || testvarchar as concat, testnumeric + 42 as sum
  from basictest;

+ -- check that union/case/coalesce type resolution handles domains properly
+ select coalesce(4::domainint4, 7) is of (int4) as t;
+ select coalesce(4::domainint4, 7) is of (domainint4) as f;
+ select coalesce(4::domainint4, 7::domainint4) is of (domainint4) as t;
+
  drop table basictest;
  drop domain domainvarchar restrict;
  drop domain domainnumeric restrict;