Обсуждение: polymorphic types - enforce casting to most common type automatically

Hi

I am sending a proof concept. Current implementation is not suboptimal - I wrote this code for demonstration of current issues, and checking possible side effects of changes in this patch.

The basic problem is strong restrictive implementation of polymorphic types - now these types doesn't allow any cast although it is possible. It can be changed relatively simply I though (after we implemented variadic functions).

CREATE OR REPLACE FUNCTION public.foo1(anyelement, anyelement)
 RETURNS anyelement
 LANGUAGE sql
AS $function$
SELECT $1 + $2;
$function$

CREATE OR REPLACE FUNCTION public.foo2(anyelement, anyelement)
 RETURNS anyarray
 LANGUAGE sql
AS $function$
SELECT ARRAY[$1, $2]
$function$

Now, polymorphic functions don't allow some natively expected calls:

postgres=# select foo1(1,1);
 foo1
------
    2
(1 row)

postgres=# select foo1(1,1.1);
ERROR:  function foo1(integer, numeric) does not exist
LINE 1: select foo1(1,1.1);
               ^
HINT:  No function matches the given name and argument types. You might need to add explicit type casts.

postgres=# select foo2(1,1);
 foo2 
-------
 {1,1}
(1 row)

postgres=# select foo2(1,1.1);
ERROR:  function foo2(integer, numeric) does not exist
LINE 1: select foo2(1,1.1);
               ^
HINT:  No function matches the given name and argument types. You might need to add explicit type casts.


CREATE OR REPLACE FUNCTION public.foo3(VARIADIC anyarray)
 RETURNS anyelement
 LANGUAGE sql
AS $function$
SELECT min(v) FROM unnest($1) g(v)
$function$

postgres=# SELECT foo3(1,2,3);
 foo3
------
    1
(1 row)

postgres=# SELECT foo3(1,2,3.1);
ERROR:  function foo3(integer, integer, numeric) does not exist
LINE 1: SELECT foo3(1,2,3.1);
               ^
HINT:  No function matches the given name and argument types. You might need to add explicit type casts.

Some our functions like COALESCE are not too restrictive and allow to use types from same category.

postgres=# select coalesce(1,1.1);
 coalesce
----------
        1
(1 row)

With attached patch the polymorphic functions use same mechanism as our buildin functions. It is applied on ANYARRAY, ANYELEMENT types only.

postgres=# select foo1(1,1.1), foo2(1,1.1), foo3(1.1,2,3.1);
 foo1 |  foo2   | foo3
------+---------+------
  2.1 | {1,1.1} |  1.1
(1 row)

Comments, notices, ... ?

Regards

Pavel

2014-11-24 20:52 GMT+01:00 Pavel Stehule <pavel.stehule@gmail.com>:

Hello

now a functions with more than one polymorphic arguments are relative fragile due missing casting to most common type. Some our "functions" like "coalesce" can do it, so it is surprising for our users.

our custom polymorphic function foo(anyelement, anyelement) working well for

foo(10,20) or foo(10.1, 20.1), but not for foo(10, 20.1)

I am thinking, so we can add a searching most common type stage without breaking to backing compatibility.

What do you think about it?

Regards

Pavel

Hi

2015-07-10 18:43 GMT+02:00 Tom Lane <tgl@sss.pgh.pa.us>:

Pavel Stehule <pavel.stehule@gmail.com> writes:
> now a functions with more than one polymorphic arguments are relative
> fragile due missing casting to most common type. Some our "functions" like
> "coalesce" can do it, so it is surprising for our users.

> our custom polymorphic function foo(anyelement, anyelement) working well for
> foo(10,20) or foo(10.1, 20.1), but not for foo(10, 20.1)

> I am thinking, so we can add a searching most common type stage without
> breaking to backing compatibility.

> What do you think about it?

I see nobody's replied to this, still, so ...

I think this is simply a bad idea, for a couple of reasons:

1. It will reduce predictability of type resolution.

I don't think - same mechanism we use - it doesn't introduce some new.

 

2. It will greatly increase the risk of getting "ambiguous function call"
failures, because of adding more possible ways to match the same call.
(The argument that we'd not break backwards compatibility is thus bogus.)

Maybe I not described well my idea.

This can generate new conflicts only when new behave will be different than old behave. And different old behave is not possible - it fails on error now. So there is possible, with this patch, some queries can fail on conflict, but this code fails on "function doesn't exists" now. So if there is some possibility of breaking compatibility, then one error can be replaced by different error. It is known best practice to don't mix polymorphic parameters and function overloading.

Why I need it - the motivation, why I returned to this topic is issue https://github.com/orafce/orafce/issues/17 and some questions about same topic on stackoverflow.

There is workaround with "any" type - but I have to repeat lot of work what core analyzer can do, and the code in extension is longer. And I have to write extension in C.

 

Worth noting for onlookers is that the submitted patch seems to be using
UNION-style rules to determine a common type for anyelement arguments,
not just counting the "most common" type among the arguments as you might
think from the subject.  But that doesn't make things any better.

it is related to only polymorphic types.

An example of what would presumably happen if we adopted this sort of rule
(I've not checked whether the patch as written does this, but it would
logically follow) is that appending a float to an integer array would
cause the whole array to be silently promoted to float, with attendant
possible loss of precision for existing array elements. 

it is based on select_common_type() - so it is use only available implicit casts.
 

That does not
seem to me to satisfy the principle of least astonishment.  Related,
even more astonishing behaviors could ensue from type promotion in
anyrange situations, eg range_contains_elem(anyrange,anyelement).
So I think it's just as well that we make people write a cast to show
what they mean in such cases.

The polymorphic parameters create much bigger space - if somebody needs to less variability, then he doesn't use polymorphic params.

I understand to some situation, when we prefer strict work with polymorphic parameters - theoretically we can introduce new option that enforce it.

In fact, if you discount cases involving anyarray and anyrange, we do not
have *any* built-in functions for which this patch would do anything,
except for the three-argument forms of lead() and lag(), where I think it
would be rather astonishing to let the default-value argument control the
result type, anyway.  This leaves me feeling dubious both about the actual
scope of the use-case for such a change, and about whether "use the UNION
rules" would be a sensible heuristic even if we wanted to do something.
There seem to be too many cases where it's not a great idea to put all the
arguments on exactly equal footing for deciding what common type to
choose.

Very common problem of polymorphic parameters is mix of numeric and integers as parameters. It is one known gotcha - and I am trying to solve it.

Regards

Pavel

 

So I'm inclined to mark this patch as Rejected.

                        regards, tom lane

On 07/11/2015 12:19 AM, Pavel Stehule wrote:

2015-07-10 18:43 GMT+02:00 Tom Lane <tgl@sss.pgh.pa.us>:

An example of what would presumably happen if we adopted this sort of rule
(I've not checked whether the patch as written does this, but it would
logically follow) is that appending a float to an integer array would
cause the whole array to be silently promoted to float, with attendant
possible loss of precision for existing array elements.

it is based on select_common_type() - so it is use only available implicit
casts.

Without patch:

postgres=# select pg_typeof(array_append('{1,2,3}'::int[], 1.2::float));
ERROR:  function array_append(integer[], double precision) does not exist

With patch:

postgres=# select pg_typeof(array_append('{1,2,3}'::int[], 1.2::float));
     pg_typeof
--------------------
 double precision[]
(1 row)


Yeah, I agree with Tom that we don't want that change in behaviour. I'll mark this as rejected.

- Heikki