Re: Re: proposal: new polymorphic types - commontype and commontypearray

Hi,

the possibility to use polymorphic types is a specific interesting
PostgreSQL feature. The polymorphic types allows to use almost all types,
but when some type is selected, then this type is required strictly without
possibility to use some implicit casting.

So if I have a fx(anyelement, anyelement), then I can call function fx with
parameters (int, int), (numeric, numeric), but I cannot to use parameters
(int, numeric). The strict design has sense, but for few important cases is
too restrictive. We are not able to implement (with plpgsql) functions like
coalesce, greatest, least where all numeric types can be used.

Alternative solution can be based on usage "any" type. But we can work with
this type only from "C" extensions, and there is some performance
penalization due dynamic casting inside function.

Four years ago I proposed implicit casting to common type of arguments with
anyelement type.

/message-id/CAFj8pRCZVo_xoW0cfxt%3DmmgjXKBgr3Gm1VMGL_zx9wDRHmm6Cw%40mail.gmail.com

My proposal was rejected, because it introduce compatibility issues.

Now I have a solution that doesn't break anything. With two new polymorphic
types: commontype and commontypearray we can write functions like coalesce,
greatest, ..

More, these types are independent on current polymorphic types - and can be
used with current polymorphic types together to cover some new use cases.

CREATE OR REPLACE FUNCTION fx(anyelement, commontype, anyelement,
commontype)
RETURNS commontype

or

CREATE OR REPLACE FUNCTION fx(anyelement, commontype, anyelement,
commontype)
RETURNS anyelement

and commontype and anyelement types can be really independent.

Comments, notes?

Regards

Pavel

Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> writes:
> Four years ago I proposed implicit casting to common type of arguments with
> anyelement type.
> /message-id/CAFj8pRCZVo_xoW0cfxt%3DmmgjXKBgr3Gm1VMGL_zx9wDRHmm6Cw%40mail.gmail.com
> My proposal was rejected, because it introduce compatibility issues.

Yup.

> Now I have a solution that doesn't break anything. With two new polymorphic
> types: commontype and commontypearray we can write functions like coalesce,
> greatest, ..

I think this is a good idea at its core, but I don't like the specifics
too much.

I agree with the basic idea of introducing a second, independent set of
polymorphic-type variables. Way back when we first discussed polymorphic
types, we thought maybe we should invent anyelement2 and anyarray2, and
perhaps even more pairs, to allow polymorphic functions to deal with two
or more base types. We didn't do that for lack of convincing examples of
the need for it, but I expected some would emerge; I'm rather astonished
that we've gotten by for so many years without adding a second set.
So where I think we should go with this is to solve that need while
we're at it.

However, this proposal doesn't do so, because it omits "commonrange".
I'm prepared to believe that we don't need "commonenum"; that would
presumably have the semantics of "resolve the common type and then
it must be an enum". And that seems pretty useless, because there
are no type resolution rules that would let us choose one enum out of
a set. (I suppose somebody might create implicit casts between some
enum types, but it doesn't seem very likely.) I also suspect that
we could get away without "commonnonarray". Anynonarray is really
just a hack that we invented to avoid ambiguity around the ||
operator, and an equivalent need would likely not come up for this
second set of types. (I could be wrong though; I'm not sure right
now whether array_agg's use of anynonarray rather than anyelement
is essential or just randomness.) But neither of those arguments
apply to commonrange; in fact it's highly likely that somebody would
want to have "myfunc(commontype, commontype) returns commonrange"
as a customized range constructor that can deal with slightly
different input types.

My second problem with this proposal is that it simply ignores
the naming precedent of the existing polymorphic types. We have
a convention that polymorphic types are named "any-something",
and I do not think we should just toss that overboard. Moreover,
if we do end up needing "commonnonarray" or "commonenum", those
names are ugly, typo-prone, and unreasonably long.

We could do worse than to call these types anyelement2, anyarray2,
anyrange2 and just document that their resolution rule depends
on finding a common type rather than identical base types.
I suppose that's not too pretty --- it reminds one of Oracle finally
getting varchar semantics right with varchar2 :-(. Another idea
is anyelementc, anyarrayc, anyrangec ("c" for "common") but that's
not pretty either. Anyway I think the names need to be any-something.

I haven't particularly studied the patch code, but I will note that
this sort of change seems pretty dumb:

@@ -953,7 +953,7 @@ make_scalar_array_op(ParseState *pstate, List *opname,
* enforce_generic_type_consistency may or may not have replaced a
* polymorphic type with a real one.
*/
- if (IsPolymorphicType(declared_arg_types[1]))
+ if (IsPolymorphicTypeAny(declared_arg_types[1]))
{
/* assume the actual array type is OK */
res_atypeId = atypeId;

Why would we want to reject the new poly types here? Or just about
anyplace else that tests IsPolymorphicType? The argument-type resolution
functions themselves need to distinguish the two groups of types,
at least for some purposes, but it's very hard to believe anyplace
else should do so.

regards, tom lane

so 26. 1. 2019 v 1:20 odesílatel Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> napsal:

> Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> writes:
> > Four years ago I proposed implicit casting to common type of arguments
> with
> > anyelement type.
> >
> /message-id/CAFj8pRCZVo_xoW0cfxt%3DmmgjXKBgr3Gm1VMGL_zx9wDRHmm6Cw%40mail.gmail.com
> > My proposal was rejected, because it introduce compatibility issues.
>
> Yup.
>
> > Now I have a solution that doesn't break anything. With two new
> polymorphic
> > types: commontype and commontypearray we can write functions like
> coalesce,
> > greatest, ..
>
> I think this is a good idea at its core, but I don't like the specifics
> too much.
>
> I agree with the basic idea of introducing a second, independent set of
> polymorphic-type variables. Way back when we first discussed polymorphic
> types, we thought maybe we should invent anyelement2 and anyarray2, and
> perhaps even more pairs, to allow polymorphic functions to deal with two
> or more base types. We didn't do that for lack of convincing examples of
> the need for it, but I expected some would emerge; I'm rather astonished
> that we've gotten by for so many years without adding a second set.
> So where I think we should go with this is to solve that need while
> we're at it.
>

I still expect we can have a more polymorphic types like any***(N)

There are important questions

1. Has a sense to have more distinct polymorphic types with same be behave?
- I see a benefit of this possibility - we can introduce anyelement2 .. AN2
and we can have a function

fx(AN,AN, AN2,AN2) .. that means P2 and P4 should to have a same types
like P1 and P3.

2. What are a strategy of choosing real type for polymorphic types? - now
only equivalence is supported, but I can see more possibilities

* common type - I did it
* first win - often used in Oracle

The common type strategy is more important, because it is typical for some
"pseudo" functions like coalesce, least, greatest, .. in Postgres - and
extension's developers can design functions more compatible with core
functionality.

first win can be interesting for me (like Orafce creator and maintainer).
It can increase level of similarity implemented functions there, and reduce
work when queries are ported to Postgres. But this is important for smaller
group of PostgreSQL users.

>
> However, this proposal doesn't do so, because it omits "commonrange".
> I'm prepared to believe that we don't need "commonenum"; that would
> presumably have the semantics of "resolve the common type and then
> it must be an enum". And that seems pretty useless, because there
> are no type resolution rules that would let us choose one enum out of
> a set. (I suppose somebody might create implicit casts between some
> enum types, but it doesn't seem very likely.) I also suspect that
> we could get away without "commonnonarray". Anynonarray is really
> just a hack that we invented to avoid ambiguity around the ||
> operator, and an equivalent need would likely not come up for this
> second set of types. (I could be wrong though; I'm not sure right
> now whether array_agg's use of anynonarray rather than anyelement
> is essential or just randomness.) But neither of those arguments
> apply to commonrange; in fact it's highly likely that somebody would
> want to have "myfunc(commontype, commontype) returns commonrange"
> as a customized range constructor that can deal with slightly
> different input types.
>

I implemented just minimal set of new polymorphic types, just for
demonstration of my idea. Better coverage of other variants (where it has a
sense) is not a problem. Now, mostly I am searching a design where can be a
some agreement.

>
> My second problem with this proposal is that it simply ignores
> the naming precedent of the existing polymorphic types. We have
> a convention that polymorphic types are named "any-something",
> and I do not think we should just toss that overboard. Moreover,
> if we do end up needing "commonnonarray" or "commonenum", those
> names are ugly, typo-prone, and unreasonably long.
>
> We could do worse than to call these types anyelement2, anyarray2,
> anyrange2 and just document that their resolution rule depends
> on finding a common type rather than identical base types.
> I suppose that's not too pretty --- it reminds one of Oracle finally
> getting varchar semantics right with varchar2 :-(. Another idea
> is anyelementc, anyarrayc, anyrangec ("c" for "common") but that's
> not pretty either. Anyway I think the names need to be any-something.
>

I am open to any ideas. I don't like anyelement2, anyarray2 because

a) it is not verbose - and really different behave should not be signed by
number
b) I can imagine very well more anyelementX types.

I don't think so length is too important factor (but I fully agree -
shorter is better here). The polymorphic types are not too common.

I though about your proposed anyelementc, but the "c" is not much visible.
Can we use snake notation?

commontype, commottype_array, commontype_nonarray ..
common_type, common_type_array, ...

I am not fully happy with "commontype", but I didn't find better

> I haven't particularly studied the patch code, but I will note that
> this sort of change seems pretty dumb:
>
> @@ -953,7 +953,7 @@ make_scalar_array_op(ParseState *pstate, List *opname,
> * enforce_generic_type_consistency may or may not have replaced a
> * polymorphic type with a real one.
> */
> - if (IsPolymorphicType(declared_arg_types[1]))
> + if (IsPolymorphicTypeAny(declared_arg_types[1]))
> {
> /* assume the actual array type is OK */
> res_atypeId = atypeId;
>
> Why would we want to reject the new poly types here? Or just about
> anyplace else that tests IsPolymorphicType? The argument-type resolution
> functions themselves need to distinguish the two groups of types,
> at least for some purposes, but it's very hard to believe anyplace
> else should do so.
>

Just I use original behave everywhere where I had not a stronger idea to
use new polymorphic types there.

Regards

Pavel

>
> regards, tom lane
>

>
>
>
>> My second problem with this proposal is that it simply ignores
>> the naming precedent of the existing polymorphic types. We have
>> a convention that polymorphic types are named "any-something",
>> and I do not think we should just toss that overboard. Moreover,
>> if we do end up needing "commonnonarray" or "commonenum", those
>> names are ugly, typo-prone, and unreasonably long.
>>
>
the convention "any-something" is joined with just currently implemented
families of polymorphic types. I propose new family, so I think so it
should not be named "any-xxxx"

Maybe we can use some form of typemod - but typemod is ignored for function
parameters - so it can be much more significant change

a alternative, probably very simple, but less power solution can be some
special flag for function parameters - at the end, it is similar to
previous solution.

I can imagine

create or replace function fx(p1 anyelement use_common_type, p2 anyelement,
...)
create or replace function fx2(p1 int, p2 int, variadic p3 anyarray
use_common_type)

or maybe

create or replace function fx(p1 anyelement, p2 anyelement ...) ...
language plpgsql options (use_common_type = true)

or we can drop it - on other thread you propose supported functions - can
be some function, that can preproces parameters - and can replace
polymorphic types by real types.

Comments, notes?

Pavel

> regards, tom lane
>>
>

On Fri, Jan 25, 2019 at 7:21 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Anyway I think the names need to be any-something.

To me, that seems unnecessarily rigid. Not a bad idea if we can come
up with something that is otherwise acceptable. But all of your
suggestions sound worse than Pavel's proposal, so...

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

po 28. 1. 2019 v 20:47 odesílatel Robert Haas <robertmhaas(at)gmail(dot)com>
napsal:

> On Fri, Jan 25, 2019 at 7:21 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> > Anyway I think the names need to be any-something.
>
> To me, that seems unnecessarily rigid. Not a bad idea if we can come
> up with something that is otherwise acceptable. But all of your
> suggestions sound worse than Pavel's proposal, so...
>

I implemented commontypenonarray, and commontyperange types. Now, a SQL
functions are supported too.

The naming is same - I had not a better idea. But it can be changed without
any problems, if somebody come with some more acceptable.

I don't think so the name is too important. The polymorphic types are
important, interesting for extension's developers what is small group of
Postgres users.

And personally, I think so commontype and commontypearray are good enough
for not native speakers like me. But I am opened any variant - I think so
this functionality is interesting
and partially coverage one gap in our implementation of polymorphic types.

Regards

Pavel

> --
> Robert Haas
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

st 30. 1. 2019 v 17:00 odesílatel Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
napsal:

>
>
> po 28. 1. 2019 v 20:47 odesílatel Robert Haas <robertmhaas(at)gmail(dot)com>
> napsal:
>
>> On Fri, Jan 25, 2019 at 7:21 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> > Anyway I think the names need to be any-something.
>>
>> To me, that seems unnecessarily rigid. Not a bad idea if we can come
>> up with something that is otherwise acceptable. But all of your
>> suggestions sound worse than Pavel's proposal, so...
>>
>
> I implemented commontypenonarray, and commontyperange types. Now, a SQL
> functions are supported too.
>
> The naming is same - I had not a better idea. But it can be changed
> without any problems, if somebody come with some more acceptable.
>
> I don't think so the name is too important. The polymorphic types are
> important, interesting for extension's developers what is small group of
> Postgres users.
>
> And personally, I think so commontype and commontypearray are good enough
> for not native speakers like me. But I am opened any variant - I think so
> this functionality is interesting
> and partially coverage one gap in our implementation of polymorphic types.
>

maybe "supertype". It is one char shorter .. somewhere is term
"supperclass, ..."

In Czech language this term is short, "nadtyp", but probably it is not
acceptable :)

> Regards
>
> Pavel
>
>
>
>> --
>> Robert Haas
>> EnterpriseDB: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>

On Wed, Jan 30, 2019 at 05:08:03PM +0100, Pavel Stehule wrote:
> maybe "supertype". It is one char shorter .. somewhere is term
> "supperclass, ..."
>
> In Czech language this term is short, "nadtyp", but probably it is not
> acceptable :)

Moved to next CF.
--
Michael

On 2/4/19 4:21 AM, Michael Paquier wrote:
> On Wed, Jan 30, 2019 at 05:08:03PM +0100, Pavel Stehule wrote:
>> maybe "supertype". It is one char shorter .. somewhere is term
>> "supperclass, ..."
>>
>> In Czech language this term is short, "nadtyp", but probably it is not
>> acceptable :)
>
> Moved to next CF.

This thread has been very quiet for a month. I agree with Andres [1]
that we should push this to PG13.

--
-David
david(at)pgmasters(dot)net

[1]
/message-id/raw/20190214203752.t4hl574k6jlu4t25%40alap3.anarazel.de

út 5. 3. 2019 v 14:38 odesílatel David Steele <david(at)pgmasters(dot)net> napsal:

> On 2/4/19 4:21 AM, Michael Paquier wrote:
> > On Wed, Jan 30, 2019 at 05:08:03PM +0100, Pavel Stehule wrote:
> >> maybe "supertype". It is one char shorter .. somewhere is term
> >> "supperclass, ..."
> >>
> >> In Czech language this term is short, "nadtyp", but probably it is not
> >> acceptable :)
> >
> > Moved to next CF.
>
> This thread has been very quiet for a month. I agree with Andres [1]
> that we should push this to PG13.
>

ok

Pavel

> --
> -David
> david(at)pgmasters(dot)net
>
> [1]
>
> /message-id/raw/20190214203752.t4hl574k6jlu4t25%40alap3.anarazel.de
>
>

David Steele <david(at)pgmasters(dot)net> writes:
> This thread has been very quiet for a month. I agree with Andres [1]
> that we should push this to PG13.

I think the main thing it's blocked on is disagreement on what the
type name should be, which is kind of a silly thing to get blocked on,
but nonetheless it's important ...

regards, tom lane

út 5. 3. 2019 v 15:35 odesílatel Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> napsal:

> David Steele <david(at)pgmasters(dot)net> writes:
> > This thread has been very quiet for a month. I agree with Andres [1]
> > that we should push this to PG13.
>
> I think the main thing it's blocked on is disagreement on what the
> type name should be, which is kind of a silly thing to get blocked on,
> but nonetheless it's important ...
>

I sent some others possible names, but probably this mail was forgotten

What about "ctype" like shortcut for common type? carraytype, cnonarraytype?

Regards

Pavel

>
> regards, tom lane
>

Hi

út 5. 3. 2019 v 18:37 odesílatel Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
napsal:

>
>
> út 5. 3. 2019 v 15:35 odesílatel Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> napsal:
>
>> David Steele <david(at)pgmasters(dot)net> writes:
>> > This thread has been very quiet for a month. I agree with Andres [1]
>> > that we should push this to PG13.
>>
>> I think the main thing it's blocked on is disagreement on what the
>> type name should be, which is kind of a silly thing to get blocked on,
>> but nonetheless it's important ...
>>
>
> I sent some others possible names, but probably this mail was forgotten
>
> What about "ctype" like shortcut for common type? carraytype,
> cnonarraytype?
>

rebase for PostgreSQL 13

Regards

Pavel

> Regards
>
> Pavel
>
>>
>> regards, tom lane
>>
>

The proposals I see above are "commontype", "supertype", "anycommontype",
and various abbreviations of those. I would humbly add "compatibletype".

Fwiw I kind of like commontype.

Alternately an argument could be made that length and typing convenience
isn't really a factor here since database users never have to type these
types. The only place they get written is when defining polymorphic
functions which is a pretty uncommon operation.

In which case a very explicit "anycompatibletype" may be better.

On Tue., Mar. 5, 2019, 12:38 p.m. Pavel Stehule, <pavel(dot)stehule(at)gmail(dot)com>
wrote:

>
>
> út 5. 3. 2019 v 15:35 odesílatel Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> napsal:
>
>> David Steele <david(at)pgmasters(dot)net> writes:
>> > This thread has been very quiet for a month. I agree with Andres [1]
>> > that we should push this to PG13.
>>
>> I think the main thing it's blocked on is disagreement on what the
>> type name should be, which is kind of a silly thing to get blocked on,
>> but nonetheless it's important ...
>>
>
> I sent some others possible names, but probably this mail was forgotten
>
> What about "ctype" like shortcut for common type? carraytype,
> cnonarraytype?
>
> Regards
>
> Pavel
>
>>
>> regards, tom lane
>>
>

Greg Stark <stark(at)mit(dot)edu> writes:
> The proposals I see above are "commontype", "supertype", "anycommontype",
> and various abbreviations of those. I would humbly add "compatibletype".
> Fwiw I kind of like commontype.
> Alternately an argument could be made that length and typing convenience
> isn't really a factor here since database users never have to type these
> types. The only place they get written is when defining polymorphic
> functions which is a pretty uncommon operation.
> In which case a very explicit "anycompatibletype" may be better.

I could go with "anycompatibletype". That would lead us to needing
related names like "anycompatiblearraytype", which is getting annoyingly
long, but you might be right that people wouldn't have to type it that
often.

Also, given the precedent of "anyarray" and "anyrange", it might be
okay to make these just "anycompatible" and "anycompatiblearray".

[ wanders away wondering if psql can tab-complete type names in
function definitions ... ]

regards, tom lane

čt 13. 6. 2019 v 2:37 odesílatel Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> napsal:

> Greg Stark <stark(at)mit(dot)edu> writes:
> > The proposals I see above are "commontype", "supertype", "anycommontype",
> > and various abbreviations of those. I would humbly add "compatibletype".
> > Fwiw I kind of like commontype.
> > Alternately an argument could be made that length and typing convenience
> > isn't really a factor here since database users never have to type these
> > types. The only place they get written is when defining polymorphic
> > functions which is a pretty uncommon operation.
> > In which case a very explicit "anycompatibletype" may be better.
>
> I could go with "anycompatibletype". That would lead us to needing
> related names like "anycompatiblearraytype", which is getting annoyingly
> long, but you might be right that people wouldn't have to type it that
> often.
>
> Also, given the precedent of "anyarray" and "anyrange", it might be
> okay to make these just "anycompatible" and "anycompatiblearray".
>

I like anycompatible and anycompatiblearray.

I'll update the patch

Regards

Pavel

> [ wanders away wondering if psql can tab-complete type names in
> function definitions ... ]
>
> regards, tom lane
>

Hi

pá 14. 6. 2019 v 6:09 odesílatel Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
napsal:

>
>
> čt 13. 6. 2019 v 2:37 odesílatel Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> napsal:
>
>> Greg Stark <stark(at)mit(dot)edu> writes:
>> > The proposals I see above are "commontype", "supertype",
>> "anycommontype",
>> > and various abbreviations of those. I would humbly add "compatibletype".
>> > Fwiw I kind of like commontype.
>> > Alternately an argument could be made that length and typing convenience
>> > isn't really a factor here since database users never have to type these
>> > types. The only place they get written is when defining polymorphic
>> > functions which is a pretty uncommon operation.
>> > In which case a very explicit "anycompatibletype" may be better.
>>
>> I could go with "anycompatibletype". That would lead us to needing
>> related names like "anycompatiblearraytype", which is getting annoyingly
>> long, but you might be right that people wouldn't have to type it that
>> often.
>>
>> Also, given the precedent of "anyarray" and "anyrange", it might be
>> okay to make these just "anycompatible" and "anycompatiblearray".
>>
>
> I like anycompatible and anycompatiblearray.
>
> I'll update the patch
>

and here it is

Regards

Pavel

>
> Regards
>
> Pavel
>
>
>> [ wanders away wondering if psql can tab-complete type names in
>> function definitions ... ]
>>
>> regards, tom lane
>>
>

> On Mon, Jun 17, 2019 at 05:31:40AM +0200, Pavel Stehule wrote:
>
> > I like anycompatible and anycompatiblearray.
> >
> > I'll update the patch
> >
>
> and here it is

Thanks for the patch! I've reviewed it a bit, and have a few small
commentaries:

* There are few traces of copy paste in comments

+static Oid
+select_common_type_from_vector(int nargs, Oid *typeids, bool noerror)
...
+ /*
+ * Nope, so set up for the full algorithm. Note that at this point, lc
+ * points to the first list item with type different from pexpr's; we need
+ * not re-examine any items the previous loop advanced over.
+ */

Seems like it was taken from select_common_type, but in
select_common_type_from_vector there is no `lc`, since it doesn't
accept a list.

* I guess it's would be beneficial to update also commentaries for
check_generic_type_consistency and enforce_generic_type_consistency

* The argument consistency rules are:
*
* 1) All arguments declared ANYELEMENT must have the same datatype.
* ...

Since they do not reflect the current state of things in this patch.

* I've noticed that there is a small difference in how anyelement and
anycompatible behave, namely anycompatible do not handle unknowns:

=# select 'aaa'::anyelement;
anyelement
------------
aaa

=# select 'aaa'::anycompatible;
ERROR: 42846: cannot cast type unknown to anycompatible
LINE 1: select 'aaa'::anycompatible;
^
LOCATION: transformTypeCast, parse_expr.c:2823

It happens due to unknowns being filtered out quite early in
check_generic_type_consistency and similar. By itself this difference it not a
problem, but it causes different error messages in functions:

-- this function accepts anycompatible
=# select test_anycompatible('aaa');
ERROR: 42883: function test_anycompatible(unknown) does not exist
LINE 1: select test_anycompatible('aaa');
^
HINT: No function matches the given name and argument types. You might need to add explicit type casts.
LOCATION: ParseFuncOrColumn, parse_func.c:627

-- this function accepts anyelement
=# select test_anyelement('aaa');
ERROR: 42804: could not determine polymorphic type because input has type unknown
LOCATION: enforce_generic_type_consistency, parse_coerce.c:2177

Although of course it's not that serious.

* I'm also curious about the following situation:

=# create function test_both(a anycompatible) returns anycompatible as $$
begin
return a;
end$$ language plpgsql;
CREATE FUNCTION

=# create function test_both(a anyelement) returns anyelement as $$
begin
return a;
end$$ language plpgsql;
CREATE FUNCTION

=# select test_both('aaa'::text);
ERROR: 42725: function test_both(text) is not unique
LINE 1: select test_both('aaa'::text);
^
HINT: Could not choose a best candidate function. You might need to add explicit type casts.
LOCATION: ParseFuncOrColumn, parse_func.c:568

=# select test_both('aaa'::anyelement);
ERROR: 42804: could not determine polymorphic type because input has type unknown
LOCATION: enforce_generic_type_consistency, parse_coerce.c:2177

Is it possible somehow to invoke any of these functions?

Other than that the functionality looks pretty solid. It may look obvious, but
I've also tested performance in different use cases for anycompatible, looks
the same as for anyelement.

po 25. 11. 2019 v 14:35 odesílatel Dmitry Dolgov <9erthalion6(at)gmail(dot)com>
napsal:

> > On Mon, Jun 17, 2019 at 05:31:40AM +0200, Pavel Stehule wrote:
> >
> > > I like anycompatible and anycompatiblearray.
> > >
> > > I'll update the patch
> > >
> >
> > and here it is
>
> Thanks for the patch! I've reviewed it a bit, and have a few small
> commentaries:
>
> * There are few traces of copy paste in comments
>
> +static Oid
> +select_common_type_from_vector(int nargs, Oid *typeids, bool noerror)
> ...
> + /*
> + * Nope, so set up for the full algorithm. Note that at this
> point, lc
> + * points to the first list item with type different from pexpr's;
> we need
> + * not re-examine any items the previous loop advanced over.
> + */
>

> Seems like it was taken from select_common_type, but in
> select_common_type_from_vector there is no `lc`, since it doesn't
> accept a list.
>

fixed

>
> * I guess it's would be beneficial to update also commentaries for
> check_generic_type_consistency and enforce_generic_type_consistency
>
> * The argument consistency rules are:
> *
> * 1) All arguments declared ANYELEMENT must have the same datatype.
> * ...
>
> Since they do not reflect the current state of things in this patch.
>

I add rules 8 and 9 about ANYCOMPATIBLE types

> * I've noticed that there is a small difference in how anyelement and
> anycompatible behave, namely anycompatible do not handle unknowns:
>
> =# select 'aaa'::anyelement;
> anyelement
> ------------
> aaa
>
> =# select 'aaa'::anycompatible;
> ERROR: 42846: cannot cast type unknown to anycompatible
> LINE 1: select 'aaa'::anycompatible;
> ^
> LOCATION: transformTypeCast, parse_expr.c:2823
>
>
It happens due to unknowns being filtered out quite early in
> check_generic_type_consistency and similar. By itself this difference it
> not a
> problem, but it causes different error messages in functions:
>
> -- this function accepts anycompatible
> =# select test_anycompatible('aaa');
> ERROR: 42883: function test_anycompatible(unknown) does not exist
> LINE 1: select test_anycompatible('aaa');
> ^
> HINT: No function matches the given name and argument types. You
> might need to add explicit type casts.
> LOCATION: ParseFuncOrColumn, parse_func.c:627
>
> -- this function accepts anyelement
> =# select test_anyelement('aaa');
> ERROR: 42804: could not determine polymorphic type because input
> has type unknown
> LOCATION: enforce_generic_type_consistency, parse_coerce.c:2177
>

fixed

> Although of course it's not that serious.
>
> * I'm also curious about the following situation:
>
> =# create function test_both(a anycompatible) returns
> anycompatible as $$
> begin
> return a;
> end$$ language plpgsql;
> CREATE FUNCTION
>
> =# create function test_both(a anyelement) returns anyelement as $$
> begin
> return a;
> end$$ language plpgsql;
> CREATE FUNCTION
>
> =# select test_both('aaa'::text);
> ERROR: 42725: function test_both(text) is not unique
> LINE 1: select test_both('aaa'::text);
> ^
> HINT: Could not choose a best candidate function. You might need
> to add explicit type casts.
> LOCATION: ParseFuncOrColumn, parse_func.c:568
>
> =# select test_both('aaa'::anyelement);
> ERROR: 42804: could not determine polymorphic type because input
> has type unknown
> LOCATION: enforce_generic_type_consistency, parse_coerce.c:2177
>

fixed

> Is it possible somehow to invoke any of these functions?
>

unfortunately - it's not possible - the construct 'aaa'::"polymorphic type"
doesn't create a value of this type - result type is a text type, and then
it doesn't help in this situation.

It is similar if you create fx(anyelement) and fx(anyarray) - and you
cannot to call fx(anyelement) by fx(1::anyelement)

> Other than that the functionality looks pretty solid. It may look obvious,
> but
> I've also tested performance in different use cases for anycompatible,
> looks
> the same as for anyelement.
>

Thank you for review, I am sending fixed patch

Regards

Pavel

Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> writes:
> [ anycompatible-types-20191127.patch ]

I'm starting to review this patch seriously. I've found some bugs and
things I didn't like, and the documentation certainly needs work, but
I think I can get it to a committable state before too much longer.

What I want to talk about right now is some preliminary refactoring
that I'd like to do, as shown in the 0001 patch below. (0002 is the
rest of the patch as I currently have it.) There are two main things
in it:

1. Rearrange the macros in pseudotypes.c so that we don't have any
pure-boilerplate functions that aren't built by the macros. I don't
think this should be controversial, as it's not changing anything
functionally.

2. Refactor the function signature validation logic in pg_proc.c and
pg_aggregate.c to avoid having duplicate logic between those two.
I did that by creating new functions in parse_coerce.c (for lack of
a better place) that say whether a proposed result type or aggregate
transition type is valid given a particular set of declared input types.
The reason that this might be controversial is that it forces a slightly
less precise error detail message to be issued, since the call site that's
throwing the error doesn't know exactly which rule was being violated.
(For example, before there was a specific error message about anyrange
result requiring an anyrange input, and now there isn't.)

I think this is all right, mainly because we'd probably end up with
less-precise messages anyway for the more complex rules that 0002 is
going to add. If anybody's really hot about it, we could complicate
the API, say by having the call sites pass in the primary error message
or by having the checking subroutines pass back an errdetail string.

We definitely need to do *something* about that, because it's already
the case that pg_aggregate.c is out of step with pg_proc.c about
polymorphism rules --- it's not enforcing the anyrange rule. I think
there's probably no user-reachable bug in that, because an aggregate
is constrained by its implementation functions for which the rule
would be enforced, but it still seems not good.

Thoughts?

regards, tom lane

pá 13. 3. 2020 v 23:42 odesílatel Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> napsal:

> Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> writes:
> > [ anycompatible-types-20191127.patch ]
>
> I'm starting to review this patch seriously. I've found some bugs and
> things I didn't like, and the documentation certainly needs work, but
> I think I can get it to a committable state before too much longer.
>
> What I want to talk about right now is some preliminary refactoring
> that I'd like to do, as shown in the 0001 patch below. (0002 is the
> rest of the patch as I currently have it.) There are two main things
> in it:
>
> 1. Rearrange the macros in pseudotypes.c so that we don't have any
> pure-boilerplate functions that aren't built by the macros. I don't
> think this should be controversial, as it's not changing anything
> functionally.
>
> 2. Refactor the function signature validation logic in pg_proc.c and
> pg_aggregate.c to avoid having duplicate logic between those two.
> I did that by creating new functions in parse_coerce.c (for lack of
> a better place) that say whether a proposed result type or aggregate
> transition type is valid given a particular set of declared input types.
> The reason that this might be controversial is that it forces a slightly
> less precise error detail message to be issued, since the call site that's
> throwing the error doesn't know exactly which rule was being violated.
> (For example, before there was a specific error message about anyrange
> result requiring an anyrange input, and now there isn't.)
>
> I think this is all right, mainly because we'd probably end up with
> less-precise messages anyway for the more complex rules that 0002 is
> going to add. If anybody's really hot about it, we could complicate
> the API, say by having the call sites pass in the primary error message
> or by having the checking subroutines pass back an errdetail string.
>
> We definitely need to do *something* about that, because it's already
> the case that pg_aggregate.c is out of step with pg_proc.c about
> polymorphism rules --- it's not enforcing the anyrange rule. I think
> there's probably no user-reachable bug in that, because an aggregate
> is constrained by its implementation functions for which the rule
> would be enforced, but it still seems not good.
>

Unfortunately the error message " A function returning "anyrange" must have
at least one "anyrange" argument." will be missing.

This prerequisite is not intuitive. Second question is if we need special
rule for anyrange.

Regards

Pavel

> Thoughts?
>
> regards, tom lane
>
>

Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> writes:
> pá 13. 3. 2020 v 23:42 odesílatel Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> napsal:
>> The reason that this might be controversial is that it forces a slightly
>> less precise error detail message to be issued, since the call site that's
>> throwing the error doesn't know exactly which rule was being violated.
>> (For example, before there was a specific error message about anyrange
>> result requiring an anyrange input, and now there isn't.)

> Unfortunately the error message " A function returning "anyrange" must have
> at least one "anyrange" argument." will be missing.

Yeah, that's what I said. But does it really add anything beyond the
proposed text "A function returning a polymorphic type must have at least
one matching polymorphic argument"? I don't think it'd be terribly
helpful to say "A function returning anyelement must have at least one
anyelement, anyarray, anynonarray, anyenum, or anyrange argument", and
for sure such an error message would be a pain to maintain.

regards, tom lane

so 14. 3. 2020 v 14:26 odesílatel Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> napsal:

> Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> writes:
> > pá 13. 3. 2020 v 23:42 odesílatel Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> napsal:
> >> The reason that this might be controversial is that it forces a slightly
> >> less precise error detail message to be issued, since the call site
> that's
> >> throwing the error doesn't know exactly which rule was being violated.
> >> (For example, before there was a specific error message about anyrange
> >> result requiring an anyrange input, and now there isn't.)
>
> > Unfortunately the error message " A function returning "anyrange" must
> have
> > at least one "anyrange" argument." will be missing.
>
> Yeah, that's what I said. But does it really add anything beyond the
> proposed text "A function returning a polymorphic type must have at least
> one matching polymorphic argument"? I don't think it'd be terribly
> helpful to say "A function returning anyelement must have at least one
> anyelement, anyarray, anynonarray, anyenum, or anyrange argument", and
> for sure such an error message would be a pain to maintain.
>

The error message in your first patch is ok for all types without anyrange.
A behave of this type is more strict and +/- different than from other
polymorphic types.

Pavel

> regards, tom lane
>

Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> writes:
> Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> napsal:
>> Yeah, that's what I said. But does it really add anything beyond the
>> proposed text "A function returning a polymorphic type must have at least
>> one matching polymorphic argument"? I don't think it'd be terribly
>> helpful to say "A function returning anyelement must have at least one
>> anyelement, anyarray, anynonarray, anyenum, or anyrange argument", and
>> for sure such an error message would be a pain to maintain.

> The error message in your first patch is ok for all types without anyrange.
> A behave of this type is more strict and +/- different than from other
> polymorphic types.

Well, here's a version that does it like that, but personally I find these
messages too verbose and not an improvement on what I had before.

(This is also rebased over the stuff I committed yesterday.)

regards, tom lane

ne 15. 3. 2020 v 17:48 odesílatel Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> napsal:

> Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> writes:
> > Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> napsal:
> >> Yeah, that's what I said. But does it really add anything beyond the
> >> proposed text "A function returning a polymorphic type must have at
> least
> >> one matching polymorphic argument"? I don't think it'd be terribly
> >> helpful to say "A function returning anyelement must have at least one
> >> anyelement, anyarray, anynonarray, anyenum, or anyrange argument", and
> >> for sure such an error message would be a pain to maintain.
>
> > The error message in your first patch is ok for all types without
> anyrange.
> > A behave of this type is more strict and +/- different than from other
> > polymorphic types.
>
> Well, here's a version that does it like that, but personally I find these
> messages too verbose and not an improvement on what I had before.
>

There was a problem just with anyrange type. This last version looks
perfect.

Regards

Pavel

> (This is also rebased over the stuff I committed yesterday.)
>
> regards, tom lane
>
>

Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> writes:
> ne 15. 3. 2020 v 17:48 odesílatel Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> napsal:
>> Well, here's a version that does it like that, but personally I find these
>> messages too verbose and not an improvement on what I had before.

> There was a problem just with anyrange type. This last version looks
> perfect.

If you think that "matching polymorphic types" is too vague, I'm
not sure there's much daylight between there and spelling it out
in full as this latest patch does. "anyrange is the only problem"
might be a tenable viewpoint today, but once this patchset goes
in there's going to be much more scope for confusion about which
arguments potentially match a polymorphic result.

regards, tom lane

I wrote:
> Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> writes:
>> There was a problem just with anyrange type. This last version looks
>> perfect.

> If you think that "matching polymorphic types" is too vague, I'm
> not sure there's much daylight between there and spelling it out
> in full as this latest patch does. "anyrange is the only problem"
> might be a tenable viewpoint today, but once this patchset goes
> in there's going to be much more scope for confusion about which
> arguments potentially match a polymorphic result.

On further reflection it seems like that's actually a fairly convincing
argument for going with the more-verbose style. Hence, I pushed 0001
that way.

The cfbot will be unhappy at this point, but I need to rebase the
main patch again ...

regards, tom lane

I wrote:
> The cfbot will be unhappy at this point, but I need to rebase the
> main patch again ...

And rebased. Still not quite happy about some of the details in
enforce_generic_type_consistency, and I've not looked at the test
cases or documentation at all. But this should make the cfbot
happy.

regards, tom lane

So ... there is a definitional question here that doesn't seem to have
been mentioned anywhere in the thread. For the traditional polymorphic
types, we insist that at least one non-unknown input be supplied, thus
you get

regression=# create function foo(anyelement, anyelement) returns bool
regression-# language sql as 'select $1 = $2';
CREATE FUNCTION

regression=# select foo('a', 'b');
ERROR: could not determine polymorphic type because input has type unknown

regression=# select foo('a', 'b'::text);
foo
-----
f
(1 row)

As this patch stands, the ANYCOMPATIBLE types also require that:

regression=# create function foo2(anycompatible, anycompatible) returns bool
language sql as 'select $1 = $2';
CREATE FUNCTION

regression=# select foo2('a', 'b');
ERROR: could not determine polymorphic common type because input has type unknown

However, it seems to me that this is inconsistent with the definition,
namely that we resolve the common type the same way select_common_type()
does, because select_common_type() will choose TEXT when given all-unknown
inputs. So shouldn't we choose TEXT here?

Admittedly, the fact that select_common_type() falls back to TEXT is a
bit of a wart, so maybe we don't want to propagate it here. But if we
don't, we'll have to document the selection rule as almost but not
quite like what it says in section 10.5. That seems confusing.

Documentation issues aside, I'm not quite sure whether this behavior
would be more or less preferable in practice than sticking with the
existing behavior. It seems like it'd be convenient in some cases
but possibly allow mistakes to go undetected in others.

Thoughts?

regards, tom lane

st 18. 3. 2020 v 17:14 odesílatel Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> napsal:

> So ... there is a definitional question here that doesn't seem to have
> been mentioned anywhere in the thread. For the traditional polymorphic
> types, we insist that at least one non-unknown input be supplied, thus
> you get
>
> regression=# create function foo(anyelement, anyelement) returns bool
> regression-# language sql as 'select $1 = $2';
> CREATE FUNCTION
>
> regression=# select foo('a', 'b');
> ERROR: could not determine polymorphic type because input has type unknown
>
> regression=# select foo('a', 'b'::text);
> foo
> -----
> f
> (1 row)
>
> As this patch stands, the ANYCOMPATIBLE types also require that:
>
> regression=# create function foo2(anycompatible, anycompatible) returns
> bool
> language sql as 'select $1 = $2';
> CREATE FUNCTION
>
> regression=# select foo2('a', 'b');
> ERROR: could not determine polymorphic common type because input has type
> unknown
>
> However, it seems to me that this is inconsistent with the definition,
> namely that we resolve the common type the same way select_common_type()
> does, because select_common_type() will choose TEXT when given all-unknown
> inputs. So shouldn't we choose TEXT here?
>
Admittedly, the fact that select_common_type() falls back to TEXT is a
> bit of a wart, so maybe we don't want to propagate it here. But if we
> don't, we'll have to document the selection rule as almost but not
> quite like what it says in section 10.5. That seems confusing.
>
> Documentation issues aside, I'm not quite sure whether this behavior
> would be more or less preferable in practice than sticking with the
> existing behavior. It seems like it'd be convenient in some cases
> but possibly allow mistakes to go undetected in others.
>
> Thoughts?
>

It is difficult question. What I know, this issue is less than we can
expect, because almost all functions are called with typed parameters
(columns, variables).

the fallback to text is enticement but maybe better is consistency with
other polymorphic types.

Maybe users can implement own fallback behave with next custom function

create function foo2(text, text) returns bool
language sql as 'select $1 = $2';

Regards

Pavel

> regards, tom lane
>

Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> writes:
> st 18. 3. 2020 v 17:14 odesílatel Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> napsal:
>> However, it seems to me that this is inconsistent with the definition,
>> namely that we resolve the common type the same way select_common_type()
>> does, because select_common_type() will choose TEXT when given all-unknown
>> inputs. So shouldn't we choose TEXT here?

> It is difficult question. What I know, this issue is less than we can
> expect, because almost all functions are called with typed parameters
> (columns, variables).

True, in actual production queries it's less likely that all the inputs
would be literal constants. So this is mainly about surprise factor,
or lack of it, for handwritten test queries.

> Maybe users can implement own fallback behave with next custom function

> create function foo2(text, text) returns bool
> language sql as 'select $1 = $2';

No, because if you've got that alongside foo2(anycompatible,
anycompatible) then your queries will fail due to both functions
matching anything that's promotable to text.

regards, tom lane

st 18. 3. 2020 v 17:54 odesílatel Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> napsal:

> Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> writes:
> > st 18. 3. 2020 v 17:14 odesílatel Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> napsal:
> >> However, it seems to me that this is inconsistent with the definition,
> >> namely that we resolve the common type the same way select_common_type()
> >> does, because select_common_type() will choose TEXT when given
> all-unknown
> >> inputs. So shouldn't we choose TEXT here?
>
> > It is difficult question. What I know, this issue is less than we can
> > expect, because almost all functions are called with typed parameters
> > (columns, variables).
>
> True, in actual production queries it's less likely that all the inputs
> would be literal constants. So this is mainly about surprise factor,
> or lack of it, for handwritten test queries.
>
> > Maybe users can implement own fallback behave with next custom function
>
> > create function foo2(text, text) returns bool
> > language sql as 'select $1 = $2';
>
> No, because if you've got that alongside foo2(anycompatible,
> anycompatible) then your queries will fail due to both functions
> matching anything that's promotable to text.
>

It is working for anyelement

postgres=# create or replace function fx(anyelement, anyelement)
postgres-# returns bool as $$ select $1=$2 $$ language sql;
CREATE FUNCTION
postgres=# create or replace function fx(text, text)
returns bool as $$ select $1=$2 $$ language sql;
CREATE FUNCTION
postgres=# select fx(1,2);
┌────┐
│ fx │
╞════╡
│ f │
└────┘
(1 row)

postgres=# select fx('ahoj','nazdar');
┌────┐
│ fx │
╞════╡
│ f │
└────┘
(1 row)

> regards, tom lane
>

Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> writes:
> st 18. 3. 2020 v 17:54 odesílatel Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> napsal:
>> No, because if you've got that alongside foo2(anycompatible,
>> anycompatible) then your queries will fail due to both functions
>> matching anything that's promotable to text.

> It is working for anyelement

[ pokes at that... ] Hm, looks like you're getting away with that
because of the preference for functions taking preferred types.
Seems pretty shaky to me though --- you can probably invent
cases that will throw 'ambiguous function' if you try a bit harder.
In any case, I don't think users will understand why they have to
write two versions of the same function.

regards, tom lane

st 18. 3. 2020 v 18:09 odesílatel Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> napsal:

> Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> writes:
> > st 18. 3. 2020 v 17:54 odesílatel Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> napsal:
> >> No, because if you've got that alongside foo2(anycompatible,
> >> anycompatible) then your queries will fail due to both functions
> >> matching anything that's promotable to text.
>
> > It is working for anyelement
>
> [ pokes at that... ] Hm, looks like you're getting away with that
> because of the preference for functions taking preferred types.
> Seems pretty shaky to me though --- you can probably invent
> cases that will throw 'ambiguous function' if you try a bit harder.
> In any case, I don't think users will understand why they have to
> write two versions of the same function.
>

yes, it is not for usual user.

Pavel

>
> regards, tom lane
>

Here's a pretty-nearly-final version of the patch.

In 0001 below, I've left it throwing an error for the case of all
ANYCOMPATIBLE inputs being unknown, but the documentation fails to
acknowledge that. 0002 below is a delta patch that switches to the
other approach of resolving as TEXT. I'm pretty well convinced that
0002 is what we should do, so I have not bothered to write a doc
change that would explain 0001's behavior on this point.

regards, tom lane

I wrote:
> Here's a pretty-nearly-final version of the patch.
> In 0001 below, I've left it throwing an error for the case of all
> ANYCOMPATIBLE inputs being unknown, but the documentation fails to
> acknowledge that. 0002 below is a delta patch that switches to the
> other approach of resolving as TEXT. I'm pretty well convinced that
> 0002 is what we should do, so I have not bothered to write a doc
> change that would explain 0001's behavior on this point.

Pushed with the resolve-to-TEXT mod, and some last-minute
polishing from a final re-read of the patch.

regards, tom lane

čt 19. 3. 2020 v 16:44 odesílatel Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> napsal:

> I wrote:
> > Here's a pretty-nearly-final version of the patch.
> > In 0001 below, I've left it throwing an error for the case of all
> > ANYCOMPATIBLE inputs being unknown, but the documentation fails to
> > acknowledge that. 0002 below is a delta patch that switches to the
> > other approach of resolving as TEXT. I'm pretty well convinced that
> > 0002 is what we should do, so I have not bothered to write a doc
> > change that would explain 0001's behavior on this point.
>
> Pushed with the resolve-to-TEXT mod, and some last-minute
> polishing from a final re-read of the patch.
>

great, thank you very much

Pavel

> regards, tom lane
>