proposal: variadic argument support for least, greatest function

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

proposal: variadic argument support for least, greatest function

Pavel Stehule
Hi

We can pass variadic arguments as a array to any variadic function. But some our old variadic functions doesn't supports this feature.

We cannot to write

SELECT least(VARIADIC ARRAY[1,2,3]);

Attached patch add this possibility to least, greatest functions.

Regards

Pavel

minmax_variadic.patch (15K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: proposal: variadic argument support for least, greatest function

Vik Fearing-4
On 08/11/2018 15:59, Pavel Stehule wrote:

> Hi
>
> We can pass variadic arguments as a array to any variadic function. But
> some our old variadic functions doesn't supports this feature.
>
> We cannot to write
>
> SELECT least(VARIADIC ARRAY[1,2,3]);
>
> Attached patch add this possibility to least, greatest functions.

Is there any particular reason you didn't just make least and greatest
actual functions?
--
Vik Fearing                                          +33 6 46 75 15 36
http://2ndQuadrant.fr     PostgreSQL : Expertise, Formation et Support

Reply | Threaded
Open this post in threaded view
|

Re: proposal: variadic argument support for least, greatest function

Andrew Gierth
>>>>> "Vik" == Vik Fearing <[hidden email]> writes:

 >> Attached patch add this possibility to least, greatest functions.

 Vik> Is there any particular reason you didn't just make least and
 Vik> greatest actual functions?

least() and greatest() have some type unification logic that I don't
think works for actual functions.

create function s(variadic anyarray) returns anyelement
  language sql immutable
  as $$ select min(v) from unnest($1) u(v); $$;

select s(1,2,3); -- works
select s(1,2,3.0);  -- ERROR:  function s(integer, integer, numeric) does not exist
select least(1,2,3.0);  -- works

--
Andrew (irc:RhodiumToad)

Reply | Threaded
Open this post in threaded view
|

Re: proposal: variadic argument support for least, greatest function

Pavel Stehule
In reply to this post by Vik Fearing-4


so 10. 11. 2018 v 19:12 odesílatel Vik Fearing <[hidden email]> napsal:
On 08/11/2018 15:59, Pavel Stehule wrote:
> Hi
>
> We can pass variadic arguments as a array to any variadic function. But
> some our old variadic functions doesn't supports this feature.
>
> We cannot to write
>
> SELECT least(VARIADIC ARRAY[1,2,3]);
>
> Attached patch add this possibility to least, greatest functions.

Is there any particular reason you didn't just make least and greatest
actual functions?

These functions has are able to use most common type and enforce casting. Generic variadic function cannot do it.

Regards

Pavel


--
Vik Fearing                                          +33 6 46 75 15 36
http://2ndQuadrant.fr     PostgreSQL : Expertise, Formation et Support
Reply | Threaded
Open this post in threaded view
|

Re: proposal: variadic argument support for least, greatest function

Tom Lane-2
In reply to this post by Pavel Stehule
Pavel Stehule <[hidden email]> writes:
> We cannot to write
> SELECT least(VARIADIC ARRAY[1,2,3]);
> Attached patch add this possibility to least, greatest functions.

TBH, I don't find that natural at all.  If I were looking for the
functionality "smallest element of an array", I think I'd expect to find
that exposed as "array_smallest(anyarray) returns anyelement", not as
some weird syntax option for LEAST.

It also seems rather inconsistent that this behaves so differently
from, eg,

=# select least(array[1,2], array[3,4]);
 least
-------
 {1,2}
(1 row)

Normally, if you have a variadic function, it doesn't also take arrays,
so that there's less possibility for confusion.

The implementation seems mighty ugly too, in that it has to treat this
as entirely disjoint from MinMaxExpr's normal argument interpretation.
But that seems like a symptom of the fact that the definition is
disjointed itself.

In short, I'd rather see this done with a couple of array functions,
independently of MinMaxExpr.

                        regards, tom lane

Reply | Threaded
Open this post in threaded view
|

Re: proposal: variadic argument support for least, greatest function

Pavel Stehule


st 9. 1. 2019 v 1:07 odesílatel Tom Lane <[hidden email]> napsal:
Pavel Stehule <[hidden email]> writes:
> We cannot to write
> SELECT least(VARIADIC ARRAY[1,2,3]);
> Attached patch add this possibility to least, greatest functions.

TBH, I don't find that natural at all.  If I were looking for the
functionality "smallest element of an array", I think I'd expect to find
that exposed as "array_smallest(anyarray) returns anyelement", not as
some weird syntax option for LEAST.

The target of this patch is a consistency LEAST, GREATEST variadic functions (implementet) with generic variadic functions.

Sure it is possible to implement array_smallest(anyarray), but it different. This patch try to eliminate unpleasing surprising about different behave LEAST, GREATEST from other variadic functions.


It also seems rather inconsistent that this behaves so differently
from, eg,

=# select least(array[1,2], array[3,4]);
 least
-------
 {1,2}
(1 row)

Normally, if you have a variadic function, it doesn't also take arrays,
so that there's less possibility for confusion.

This is different case - the keyword VARIADIC was not used here.


The implementation seems mighty ugly too, in that it has to treat this
as entirely disjoint from MinMaxExpr's normal argument interpretation.
But that seems like a symptom of the fact that the definition is
disjointed itself.

I don't think so there is any other possibility - I have not a possibility to unpack a array to elements inside analyze stage.


In short, I'd rather see this done with a couple of array functions,
independently of MinMaxExpr.

It doesn't help to user, when they try to use VARIADIC keyword on LEAST, GREATEST functions.

Regards

Pavel
 

                        regards, tom lane
Reply | Threaded
Open this post in threaded view
|

Re: proposal: variadic argument support for least, greatest function

Chapman Flack
The following review has been posted through the commitfest application:
make installcheck-world:  tested, failed
Implements feature:       not tested
Spec compliant:           not tested
Documentation:            not tested

The argument for consistency with other functions that are variadic makes sense to me: although they are different from ordinary variadic functions in the type unification they do, their description in the manual simply calls them functions without calling attention to any way that they are different beasts. So, a reader might reasonably be surprised that VARIADIC doesn't work in the usual way.

This patch applies (with some offsets) but the build produces several incompatible pointer type assignment warnings, and fails on errors where fcinfo->arg is no longer a thing (so should be rebased over the variable-length function call args patch).

It does not yet add regression tests, or update the documentation in func.sgml.
Reply | Threaded
Open this post in threaded view
|

Re: proposal: variadic argument support for least, greatest function

Pavel Stehule
Hi

út 12. 2. 2019 v 2:00 odesílatel Chapman Flack <[hidden email]> napsal:
The following review has been posted through the commitfest application:
make installcheck-world:  tested, failed
Implements feature:       not tested
Spec compliant:           not tested
Documentation:            not tested

The argument for consistency with other functions that are variadic makes sense to me: although they are different from ordinary variadic functions in the type unification they do, their description in the manual simply calls them functions without calling attention to any way that they are different beasts. So, a reader might reasonably be surprised that VARIADIC doesn't work in the usual way.

This patch applies (with some offsets) but the build produces several incompatible pointer type assignment warnings, and fails on errors where fcinfo->arg is no longer a thing (so should be rebased over the variable-length function call args patch).

It does not yet add regression tests, or update the documentation in func.sgml.

here is fixed patch

Regards

Pavel

minmax_variadic-20190212.patch (15K) Download Attachment