Thread: variadic function support
Hello
this patch enhance current syntax of CREATE FUNCTION statement. It
allows creating functions with variable number of arguments. This
version is different than last my patches. It doesn't need patching
PL. Basic idea is transformation of real arguments (related to
declared variadic argument) to array. All changes are mostly in
parser.
Demo:
CREATE FUNCTION public.least(double precision[]) RETURNS double precision AS $$
SELECT min($1[i])
FROM generate_subscripts($1,1) g(i)
$$ LANGUAGE SQL VARIADIC;
SELECT public.least(3,2,1);
least
-------
1
(1 row)
SELECT public.least(3,2,1,0,-1);
least
-------
-1
CREATE FUNCTION concat(varchar, anyarray) RETURNS varchar AS $$
SELECT array_to_string($2, $1);
$$ LANGUAGE SQL VARIADIC;
SELECT concat('-',2008,10,12);
concat
------------
2008-10-12
(1 row)
Regards
Pavel Stehule
Attachment
Pavel Stehule wrote:
> Hello
>
> this patch enhance current syntax of CREATE FUNCTION statement. It
> allows creating functions with variable number of arguments. This
> version is different than last my patches. It doesn't need patching
> PL. Basic idea is transformation of real arguments (related to
> declared variadic argument) to array. All changes are mostly in
> parser.
>
> Demo:
> CREATE FUNCTION public.least(double precision[]) RETURNS double precision AS $$
> SELECT min($1[i])
> FROM generate_subscripts($1,1) g(i)
> $$ LANGUAGE SQL VARIADIC;
>
> SELECT public.least(3,2,1);
> least
> -------
> 1
> (1 row)
>
> SELECT public.least(3,2,1,0,-1);
> least
> -------
> -1
> CREATE FUNCTION concat(varchar, anyarray) RETURNS varchar AS $$
> SELECT array_to_string($2, $1);
> $$ LANGUAGE SQL VARIADIC;
>
> SELECT concat('-',2008,10,12);
> concat
> ------------
> 2008-10-12
> (1 row)
>
>
>
>
And what about a function that takes 2 arrays as arguments?
This proposal strikes me as half-baked. Either we need proper and full
support for variadic functions, or we don't, but I don't think we need
syntactic sugar like the above (or maybe in this case it's really
syntactic saccharine).
cheers
andrew
Andrew Dunstan <andrew@dunslane.net> writes:
> This proposal strikes me as half-baked. Either we need proper and full
> support for variadic functions, or we don't, but I don't think we need
> syntactic sugar like the above (or maybe in this case it's really
> syntactic saccharine).
What would you consider "proper and full support"?
regards, tom lane
Tom Lane wrote: > Andrew Dunstan <andrew@dunslane.net> writes: > >> This proposal strikes me as half-baked. Either we need proper and full >> support for variadic functions, or we don't, but I don't think we need >> syntactic sugar like the above (or maybe in this case it's really >> syntactic saccharine). >> > > What would you consider "proper and full support"? > > > I don't know. But this doesn't feel like it. I'm not even sure it's possible to so in any sane way alongside overloading. cheers andrew
Andrew Dunstan <andrew@dunslane.net> writes:
> Tom Lane wrote:
>> What would you consider "proper and full support"?
>
> I don't know. But this doesn't feel like it.
That's a fairly weak argument for rejecting a patch that provides a
feature many people have asked for.
I thought the patch was pretty clever, actually. The main functionality
complaint someone might level against it is that all the variadic
arguments have to be (coercible to) the same type. However, that's
still pretty useful, and I don't see a reasonable solution that provides
more generality than that in a type-safe way. I'm quite happy that you
can't write sprintf() using this ;-)
A different line of argument is whether this functionality is
sufficiently badly needed that we should get out in front of the SQL
standard on providing it, and risk being stuck with legacy behavior
if they eventually adopt some other mechanism to solve the same problem.
I'm not sure how worried I am about that. There are certainly a
boatload of Postgres-isms in and around CREATE FUNCTION already,
so it's hard to make a case against "just one more".
regards, tom lane
Tom Lane wrote:
> Andrew Dunstan <andrew@dunslane.net> writes:
>
>> Tom Lane wrote:
>>
>>> What would you consider "proper and full support"?
>>>
>> I don't know. But this doesn't feel like it.
>>
>
> That's a fairly weak argument for rejecting a patch that provides a
> feature many people have asked for.
>
OK. Let me be a bit more specific. I think (forcing myself to be a bit
more analytic than I have been up to now) my main objection is that the
variadic part of the parameters should be marked explicitly in the
formal parameter list.
I don't mind having it limited to a single typed array - as you say we
probably don't want someone implementing sprintf.
But if I have
foo( a text, b int[])
it looks odd if both these calls are legal:
foo('a',1,2,3,)
foo('a',ARRAY[1,2,3])
which I understand would be the case with the current patch.
I'm also still curious to know how the following would be handled:
foo(a text[], b text[])
cheers
andrew
Andrew Dunstan <andrew@dunslane.net> writes:
> But if I have
> foo( a text, b int[])
> it looks odd if both these calls are legal:
> foo('a',1,2,3,)
> foo('a',ARRAY[1,2,3])
> which I understand would be the case with the current patch.
Maybe I misunderstand what is supposed to happen, but I believe that
if the function is marked VARIADIC then the second case would in fact
be rejected: the signature of the function for parameter-matching
purposes is text followed by one or more ints, never text and int[].
> I'm also still curious to know how the following would be handled:
> foo(a text[], b text[])
I think a is just text[], full stop. Only the last parameter is
interpreted differently for variadic.
Your point about the syntax is good though. It would be better if
the syntax were like
create function foo (a text, variadic b int[])
or maybe even better
create function foo (a text, variadic b int)
since (a) this makes it much more obvious to the reader what the
function might match, and (b) it leaves the door open for marking
multiple parameters as variadic, if we can figure out what that means.
regards, tom lane
Tom Lane wrote: > Your point about the syntax is good though. It would be better if > the syntax were like > > create function foo (a text, variadic b int[]) > > or maybe even better > > create function foo (a text, variadic b int) > > since (a) this makes it much more obvious to the reader what the > function might match, and (b) it leaves the door open for marking > multiple parameters as variadic, if we can figure out what that means. > > > Yes. I understand from the family Java expert that (surface syntax issues aside) the second is similar to the way Java does this, in fact, so there's some precedent. That would mean that your first would actually mean each variadic arg has to be an array of ints, which we might well want to provide for. So with that modification I'll be lots happier with the feature. cheers andrew
2008/6/24 Andrew Dunstan <andrew@dunslane.net>: > > > Tom Lane wrote: >> >> Your point about the syntax is good though. It would be better if >> the syntax were like >> >> create function foo (a text, variadic b int[]) >> >> or maybe even better >> >> create function foo (a text, variadic b int) >> >> since (a) this makes it much more obvious to the reader what the >> function might match, and (b) it leaves the door open for marking >> multiple parameters as variadic, if we can figure out what that means. >> >> >> > > Yes. I understand from the family Java expert that (surface syntax issues > aside) the second is similar to the way Java does this, in fact, so there's > some precedent. That would mean that your first would actually mean each > variadic arg has to be an array of ints, which we might well want to provide > for. > > So with that modification I'll be lots happier with the feature. I don't see problem with your syntax. It well block combination OUT and VARIADIC parameter - my one request, variadic parameter have to be array. It's more consistent with following procedure implementation - inside procedures is really array. sample: CREATE OR REPLACE least(varidic values numeric[]) --< ARRAY RETURNS numeric AS $$ SELECT $1[i] --< ARRAY FROM Regards Pavel Stehule p.s. with one exception "any", because there isn't possible array from "any" > > cheers > > andrew >
2008/6/23 Andrew Dunstan <andrew@dunslane.net>: > > > And what about a function that takes 2 arrays as arguments? only last argument is evaluated as variadic so function create or replace function foo(a int[], b int[]) ... variadic is called select foo(array[1,2,3], 1,2,3,4,5,6) > > This proposal strikes me as half-baked. Either we need proper and full > support for variadic functions, or we don't, but I don't think we need > syntactic sugar like the above (or maybe in this case it's really syntactic > saccharine). there is some functions like Oracle's least,greater, decode that needs this feature. So I can write wrappers. For me most important are new possibility for C procedures. All this work is related to my JSON support proposal. Pavel > > cheers > > andrew >
2008/6/24 Tom Lane <tgl@sss.pgh.pa.us>:
> Andrew Dunstan <andrew@dunslane.net> writes:
>> But if I have
>> foo( a text, b int[])
>> it looks odd if both these calls are legal:
>> foo('a',1,2,3,)
>> foo('a',ARRAY[1,2,3])
>> which I understand would be the case with the current patch.
>
> Maybe I misunderstand what is supposed to happen, but I believe that
> if the function is marked VARIADIC then the second case would in fact
> be rejected: the signature of the function for parameter-matching
> purposes is text followed by one or more ints, never text and int[].
>
>> I'm also still curious to know how the following would be handled:
>> foo(a text[], b text[])
>
> I think a is just text[], full stop. Only the last parameter is
> interpreted differently for variadic.
>
> Your point about the syntax is good though. It would be better if
> the syntax were like
>
> create function foo (a text, variadic b int[])
>
> or maybe even better
>
> create function foo (a text, variadic b int)
>
> since (a) this makes it much more obvious to the reader what the
> function might match, and (b) it leaves the door open for marking
> multiple parameters as variadic, if we can figure out what that means.
(b) has one disadvantage - argument type is different than real
parameter - and internally it is little bit cleaner (doesn't need
changes in executor). So there is two forces in opposite. a) clean
function's declaration, b) clean function definition. This syntax is
limited - I am not able implement all cases of Oracle's decode
functions - but I hope it's good compromise between functionality and
simplicity.
note - variant b doesn't block multiple parameters as variadic - is
same case as a. array or not array is unimportant - I need different
types so I can choose what is first variadic argument and what is
second.
Academic question is using structured arrays - some like
create or replace function decode(s_value anyelement1, variadic
(s_value anyalement1, o_value anyelement)[])
returns anyelement as $$
select ($2[i]).o_value
from generate_subcripts($1,1) g(i)
where ($2[i]).s_value = $1;
$$ language sql;
regards
Pavel Stehule
>
> regards, tom lane
>
Hello
this version implements syntax based on argmodes.
CREATE FUNCTION mleast(variadic numeric[]) RETURNS numeric AS $$
SELECT min($1[i])
FROM generate_subscripts($1,1) g(i);
$$ LANGUAGE SQL;
Regards
Pavel Stehule
2008/6/24 Tom Lane <tgl@sss.pgh.pa.us>:
> Andrew Dunstan <andrew@dunslane.net> writes:
>> But if I have
>> foo( a text, b int[])
>> it looks odd if both these calls are legal:
>> foo('a',1,2,3,)
>> foo('a',ARRAY[1,2,3])
>> which I understand would be the case with the current patch.
>
> Maybe I misunderstand what is supposed to happen, but I believe that
> if the function is marked VARIADIC then the second case would in fact
> be rejected: the signature of the function for parameter-matching
> purposes is text followed by one or more ints, never text and int[].
>
>> I'm also still curious to know how the following would be handled:
>> foo(a text[], b text[])
>
> I think a is just text[], full stop. Only the last parameter is
> interpreted differently for variadic.
>
> Your point about the syntax is good though. It would be better if
> the syntax were like
>
> create function foo (a text, variadic b int[])
>
> or maybe even better
>
> create function foo (a text, variadic b int)
>
> since (a) this makes it much more obvious to the reader what the
> function might match, and (b) it leaves the door open for marking
> multiple parameters as variadic, if we can figure out what that means.
>
> regards, tom lane
>
Attachment
"Pavel Stehule" <pavel.stehule@gmail.com> writes:
>> Tom Lane wrote:
>>> Your point about the syntax is good though. It would be better if
>>> the syntax were like
>>> create function foo (a text, variadic b int[])
>>> or maybe even better
>>> create function foo (a text, variadic b int)
> I don't see problem with your syntax. It well block combination OUT
> and VARIADIC parameter - my one request, variadic parameter have to be
> array.
Well, we should certainly store the parameter type as an array in
proargtypes, because that makes this feature transparent to all the
PLs. However, it doesn't follow that the CREATE FUNCTION syntax
has to specify the array type rather than the element type. I think
the Java precedent might be good reason to go with using the element
type in the function declaration.
regards, tom lane
2008/6/25 Tom Lane <tgl@sss.pgh.pa.us>: > "Pavel Stehule" <pavel.stehule@gmail.com> writes: >>> Tom Lane wrote: >>>> Your point about the syntax is good though. It would be better if >>>> the syntax were like >>>> create function foo (a text, variadic b int[]) >>>> or maybe even better >>>> create function foo (a text, variadic b int) > >> I don't see problem with your syntax. It well block combination OUT >> and VARIADIC parameter - my one request, variadic parameter have to be >> array. > > Well, we should certainly store the parameter type as an array in > proargtypes, because that makes this feature transparent to all the > PLs. However, it doesn't follow that the CREATE FUNCTION syntax > has to specify the array type rather than the element type. I think > the Java precedent might be good reason to go with using the element > type in the function declaration. There is only one break - psql functions description. It needs publishing get_element_type function and ofcourse all managers need update. regards Pavel > > regards, tom lane >
2008/6/25 Tom Lane <tgl@sss.pgh.pa.us>: > "Pavel Stehule" <pavel.stehule@gmail.com> writes: >>> Tom Lane wrote: >>>> Your point about the syntax is good though. It would be better if >>>> the syntax were like >>>> create function foo (a text, variadic b int[]) >>>> or maybe even better >>>> create function foo (a text, variadic b int) > >> I don't see problem with your syntax. It well block combination OUT >> and VARIADIC parameter - my one request, variadic parameter have to be >> array. > > Well, we should certainly store the parameter type as an array in > proargtypes, because that makes this feature transparent to all the > PLs. However, it doesn't follow that the CREATE FUNCTION syntax > has to specify the array type rather than the element type. I think > the Java precedent might be good reason to go with using the element > type in the function declaration. > it's strange.I looked for some info http://en.wikipedia.org/wiki/Variadic_function#Variadic_functions_in_C.23_and_Java C# use array Does somebody know some about variadic functions in ADA? Regards Pavel Stehule > regards, tom lane >
2008/6/25 Tom Lane <tgl@sss.pgh.pa.us>: > "Pavel Stehule" <pavel.stehule@gmail.com> writes: >>> Tom Lane wrote: >>>> Your point about the syntax is good though. It would be better if >>>> the syntax were like >>>> create function foo (a text, variadic b int[]) >>>> or maybe even better >>>> create function foo (a text, variadic b int) > >> I don't see problem with your syntax. It well block combination OUT >> and VARIADIC parameter - my one request, variadic parameter have to be >> array. > > Well, we should certainly store the parameter type as an array in > proargtypes, because that makes this feature transparent to all the > PLs. However, it doesn't follow that the CREATE FUNCTION syntax > has to specify the array type rather than the element type. I think > the Java precedent might be good reason to go with using the element > type in the function declaration. I afraid so Java syntax isn't good inspiration http://www.java-tips.org/java-se-tips/java.lang/using-the-varargs-language-feature.html http://www.clanproductions.com/java5.html they use symbol ... like specific synonym to []. public Method getMethod(String name, Class... parameterTypes) I didn't find any info about vararg in Oracle - it uses collection and it allows implicit constructors for emulation o variadic functions - but "variadic" argument isn't scalar too. So I invite any opinions about it. Regards Pavel Stehule > > regards, tom lane >
Pavel Stehule wrote: > I afraid so Java syntax isn't good inspiration > http://www.java-tips.org/java-se-tips/java.lang/using-the-varargs-language-feature.html > http://www.clanproductions.com/java5.html > > they use symbol ... like specific synonym to []. > public Method getMethod(String name, Class... parameterTypes) > > > Well, ... is really more the equivalent of your "variadic" keyword, I think. public Method getMethod(String name, Class[] ... parameterTypes) would mean each variadic argument would be an array of Class. cheers andrew
2008/6/25 Tom Lane <tgl@sss.pgh.pa.us>: > "Pavel Stehule" <pavel.stehule@gmail.com> writes: >>> Tom Lane wrote: >>>> Your point about the syntax is good though. It would be better if >>>> the syntax were like >>>> create function foo (a text, variadic b int[]) >>>> or maybe even better >>>> create function foo (a text, variadic b int) > >> I don't see problem with your syntax. It well block combination OUT >> and VARIADIC parameter - my one request, variadic parameter have to be >> array. > > Well, we should certainly store the parameter type as an array in > proargtypes, because that makes this feature transparent to all the > PLs. However, it doesn't follow that the CREATE FUNCTION syntax > has to specify the array type rather than the element type. I think > the Java precedent might be good reason to go with using the element > type in the function declaration. > I am playing with this now and two versions of proargtypes is 30% more ugly code - mostly pg_dump and paradoxically remove function - because currently RemoveFuncStatement lost argmode, so I am missing info about variadic parameter and I can't simply transformation from element to array. I thing, it isn't good way. Regards Pavel Stehule > regards, tom lane >
2008/6/25 Tom Lane <tgl@sss.pgh.pa.us>: > "Pavel Stehule" <pavel.stehule@gmail.com> writes: >>> Tom Lane wrote: >>>> Your point about the syntax is good though. It would be better if >>>> the syntax were like >>>> create function foo (a text, variadic b int[]) >>>> or maybe even better >>>> create function foo (a text, variadic b int) > >> I don't see problem with your syntax. It well block combination OUT >> and VARIADIC parameter - my one request, variadic parameter have to be >> array. > > Well, we should certainly store the parameter type as an array in > proargtypes, because that makes this feature transparent to all the > PLs. However, it doesn't follow that the CREATE FUNCTION syntax > has to specify the array type rather than the element type. I think > the Java precedent might be good reason to go with using the element > type in the function declaration. > > regards, tom lane > Hello this is third variant with variadic argumen as scalar. But I still strongly prefer second variant with conformance declared variadic array with used array variable. Regards Pavel Stehule
Attachment
On Mon, 2008-06-23 at 15:13 +0200, Pavel Stehule wrote: > this patch enhance current syntax of CREATE FUNCTION statement. It > allows creating functions with variable number of arguments. This > version is different than last my patches. It doesn't need patching > PL. Basic idea is transformation of real arguments (related to > declared variadic argument) to array. All changes are mostly in > parser. Something for the TODO. An added thought with regard to variadic functions: when we have them, we should be able to parse/transform repeated operator sequences into a call to a variadic function, if it exists. e.g. w || x || y || z can be transformed into a call to concat_variadic(w, x, y, z) This can then be made to perform better than concat(concat(concat(w, x), y), z) which would involve lots of wasted memory copies. -- Simon Riggs www.2ndQuadrant.com PostgreSQL Training, Services and Support
2008/7/4 Simon Riggs <simon@2ndquadrant.com>: > > On Mon, 2008-06-23 at 15:13 +0200, Pavel Stehule wrote: > >> this patch enhance current syntax of CREATE FUNCTION statement. It >> allows creating functions with variable number of arguments. This >> version is different than last my patches. It doesn't need patching >> PL. Basic idea is transformation of real arguments (related to >> declared variadic argument) to array. All changes are mostly in >> parser. > > Something for the TODO. > > An added thought with regard to variadic functions: > > when we have them, we should be able to parse/transform repeated > operator sequences into a call to a variadic function, if it exists. > > e.g. w || x || y || z can be transformed into a call to > concat_variadic(w, x, y, z) > > This can then be made to perform better than > > concat(concat(concat(w, x), y), z) > > which would involve lots of wasted memory copies. good idea Regards Pavel Stehule > > -- > Simon Riggs www.2ndQuadrant.com > PostgreSQL Training, Services and Support > >
On Thu, 2008-06-26 at 17:03 +0200, Pavel Stehule wrote:
> this is third variant with variadic argumen as scalar. But I still
> strongly prefer second variant with conformance declared variadic
> array with used array variable.
This version allows you to declare two functions "foo(variadic numeric)"
and "foo(numeric)", and then if you do a "\df foo" the backend crashes.
Also, you didn't update an error string:
"variadic argument isn't an array" should (in this version) be something
like: "can't find array type for variadic parameter type %s"
I suggest a slightly different wording for the following error messages:
"aggregate function has variadic argument" -> "variadic parameters not
supported for aggregate functions"
and
"variadic argument isn't last function's argument" -> "variadic
parameter must be the last parameter to the function"
Those are just suggested wordings.
Regards,
Jeff Davis
Hello 2008/7/13 Jeff Davis <pgsql@j-davis.com>: > On Thu, 2008-06-26 at 17:03 +0200, Pavel Stehule wrote: >> this is third variant with variadic argumen as scalar. But I still >> strongly prefer second variant with conformance declared variadic >> array with used array variable. > you checked second or third variant? There are two variants still. Regards Pavel Stehule Please, Tom, can you choose one? > This version allows you to declare two functions "foo(variadic numeric)" > and "foo(numeric)", and then if you do a "\df foo" the backend crashes. > > Also, you didn't update an error string: > > "variadic argument isn't an array" should (in this version) be something > like: "can't find array type for variadic parameter type %s" > > I suggest a slightly different wording for the following error messages: > > "aggregate function has variadic argument" -> "variadic parameters not > supported for aggregate functions" > > and > > "variadic argument isn't last function's argument" -> "variadic > parameter must be the last parameter to the function" > > Those are just suggested wordings. > > Regards, > Jeff Davis > >
On Sun, 2008-07-13 at 07:52 +0200, Pavel Stehule wrote:
> you checked second or third variant? There are two variants still.
Sorry for being unclear. Those comments regarded patch v2.2.1. The bug
is in pg_function_is_visible().
Additionally, I'd like to see support for calling variadic functions
with no arguments. I mentioned that in my other email, but it applies to
v2.2.1 as well.
And we should come to a consensus quickly about the declaration style,
e.g. "variadic int[]" or "variadic int".
Regards,
Jeff Davis
On Sat, 2008-07-12 at 23:06 -0700, Jeff Davis wrote:
> I don't have a strong opinion about whether the variadic argument is
> declared as an array or scalar, so I am posting my comments about this
> version of the patch as well.
>
> This version also has a problem when declaring two functions "foo(int)"
> and "foo(variadic int[])". In this version, the declaration is allowed
> but the backend crashes when the function is called.
>
> The variable "transform_variadic" should have some kind of comment. It
> seems to be there to distinguish between when you're looking for a
> candidate function for a function call, and when you're looking for a
> candidate function for, e.g., CREATE FUNCTION. It's a little hard to
> follow, and is probably the cause for the aformentioned crash.
>
> Also, it doesn't seem to allow calls to a variadic function with zero
> arguments, e.g. "mleast()". I think this should be allowed.
>
> I suggest the following error message rewording:
> "variadic argument isn't an array" should be something like: "variadic
> argument must be an array".
>
To be clear, these comments apply to v2.0.0 of the patch.
Regards,
Jeff Davis
On Tue, 2008-06-24 at 17:10 +0200, Pavel Stehule wrote:
> Hello
>
> this version implements syntax based on argmodes.
>
>
> CREATE FUNCTION mleast(variadic numeric[]) RETURNS numeric AS $$
> SELECT min($1[i])
> FROM generate_subscripts($1,1) g(i);
> $$ LANGUAGE SQL;
>
I don't have a strong opinion about whether the variadic argument is
declared as an array or scalar, so I am posting my comments about this
version of the patch as well.
This version also has a problem when declaring two functions "foo(int)"
and "foo(variadic int[])". In this version, the declaration is allowed
but the backend crashes when the function is called.
The variable "transform_variadic" should have some kind of comment. It
seems to be there to distinguish between when you're looking for a
candidate function for a function call, and when you're looking for a
candidate function for, e.g., CREATE FUNCTION. It's a little hard to
follow, and is probably the cause for the aformentioned crash.
Also, it doesn't seem to allow calls to a variadic function with zero
arguments, e.g. "mleast()". I think this should be allowed.
I suggest the following error message rewording:
"variadic argument isn't an array" should be something like: "variadic
argument must be an array".
Regards,
Jeff Davis
2008/7/13 Jeff Davis <pgsql@j-davis.com>: > On Tue, 2008-06-24 at 17:10 +0200, Pavel Stehule wrote: >> Hello >> >> this version implements syntax based on argmodes. >> >> >> CREATE FUNCTION mleast(variadic numeric[]) RETURNS numeric AS $$ >> SELECT min($1[i]) >> FROM generate_subscripts($1,1) g(i); >> $$ LANGUAGE SQL; >> > > I don't have a strong opinion about whether the variadic argument is > declared as an array or scalar, so I am posting my comments about this > version of the patch as well. ok > > This version also has a problem when declaring two functions "foo(int)" > and "foo(variadic int[])". In this version, the declaration is allowed > but the backend crashes when the function is called. > ok, I understand now > The variable "transform_variadic" should have some kind of comment. It > seems to be there to distinguish between when you're looking for a > candidate function for a function call, and when you're looking for a > candidate function for, e.g., CREATE FUNCTION. It's a little hard to > follow, and is probably the cause for the aformentioned crash. > > Also, it doesn't seem to allow calls to a variadic function with zero > arguments, e.g. "mleast()". I think this should be allowed. > It's not possible for all cases, because empty array have be typed array still. But for non polymorphic variadic functions it's probably possible - I would to solve this question later - and for now use overloading etc create function mleast() returns .. create function mleast(variadic params anyarray) returns ... > I suggest the following error message rewording: > "variadic argument isn't an array" should be something like: "variadic > argument must be an array". > I invite all you language suggestions. It's really important for me. > Regards, > Jeff Davis > my thanks Pavel Stehule >
2008/7/13 Jeff Davis <pgsql@j-davis.com>: > On Sun, 2008-07-13 at 07:52 +0200, Pavel Stehule wrote: >> you checked second or third variant? There are two variants still. > > Sorry for being unclear. Those comments regarded patch v2.2.1. The bug > is in pg_function_is_visible(). it's bug, I'll fix it > > Additionally, I'd like to see support for calling variadic functions > with no arguments. I mentioned that in my other email, but it applies to > v2.2.1 as well. for some cases, I can do less restriction. > > And we should come to a consensus quickly about the declaration style, > e.g. "variadic int[]" or "variadic int". ok > > Regards, > Jeff Davis > > thank you very much Pavel Stehule
"Pavel Stehule" <pavel.stehule@gmail.com> writes:
> 2008/7/13 Jeff Davis <pgsql@j-davis.com>:
>> Also, it doesn't seem to allow calls to a variadic function with zero
>> arguments, e.g. "mleast()". I think this should be allowed.
> It's not possible for all cases, because empty array have be typed
> array still. But for non polymorphic variadic functions it's probably
> possible - I would to solve this question later - and for now use
> overloading etc
I don't really like the idea of a feature that would work except in the
polymorphic case --- that just seems too non-orthogonal. Also I tend
to think that a pretty large fraction of variadic functions will be
polymorphic, making the feature's usefulness even more dubious.
I concur with the idea that variadic functions should only match to
calls that offer at least one value for the variadic array. If you can
actually define the behavior sensibly for the zero-element case, a
separate function of the same name can cover that case.
As far as the "variadic int" versus "variadic int[]" business, I'm
starting to agree with Pavel that "variadic int[]" offers less potential
for confusion. In particular, it seems to make it more obvious for the
function author that the argument he receives is an array. Also, the
other one would mean that what we put into pg_proc.proargtypes doesn't
agree directly with what the user thinks the argument types are. While
I think we could force that to work, it's not exactly satisfying the
principle of least surprise.
One issue that just occurred to me: what if a variadic function wants to
turn around and call another variadic function, passing the same array
argument on to the second one? This is closely akin to the problem
faced by C "..." functions, and the solutions are pretty ugly (sprintf
vs vsprintf for instance). Can we do any better? At least in the
polymorphic case, I'm not sure we can :-(.
regards, tom lane
> > As far as the "variadic int" versus "variadic int[]" business, I'm > starting to agree with Pavel that "variadic int[]" offers less potential > for confusion. In particular, it seems to make it more obvious for the > function author that the argument he receives is an array. Also, the > other one would mean that what we put into pg_proc.proargtypes doesn't > agree directly with what the user thinks the argument types are. While > I think we could force that to work, it's not exactly satisfying the > principle of least surprise. > > > One issue that just occurred to me: what if a variadic function wants to > turn around and call another variadic function, passing the same array > argument on to the second one? This is closely akin to the problem > faced by C "..." functions, and the solutions are pretty ugly (sprintf > vs vsprintf for instance). Can we do any better? At least in the > polymorphic case, I'm not sure we can :-(. > > regards, tom lane > maybe with some flag like PARAMS? SELECT least(PARAMS ARRAY[1,2,3,4,5,6]) Regards Pavel Stehule
2008/7/13 Tom Lane <tgl@sss.pgh.pa.us>: > "Pavel Stehule" <pavel.stehule@gmail.com> writes: >> 2008/7/13 Jeff Davis <pgsql@j-davis.com>: >>> Also, it doesn't seem to allow calls to a variadic function with zero >>> arguments, e.g. "mleast()". I think this should be allowed. > >> It's not possible for all cases, because empty array have be typed >> array still. But for non polymorphic variadic functions it's probably >> possible - I would to solve this question later - and for now use >> overloading etc > > I don't really like the idea of a feature that would work except in the > polymorphic case --- that just seems too non-orthogonal. Also I tend > to think that a pretty large fraction of variadic functions will be > polymorphic, making the feature's usefulness even more dubious. > > I concur with the idea that variadic functions should only match to > calls that offer at least one value for the variadic array. If you can > actually define the behavior sensibly for the zero-element case, a > separate function of the same name can cover that case. > > As far as the "variadic int" versus "variadic int[]" business, I'm > starting to agree with Pavel that "variadic int[]" offers less potential > for confusion. In particular, it seems to make it more obvious for the > function author that the argument he receives is an array. Also, the > other one would mean that what we put into pg_proc.proargtypes doesn't > agree directly with what the user thinks the argument types are. While > I think we could force that to work, it's not exactly satisfying the > principle of least surprise. > > > One issue that just occurred to me: what if a variadic function wants to > turn around and call another variadic function, passing the same array > argument on to the second one? This is closely akin to the problem > faced by C "..." functions, and the solutions are pretty ugly (sprintf > vs vsprintf for instance). Can we do any better? At least in the > polymorphic case, I'm not sure we can :-(. > fixed version Thanks for comments Pavel
Attachment
Pavel Stehule wrote:
>> One issue that just occurred to me: what if a variadic function
>> wants to turn around and call another variadic function, passing
>> the same array argument on to the second one? This is closely akin
>> to the problem faced by C "..." functions, and the solutions are
>> pretty ugly (sprintf vs vsprintf for instance). Can we do any
>> better? At least in the polymorphic case, I'm not sure we can :-(.
>> maybe with some flag like PARAMS?
>
> SELECT least(PARAMS ARRAY[1,2,3,4,5,6])
Just FYI, this is more or less how ruby handles variadic functions - a
"*" before the last argument in the function's *definition* causes all
additional arguments to be stored in an array, while a "*" before the
last argument in a function *call* expands an array into single arguments.
So, you could e.g do
def variadic1(a, b, *c)
# c is in array containing all parameters after second one.
end
def variadic_wrapper(a, *b)
variadic1("foobar", a, *b)
end
So there is precedent for the "flag idea" too. Plus, I kind of like the
idea of using the same syntax for both wrapping and unwrapping of
variadic arguments.
regards, Florian Pflug
2008/7/14 Florian G. Pflug <fgp@phlo.org>:
> Pavel Stehule wrote:
>>>
>>> One issue that just occurred to me: what if a variadic function wants to
>>> turn around and call another variadic function, passing the same array
>>> argument on to the second one? This is closely akin
>>> to the problem faced by C "..." functions, and the solutions are pretty
>>> ugly (sprintf vs vsprintf for instance). Can we do any better? At least in
>>> the polymorphic case, I'm not sure we can :-(.
>>> maybe with some flag like PARAMS?
>>
>> SELECT least(PARAMS ARRAY[1,2,3,4,5,6])
>
> Just FYI, this is more or less how ruby handles variadic functions - a
> "*" before the last argument in the function's *definition* causes all
> additional arguments to be stored in an array, while a "*" before the
> last argument in a function *call* expands an array into single arguments.
>
> So, you could e.g do
> def variadic1(a, b, *c)
> # c is in array containing all parameters after second one.
> end
>
> def variadic_wrapper(a, *b)
> variadic1("foobar", a, *b)
> end
>
> So there is precedent for the "flag idea" too. Plus, I kind of like the
> idea of using the same syntax for both wrapping and unwrapping of variadic
> arguments.
>
> regards, Florian Pflug
>
ok - it's possible, I''l look in this direction - and it's should be
usable in plpgsql - we should be able call variadic functions from
plpgsql with immutable number of arguments without dynamic SQL.
sample: select mleast(variadic array[1,2,3,4,5]);
so I wouldn't do ruby from plpgsql :). Still my goal is well support
for libraries like JSON or XML.
select json_object(name as 'name', prop as 'prop') --> '[name: xxxx,
prop: yyyy ...
It's not strong like SQL/XML, but it is independent on parser, and
could exists outside. So my next step is named parameters in SELECT
statement.
Regards
Pavel Stehule
>
"Pavel Stehule" <pavel.stehule@gmail.com> writes:
> sample: select mleast(variadic array[1,2,3,4,5]);
Note this would also address Jeff's complaint about not being able to
pass zero variadic parameters:
select mleast(variadic array[]::int[]);
Looks a bit ugly but the type is specified, so it would work correctly
with polymorphic functions.
Are you intending to change this right now and resubmit, or is it
work for later?
regards, tom lane
On Sun, 2008-07-13 at 12:32 -0400, Tom Lane wrote:
> >> Also, it doesn't seem to allow calls to a variadic function with zero
> >> arguments, e.g. "mleast()". I think this should be allowed.
>
> > It's not possible for all cases, because empty array have be typed
> > array still. But for non polymorphic variadic functions it's probably
> > possible - I would to solve this question later - and for now use
> > overloading etc
>
> I don't really like the idea of a feature that would work except in the
> polymorphic case --- that just seems too non-orthogonal. Also I tend
> to think that a pretty large fraction of variadic functions will be
> polymorphic, making the feature's usefulness even more dubious.
I think it has the potential for surprise both ways. I was surprised
when it didn't allow a zero-argument call.
> I concur with the idea that variadic functions should only match to
> calls that offer at least one value for the variadic array. If you can
> actually define the behavior sensibly for the zero-element case, a
> separate function of the same name can cover that case.
Similarly, if zero-argument calls to variadic functions were allowed,
and you want the polymorphism to work as you suggest, you can just
define:
foo(int, variadic int[])
foo(text, variadic text[])
and that forces one argument to be provided, and the functions don't
conflict. If this is the common case, I can see why you wouldn't want to
require declaration of the extra parameter each time.
I don't have a strong opinion, but allowing zero-argument variadic
function calls -- and therefore causing foo(variadic int[]) and
foo(variadic text[]) to conflict -- makes more sense than requiring one
argument. It also seems a little more natural from a function author's
perspective.
Regards,
Jeff Davis
Jeff Davis <pgsql@j-davis.com> writes:
> I don't have a strong opinion, but allowing zero-argument variadic
> function calls -- and therefore causing foo(variadic int[]) and
> foo(variadic text[]) to conflict -- makes more sense than requiring one
> argument.
I hadn't even thought about that point, but the idea that those two
would conflict bothers me quite a lot. Not least because there's no
reasonable way to enforce it with the existing unique indexes on pg_proc.
I think you'd have to leave the variadic argument out of proargtypes
altogether, and that seems mad.
regards, tom lane
2008/7/14 Tom Lane <tgl@sss.pgh.pa.us>: > "Pavel Stehule" <pavel.stehule@gmail.com> writes: >> sample: select mleast(variadic array[1,2,3,4,5]); > > Note this would also address Jeff's complaint about not being able to > pass zero variadic parameters: > > select mleast(variadic array[]::int[]); > > Looks a bit ugly but the type is specified, so it would work correctly > with polymorphic functions. > > Are you intending to change this right now and resubmit, or is it > work for later? I prefer separate it to other patch. regards Pavel Stehule > > regards, tom lane >
"Pavel Stehule" <pavel.stehule@gmail.com> writes:
> 2008/7/14 Tom Lane <tgl@sss.pgh.pa.us>:
>> Are you intending to change this right now and resubmit, or is it
>> work for later?
> I prefer separate it to other patch.
It seems a fairly important part of the feature, especially given the
connection to the zero-argument issue.
regards, tom lane
2008/7/14 Tom Lane <tgl@sss.pgh.pa.us>: > "Pavel Stehule" <pavel.stehule@gmail.com> writes: >> 2008/7/14 Tom Lane <tgl@sss.pgh.pa.us>: >>> Are you intending to change this right now and resubmit, or is it >>> work for later? > >> I prefer separate it to other patch. > > It seems a fairly important part of the feature, especially given the > connection to the zero-argument issue. ok tomorrow I have some work, but I can do it to Friday. regards Pavel Stehule > > regards, tom lane >
"Pavel Stehule" <pavel.stehule@gmail.com> writes:
> sample: select mleast(variadic array[1,2,3,4,5]);
One other point here: in the patch as presently submitted, VARIADIC
can't be an unreserved keyword, but it seems to be enough to make it a
col_name_keyword. Some preliminary testing says that that doesn't work
anymore after adding productions to allow the above: VARIADIC will have
to become a fully reserved keyword. That's a bit annoying, since it's
not reserved according to the SQL spec, but it doesn't seem like a word
that's really likely to be in use as a user identifier. Does anyone
think this issue is a showstopper?
regards, tom lane
Hello
this version is WIP - I have to clean comments, and will do some
documentation. But I am sure, I am not able explain this feature in
english well. Please, can some body help me with documentation? So
now, plpgsql is more/less ruby :)
postgres=# select myleast(variadic array[1,2,3,4,-1]);
myleast
---------
-1
(1 row)
postgres=# select myleast(variadic array[1.1, -5.5]);
myleast
---------
-5.5
(1 row)
postgres=# --test with empty variadic call parameter
postgres=# select myleast(variadic array[]::int[]);
myleast
---------
(1 row)
postgres=# select myleast(1.1,-5.5);
myleast
---------
-5.5
regards
Pavel
2008/7/14 Pavel Stehule <pavel.stehule@gmail.com>:
> 2008/7/14 Tom Lane <tgl@sss.pgh.pa.us>:
>> "Pavel Stehule" <pavel.stehule@gmail.com> writes:
>>> 2008/7/14 Tom Lane <tgl@sss.pgh.pa.us>:
>>>> Are you intending to change this right now and resubmit, or is it
>>>> work for later?
>>
>>> I prefer separate it to other patch.
>>
>> It seems a fairly important part of the feature, especially given the
>> connection to the zero-argument issue.
>
> ok tomorrow I have some work, but I can do it to Friday.
>
> regards
> Pavel Stehule
>>
>> regards, tom lane
>>
>
Attachment
"Pavel Stehule" <pavel.stehule@gmail.com> writes:
> this version is WIP - I have to clean comments, and will do some
> documentation. But I am sure, I am not able explain this feature in
> english well. Please, can some body help me with documentation? So
> now, plpgsql is more/less ruby :)
Applied with revisions. The duplicate-argument detection logic in
FuncnameGetCandidates was pretty thoroughly broken, and there were some
internal API decisions I didn't like, as well as a few sins of omission
like not making ruleutils.c work. I did some work on the docs but there
may be a few other places that could stand to mention variadic
functions.
I didn't do anything about the extra pg_proc column, but will start to
work on that now.
regards, tom lane