Re: Inaccurate description of UNION/CASE/etc type selection - Mailing list pgsql-docs

From David G. Johnston
Subject Re: Inaccurate description of UNION/CASE/etc type selection
Date
Msg-id CAKFQuwbn05z25KPPAwwA+nqvyb_3eqLM+Q4Awu==9bdVJhum8A@mail.gmail.com
Whole thread Raw
In response to Inaccurate description of UNION/CASE/etc type selection  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: Inaccurate description of UNION/CASE/etc type selection  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-docs
On Sun, Aug 16, 2020 at 2:26 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
We had a question about why an ARRAY[] construct was resolving the
array's type strangely [1]. 

In this specific complaint it strikes me as undesirable to have an lossy implicit cast from text to name - and if that wasn't present then text would have been chosen as expected.
 
The documentation about this [2] says
that the relevant resolution rules are:

  5. Choose the first non-unknown input type which is a preferred type in
  that category, if there is one.

  6. 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.)

But what select_common_type() actually does is:

            else if (!pispreferred &&
                     can_coerce_type(1, &ptype, &ntype, COERCION_IMPLICIT) &&
                     !can_coerce_type(1, &ntype, &ptype, COERCION_IMPLICIT))
            {
                /*
                 * take new type if can coerce to it implicitly but not the
                 * other way; but if we have a preferred type, stay on it.
                 */
                pexpr = nexpr;
                ptype = ntype;
                pcategory = ncategory;
                pispreferred = nispreferred;
            }

(ptype is the currently selected common type, ntype is the next
input type to consider, and we've already eliminated cases involving
UNKNOWN.)

Seems like 5 and 6 need to be merged - the chosen type is the first one that all subsequent types can be implicitly cast to.  We do not guarantee that previous types will be implicitly convertible to this type.

In pseudo-code:
else if (can_coerce(n->p)) continue /* matches when pispreferred */
else if (can_coerce(p->n)) "change chosen type"
else continue /* but now we expect a runtime implicit cast not found error */

To go along with the above simplification:

  /* move on to next one if no new information... */
if (ntype != UNKNOWNOID && ntype != ptype) {
should be written positively as
if (ntype == UNKNOWNOID || ntype == ptype)
    continue;

Random thought, once we get to the end of the loop why not conditionally go over the list a second time so the elements prior to the selected value's type are re-considered in lieu of the new choice?  That at least clears up the documentation to state that all encountered types are compared against the chosen type for implicit casting instead of just those appearing after the one we settled upon. Maybe on the second pass we don't actually change the chosen type but instead provide for a better error message in the case of a missing implicit type cast.

I do agree that while this doesn't feel like an ideal algorithm it seems undesirable to change the implementation - or at least its behavior in non-error situations.  Given the lack of complaints here error handling improvements don't seem like a win either.

David J.

pgsql-docs by date:

Previous
From: Tom Lane
Date:
Subject: Inaccurate description of UNION/CASE/etc type selection
Next
From: Tom Lane
Date:
Subject: Re: Inaccurate description of UNION/CASE/etc type selection