Re: proposal: fix corner use case of variadic fuctions usage

From: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Stephen Frost <sfrost(at)snowman(dot)net>, Vik Reykja <vikreykja(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: proposal: fix corner use case of variadic fuctions usage
Date: 2013-01-22 08:44:47
Message-ID: CAFj8pRAbkNwJ-KBgeq8j2EWvT_PBYqBn9qiP2UM7KLctgCj2pA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-general pgsql-hackers

2013/1/22 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:
> Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> writes:
>> so here is rewritten patch
>
> I've applied the infrastructure parts of this, but not the changes
> to format() and concat().
>
> Why are the format and concat patches so randomly different?
> Not only is the internal implementation completely different for no
> obvious reason, but the user-visible behavior is inconsistent too.
> You've got one of them iterating over elements and one over slices.
> That seems pretty bogus. Personally I'd vote for the element-level
> behavior in both cases, because that's generally what happens in other
> PG functions when there's no particular reason to pay attention to the
> structure of a multidimensional array. I certainly don't see a reason
> why they should be making opposite choices.

some months ago, there was a one argument against this patch (or this
feature) impossibility to pass array as one argument into variadic
function. I am thinking so natural reply is a slicing.

Without slicing you cannot to pass array as a argument - so it is
relative hard limit. But if I thinking about it - not too much people
use it with multidimensional array, so I prefer slicing as natural
behave, but I can live without it.

What do you thinking about limit to just only one dimensional arrays?
- then we don't need solve this question now. This behave is important
for format() -- and maybe it is premature optimization - but I don't
to close these doors too early.

>
> Also, I'm not sure that it's appropriate to throw an error if the given
> argument is null or not an array. Previous versions did not throw an
> error in such cases. Perhaps just fall back to behaving as if it
> weren't marked VARIADIC? You could possibly make an argument for
> not-an-array-type being an error, since that's a statically inconsistent
> type situation, but I really don't like a null value being an error.
> A function could easily receive a null value for an array parameter
> that it wants to pass on to format() or concat().

I did test with custom variadic function

CREATE OR REPLACE FUNCTION public.ileast(VARIADIC integer[])
RETURNS integer
LANGUAGE sql
AS $function$
select max(v) from unnest($1) g(v)
$function$

postgres=# select ileast(10,20);
ileast
--------
20
(1 row)

postgres=# select ileast(variadic null);
ileast
--------

(1 row)

postgres=# select ileast(variadic 10);
ERROR: function ileast(integer) does not exist
LINE 1: select ileast(variadic 10);
^
HINT: No function matches the given name and argument types. You
might need to add explicit type casts.

so NULL should be supported, and ARRAY should be requested.

question is about level - WARNING or ERROR - in long term perspective
I am sure so ERROR is correct.

If we raise only WARNING - then what we should to do? Ignore VARIADIC
label like before or return correct value? Then I don't see a sense
for WARNING - this is possible incompatibility :( - but better fix it
early than later

Other opinions?

>
> BTW, using array_iterate as a substitute for deconstruct_array is
> neither efficient nor good style. array_iterate is for processing the
> values as you scan the array.
>

yes - some deconstruct_sliced_array will be better and cleaner.
Depends how we decide first question about using multidimensional
arrays. Probably would not be necessary.

Regards

Pavel

> regards, tom lane

In response to

Browse pgsql-general by date

  From Date Subject
Next Message Johann Spies 2013-01-22 09:18:14 Re: Question on Trigram GIST indexes
Previous Message Nathan Clayton 2013-01-22 07:11:20 Re: Yet Another Timestamp Question: Time Defaults

Browse pgsql-hackers by date

  From Date Subject
Next Message Dimitri Fontaine 2013-01-22 09:35:51 Re: CF3+4 (was Re: Parallel query execution)
Previous Message Magnus Hagander 2013-01-22 07:29:29 Re: CF3+4 (was Re: Parallel query execution)