Обсуждение: range bug in resolve_generic_type?

Hello,

I was looking at resolve_generic_type to add anymultirange support,
and the range logic doesn't seem correct to me. This function takes 3
type Oids:

- declared_type is the declared type of a function parameter whose
actual type it would like to deduce.
- context_{declared,actual}_type are the {declared,actual} types of
another parameter, as a hint.

Here is the logic in pseudocode:

    if (declared_type == ANYARRAYOID)
    {
        if (context_declared_type == ANYARRAYOID)
            return the array type
        else if (context_declared_type == ANYELEMENTOID ||
                 context_declared_type == ANYNONARRAYOID ||
                 context_declared_type == ANYENUMOID ||
                 context_declared_type == ANYRANGEOID)
                 context_declared_type == ANYMULTIRANGEOID)
            return an array of those things
        }
    else if (declared_type == ANYELEMENTOID ||
               declared_type == ANYNONARRAYOID ||
               declared_type == ANYENUMOID ||
               declared_type == ANYRANGEOID)
        {
        if (context_declared_type == ANYARRAYOID)
            return the element type
        else if (context_declared_type == ANYRANGEOID)
            return the element type
        else if (context_declared_type == ANYELEMENTOID ||
                 context_declared_type == ANYNONARRAYOID ||
                 context_declared_type == ANYENUMOID)
            return the actual type
    }
    else {
        return declared_type  // since it's not polymorphic....
    }

It seems to me that these inputs would give invalid results:

resolve_generic_type(ANYARRAYOID, x, ANYRANGEOID) - this will return
an array of the *range type*, but that contracts the normal
relationship between anyelement and anyrange. It should return an
array of the range's element type.

resolve_generic_type(ANYRANGEOID, x, ANYRANGEOID) - this will return
the known range's *element* type, but it should return the known
range.

Fortunately we never call the function in either of those ways. The
only call site I could find is utils/fmgr/funcapi.c, and it only calls
it like this:

    resolve_generic_type(ANYELEMENTOID, anyarray_type, ANYARRAYOID)
    resolve_generic_type(ANYELEMENTOID, anyrange_type, ANYRANGEOID)
    resolve_generic_type(ANYARRAYOID, anyelement_type, ANYELEMENTOID)

So I'm curious what I should do about all that, if anything. I'm happy
to fix it (although I'm not sure how I'd test my changes), but it
seems worth asking first. The first case in particular we *could* use
to guess an anyarray's type if we wanted to, so I could add that to
funcapi.c and then probably invent some scenario to test it. For the
second case, I'm guessing if a function has the *same* polymorphic
parameter we probably make an inference without using
resolve_generic_type, since they should obviously match. But does
anyone here have a suggestion?

Thanks!
Paul

Paul A Jungwirth <pj@illuminatedcomputing.com> writes:
> I was looking at resolve_generic_type to add anymultirange support,
> and the range logic doesn't seem correct to me.

Hmmm...

> resolve_generic_type(ANYARRAYOID, x, ANYRANGEOID) - this will return
> an array of the *range type*, but that contracts the normal
> relationship between anyelement and anyrange. It should return an
> array of the range's element type.

I seem to recall that we discussed this exact point during development
of the range feature, and concluded that this was the behavior we
wanted, ie, treat anyrange like a scalar for this purpose.  Otherwise
there isn't any way to use a polymorphic function to build an array
of ranges, and that seemed more useful than being able to build
an array of the range elements.  Jeff might remember more here.

> resolve_generic_type(ANYRANGEOID, x, ANYRANGEOID) - this will return
> the known range's *element* type, but it should return the known
> range.

Yeah, that seems like a flat-out bug.

> Fortunately we never call the function in either of those ways.

Wouldn't it depend on the signature+result type of the user-defined
function we're dealing with?  (That is, calls with constant argument
types are probably not the interesting ones.)

            regards, tom lane

On Tue, Aug 27, 2019 at 8:23 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > resolve_generic_type(ANYARRAYOID, x, ANYRANGEOID) - this will return
> > an array of the *range type*, but that contracts the normal
> > relationship between anyelement and anyrange. It should return an
> > array of the range's element type.
>
> I seem to recall that we discussed this exact point during development
> of the range feature, and concluded that this was the behavior we
> wanted, ie, treat anyrange like a scalar for this purpose.  Otherwise
> there isn't any way to use a polymorphic function to build an array
> of ranges

Well, I don't think that works anyway. At least I couldn't get it to
work here:
https://www.postgresql.org/message-id/CA%2BrenyVOjb4xQZGjdCnA54-1nzVSY%2B47-h4nkM-EP5J%3D1z%3Db9w%40mail.gmail.com

But also check_generic_type_consistency works the way I'm saying:

- if anyrange = r then anyelement = elemOf(r)
- if anyarray = a then anyelement = elemOf(a)
- elemOf(r) = elemOf(a)

So resolve_generic_type should agree with that, right?

Also, I'm interested in adding not just anymultirange but also
anyrangearray, which *would* let you have polymorphic
arrays-of-ranges. (I thought I would need anyrangearray for a
multirange constructor, but actually now I think I might not need
it---maybe. But still it seems like a helpful thing.)

> > Fortunately we never call the function in either of those ways.
>
> Wouldn't it depend on the signature+result type of the user-defined
> function we're dealing with?  (That is, calls with constant argument
> types are probably not the interesting ones.)

I suppose an extension could call it (although it seems unlikely). But
I couldn't find anywhere in the Postgres code that doesn't call it
with hardcoded arguments. (I certainly could have missed something
though.)

Thanks!
Paul

On Tue, Aug 27, 2019 at 8:52 AM Paul A Jungwirth
<pj@illuminatedcomputing.com> wrote:
>
> On Tue, Aug 27, 2019 at 8:23 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > I seem to recall that we discussed this exact point during development
> > of the range feature, and concluded that this was the behavior we
> > wanted, ie, treat anyrange like a scalar for this purpose.  Otherwise
> > there isn't any way to use a polymorphic function to build an array
> > of ranges

One more thing about this: The docs only say that elemOf(anyrange) =
anyelement and elemOf(anyarray) = anyelement. I already added
anymultirange to that page, so here is what I have (from my
forthcoming patch). It also includes anyrangearray. I thought it might
be useful here to clarify how we could support polymorphic
arrays-of-ranges going forward, while avoiding any contradictions:

    <para>
     Seven pseudo-types of special interest are <type>anyelement</type>,
     <type>anyarray</type>, <type>anynonarray</type>, <type>anyenum</type>,
     <type>anyrange</type>, <type>anymultirange</type>, and
     <type>anyrangearray</type>.
     which are collectively called <firstterm>polymorphic types</firstterm>.
     Any function declared using these types is said to be
     a <firstterm>polymorphic function</firstterm>.  A polymorphic function can
     operate on many different data types, with the specific data type(s)
     being determined by the data types actually passed to it in a particular
     call.
    </para>

    <para>
     Polymorphic arguments and results are tied to each other and are resolved
     to a specific data type when a query calling a polymorphic function is
     parsed.  Each position (either argument or return value) declared as
     <type>anyelement</type> is allowed to have any specific actual
     data type, but in any given call they must all be the
     <emphasis>same</emphasis> actual type. Each
     position declared as <type>anyarray</type> can have any array data type,
     but similarly they must all be the same type.  And similarly,
     positions declared as <type>anyrange</type> must all be the same range
     type.  Likewise for <type>anymultirange</type> and
<type>anyrangearray</type>.
    </para>

    <para>
     Furthermore, if there are
     positions declared <type>anyarray</type> and others declared
     <type>anyelement</type>, the actual array type in the
     <type>anyarray</type> positions must be an array whose elements are
     the same type appearing in the <type>anyelement</type> positions.
     <type>anynonarray</type> is treated exactly the same as
<type>anyelement</type>,
     but adds the additional constraint that the actual type must not be
     an array type.
     <type>anyenum</type> is treated exactly the same as
<type>anyelement</type>,
     but adds the additional constraint that the actual type must
     be an enum type.
    </para>

    <para>
     Similarly, if there are positions declared <type>anyrange</type>
     and others declared <type>anyelement</type>, the actual range type in
     the <type>anyrange</type> positions must be a range whose subtype is
     the same type appearing in the <type>anyelement</type> positions.
     The types <type>anymultirange</type> and <type>anyrangearray</type> accept
     a multirange or array of any range type, respectively.
     If they appear along with an <type>anyrange</type> then they must be a
     multirange/array of that range type.
     If a function has both <type>anymultirange</type> and
     <type>anyrangearray</type>, they must contain ranges of the same type,
     even if there is no <type>anyrange</type> parameter.
    </para>

    <para>
     Thus, when more than one argument position is declared with a polymorphic
     type, the net effect is that only certain combinations of actual argument
     types are allowed.  For example, a function declared as
     <literal>equal(anyelement, anyelement)</literal> will take any
two input values,
     so long as they are of the same data type.
    </para>

I hope that helps!

Yours,
Paul