bugfix - VIP: variadic function ignore strict flag

Pavel Stehule <pavel.stehule@gmail.com> writes:

+ 		/*
+ 		 * If function has variadic argument, skip calling
+ 		 * when variadic array contains NULL values.
+ 		 */

I don't think this is right at all. The strict check ought to apply to
the array argument as a whole.

regards, tom lane

2010/2/9 Tom Lane <tgl@sss.pgh.pa.us>:

Pavel Stehule <pavel.stehule@gmail.com> writes:

+             /*
+              * If function has variadic argument, skip calling
+              * when variadic array contains NULL values.
+              */

I don't think this is right at all.  The strict check ought to apply to
the array argument as a whole.

yes, this isn't clear. My arguments for change:

a) the behave depends on types - "any" is different than others.

b) optimization over fmgr doesn't work now.

b1. some possible const null and strict are ignored
b2. array is non const always - so pre eval doesn't work for variadic

c) it could be confusing, and it is partially confusing.

point c could be solved by notice in documentation. But a and b are
problem. The variadic funcall cannot be optimized :(

Regards
Pavel Stehule

Pavel Stehule <pavel.stehule@gmail.com> writes:

2010/2/9 Tom Lane <tgl@sss.pgh.pa.us>:

I don't think this is right at all.

yes, this isn't clear. My arguments for change:

a) the behave depends on types - "any" is different than others.

So what? "variadic any" is different in a lot of ways.

b) optimization over fmgr doesn't work now.
b1. some possible const null and strict are ignored

That's a matter of definition.

b2. array is non const always - so pre eval doesn't work for variadic

You'd need to explain what you mean by that. An ARRAY[] construct is
subject to const-folding AFAICS.

regards, tom lane

2010/2/9 Tom Lane <tgl@sss.pgh.pa.us>:

Pavel Stehule <pavel.stehule@gmail.com> writes:

2010/2/9 Tom Lane <tgl@sss.pgh.pa.us>:

I don't think this is right at all.

yes, this isn't clear. My arguments for change:

a) the behave depends on types - "any" is different than others.

So what?  "variadic any" is different in a lot of ways.

implementation is different, but from users perspective there can not
be differences. I am not sure. From my programmer's view is all ok.
But I believe so from customer view, there can be a surprise - because
NULL value doesn't skip function call.

b) optimization over fmgr doesn't work now.
b1. some possible const null and strict are ignored

That's a matter of definition.

b2. array is non const always - so pre eval doesn't work for variadic

You'd need to explain what you mean by that.  An ARRAY[] construct is
subject to const-folding AFAICS.

I am sorry. I was confused. This optimization will work. Only NULL is problem.

Regards
Pavel

Pavel Stehule <pavel.stehule@gmail.com> writes:

2010/2/9 Tom Lane <tgl@sss.pgh.pa.us>:

So what?  "variadic any" is different in a lot of ways.

implementation is different, but from users perspective there can not
be differences. I am not sure. From my programmer's view is all ok.
But I believe so from customer view, there can be a surprise - because
NULL value doesn't skip function call.

It's going to be a bit surprising in any case. If I write

foo(1, VARIADIC ARRAY[2, NULL])

then what I'm passing is not a null, and so I'd be surprised if the
function wasn't executed.

I think we should just document this, not make a definitional change
that seems as likely to break applications as fix them.

regards, tom lane

2010/2/9 Tom Lane <tgl@sss.pgh.pa.us>:

Pavel Stehule <pavel.stehule@gmail.com> writes:

2010/2/9 Tom Lane <tgl@sss.pgh.pa.us>:

So what?  "variadic any" is different in a lot of ways.

implementation is different, but from users perspective there can not
be differences. I am not sure. From my programmer's view is all ok.
But I believe so from customer view, there can be a surprise - because
NULL value doesn't skip function call.

It's going to be a bit surprising in any case.  If I write

       foo(1, VARIADIC ARRAY[2, NULL])

then what I'm passing is not a null, and so I'd be surprised if the
function wasn't executed.

I think we should just document this, not make a definitional change
that seems as likely to break applications as fix them.

really I am not sure, what is good solution. Maybe can speak some other.

Pavel