Thread

1. bugfix - VIP: variadic function ignore strict flag

   Pavel Stehule <pavel.stehule@gmail.com> — 2010-02-09T15:45:22Z

   Hello,
   
   I am returning back to message
   http://archives.postgresql.org/pgsql-general/2010-02/msg00119.php
   
   Our implementation of variadic parameters missing correct handling
   test on NULL, and test on non constant value.
   
   This patch add correct tests for variadic parameters. Probably
   Gen_fmgrtab.pl have needs some work - there are magic constants for
   InvalidOid and ANYOID.
   
   Regards
   
   Pavel Stehule
   

2. Re: bugfix - VIP: variadic function ignore strict flag

   Tom Lane <tgl@sss.pgh.pa.us> — 2010-02-09T16:14:06Z

   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
   
   

3. Re: bugfix - VIP: variadic function ignore strict flag

   Pavel Stehule <pavel.stehule@gmail.com> — 2010-02-09T16:46:27Z

   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
   
   
   >
   >                        regards, tom lane
   >
   
   

4. Re: bugfix - VIP: variadic function ignore strict flag

   Tom Lane <tgl@sss.pgh.pa.us> — 2010-02-09T17:35:06Z

   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
   
   

5. Re: bugfix - VIP: variadic function ignore strict flag

   Pavel Stehule <pavel.stehule@gmail.com> — 2010-02-09T17:44:42Z

   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
   
   
   >
   >                        regards, tom lane
   >
   
   

6. Re: bugfix - VIP: variadic function ignore strict flag

   Tom Lane <tgl@sss.pgh.pa.us> — 2010-02-09T17:56:52Z

   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
   
   

7. Re: bugfix - VIP: variadic function ignore strict flag

   Pavel Stehule <pavel.stehule@gmail.com> — 2010-02-09T18:08:49Z

   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
   
   >
   >                        regards, tom lane
   >