Re: patch: to_string, to_array functions

On 6 May 2010 04:42, Pavel Stehule <pavel.stehule@gmail.com> wrote:
> attached patch contains to_string and to_array functions. These
> functions are equivalent of array_to_string and string_to_array
> function with maybe more correct NULL handling.

Hi Pavel,

I am reviewing your patch for the commitfest.

Overall the patch looks good, although there were some bogus
whitespace changes in the patch and some messy punctuation/grammar in
some of the code comments.  I also thought it was worth mentioning in
the docs the default value for null_string is ''.  I made an attempt
to clean those items up and have attached a v2 of the patch.

Regarding the behaviour of the third argument (null_string), I was a
little surprised by the results when I passed in a NULL.

postgres=# select to_string(array['a', 'b', 'c', 'd'], '/', NULL);
 to_string
-----------

Now, if the array had some NULL elements in it, I could understand why
the resulting string would be NULL (because str || NULL is NULL), but
in this case there are no NULLs.  Why is the result NULL?  Surely it
should be 'a/b/c/d' regardless of how the third parameter is set?

In the reverse case:

postgres=# select to_array('a/b/c/d', '/', NULL);
 to_array
----------

(1 row)

Again I find this a bit weird.  I have left the null_string NULL,
which means it is unknown.  It can't possibly match any value in the
string, so effectively passing in a NULL null_string should mean that
the user doesn't want any string items whatsoever to translate into
NULLs in the resulting array.  I would expect this call to return
{a,b,c,d}.

Cheers,
BJ