Re: polymorphic types - enforce casting to most common type automatically

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