Thread: 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