Prev: multibyte charater set in levenshtein function
Next: (9.1) btree_gist support for searching on "not equals"
From: Pavel Stehule on 16 Jul 2010 12:15
Hello

2010/7/16 Brendan Jurd <direvus(a)gmail.com>:
> On 6 May 2010 04:42, Pavel Stehule <pavel.stehule(a)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)
>

I didn't thinking about NULL as separator before. Current behave isn't
practical. When default separator is empty string, then NULL can be
used as ignore NULLs - so it can emulate current string_to_array and
array_to_string behave. It can be, because NULL can't be a separator
ever.

select to_string(array[1,2,3,null,5], ',') -> 1,2,3,,5
select to_string(array[1,2,3,null,5], ',', null) -> 1,2,3,5

maybe - next idea and maybe better - we can check NOT NULL for
separator and to add other parameter with default = false -
ignore_null

select to_string(array[1,2,3,null,5], ',', ignore_null := true) -> 1,2,3,5

what do you think?

Regards

Pavel

> 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
>

--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: Brendan Jurd on 16 Jul 2010 12:47
On 17 July 2010 02:15, Pavel Stehule <pavel.stehule(a)gmail.com> wrote:
> 2010/7/16 Brendan Jurd <direvus(a)gmail.com>:
>> Regarding the behaviour of the third argument (null_string), I was a
>> little surprised by the results when I passed in a NULL.
>>
>
> I didn't thinking about NULL as separator before. Current behave isn't
> practical. When default separator is empty string, then NULL can be
> used as ignore NULLs - so it can emulate current string_to_array and
> array_to_string behave. It can be, because NULL can't be a separator
> ever.
>
> select to_string(array[1,2,3,null,5], ',') -> 1,2,3,,5
> select to_string(array[1,2,3,null,5], ',', null) -> 1,2,3,5
>
> maybe - next idea and maybe better - we can check NOT NULL for
> separator and to add other parameter with default = false -
> ignore_null
>
> select to_string(array[1,2,3,null,5], ',', ignore_null := true) -> 1,2,3,5
>
> what do you think?

I don't have any problem with null_string = NULL in to_string taking
the meaning "skip over NULL elements". It's a slightly strange
outcome but it's more useful than returning NULL, and I do like that
it gives us a path to the current array_to_string() treatment even if
those functions are ultimately deprecated. I think adding a fourth
keyword argument might be sacrificing a little too much convenience in
the calling convention.

As for to_array, null_string = NULL should mean that there is no
string which should result in a NULL element. So I would be happy to
see the following set of behaviours:

to_string(array[1, 2, 3, 4, 5], ',', null) = '1,2,3,4,5'
to_string(array[1, 2, 3, null, 5], ',', null) = '1,2,3,5'
to_array('1,2,3,,5', ',', null) = '{1,2,3,"",5}'

Also, if we're going to make the function non-strict, we need to
consider how to respond when the user specifies NULL for the other
arguments. If the field separator is NULL, bearing in mind that NULL
can't match any string, I would expect that to_array would return the
undivided string as a single array element, and that to_string would
throw an error:

to_array('1,2,3,4,5', null) = '{"1,2,3,4,5"}'
to_string(array[1,2,3,4,5], null) = ERROR: the field separator for
to_string may not be NULL

If the first argument is NULL for either function, I think it would be
reasonable to return NULL. But I could be convinced that we should
throw an error in that case too.

Cheers,
BJ

--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: Brendan Jurd on 16 Jul 2010 10:25
On 6 May 2010 04:42, Pavel Stehule <pavel.stehule(a)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
From: Pavel Stehule on 16 Jul 2010 14:52
2010/7/16 Brendan Jurd <direvus(a)gmail.com>:
> On 17 July 2010 02:15, Pavel Stehule <pavel.stehule(a)gmail.com> wrote:
>> 2010/7/16 Brendan Jurd <direvus(a)gmail.com>:
>>> Regarding the behaviour of the third argument (null_string), I was a
>>> little surprised by the results when I passed in a NULL.
>>>
>>
>> I didn't thinking about NULL as separator before. Current behave isn't
>> practical. When default separator is empty string, then NULL can be
>> used as ignore NULLs - so it can emulate current string_to_array and
>> array_to_string behave. It can be, because NULL can't be a separator
>> ever.
>>
>> select to_string(array[1,2,3,null,5], ',') -> 1,2,3,,5
>> select to_string(array[1,2,3,null,5], ',', null) -> 1,2,3,5
>>
>> maybe - next idea and maybe better - we can check NOT NULL for
>> separator and to add other parameter with default = false -
>> ignore_null
>>
>> select to_string(array[1,2,3,null,5], ',', ignore_null := true) -> 1,2,3,5
>>
>> what do you think?
>
> I don't have any problem with null_string = NULL in to_string taking
> the meaning "skip over NULL elements".  It's a slightly strange
> outcome but it's more useful than returning NULL, and I do like that
> it gives us a path to the current array_to_string() treatment even if
> those functions are ultimately deprecated.  I think adding a fourth
> keyword argument might be sacrificing a little too much convenience in
> the calling convention.
>
> As for to_array, null_string = NULL should mean that there is no
> string which should result in a NULL element.  So I would be happy to
> see the following set of behaviours:
>
> to_string(array[1, 2, 3, 4, 5], ',', null) = '1,2,3,4,5'
> to_string(array[1, 2, 3, null, 5], ',', null) = '1,2,3,5'
> to_array('1,2,3,,5', ',', null) = '{1,2,3,"",5}'
>
> Also, if we're going to make the function non-strict, we need to
> consider how to respond when the user specifies NULL for the other
> arguments.  If the field separator is NULL, bearing in mind that NULL
> can't match any string, I would expect that to_array would return the
> undivided string as a single array element, and that to_string would
> throw an error:
>

ok, it has a sense.

other question is empty string as separator - but I think, it can has
same behave like string_to_array and array_to_string functions.

> to_array('1,2,3,4,5', null) = '{"1,2,3,4,5"}'
> to_string(array[1,2,3,4,5], null) = ERROR: the field separator for
> to_string may not be NULL
>
> If the first argument is NULL for either function, I think it would be
> reasonable to return NULL.  But I could be convinced that we should
> throw an error in that case too.
>

I agree - I prefer a NULL

Thank You very much

Pavel

> Cheers,
> BJ
>

--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: Brendan Jurd on 16 Jul 2010 15:16
On 17 July 2010 04:52, Pavel Stehule <pavel.stehule(a)gmail.com> wrote:
> 2010/7/16 Brendan Jurd <direvus(a)gmail.com>:
>> Also, if we're going to make the function non-strict, we need to
>> consider how to respond when the user specifies NULL for the other
>> arguments. �If the field separator is NULL, bearing in mind that NULL
>> can't match any string, I would expect that to_array would return the
>> undivided string as a single array element, and that to_string would
>> throw an error:
>>
>
> ok, it has a sense.
>
> other question is empty string as separator - but I think, it can has
> same behave like string_to_array and array_to_string functions.
>

Agreed. Those behaviours seem sensible.

>> If the first argument is NULL for either function, I think it would be
>> reasonable to return NULL. �But I could be convinced that we should
>> throw an error in that case too.
>>
>
> I agree - I prefer a NULL
>
> Thank You very much

No worries; I will await a revised patch from you which updates these
behaviours -- please incorporate the doc/comment changes I posted
earlier -- I will then do a further review before handing off to a
committer.

Cheers,
BJ

--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

First  |  Prev  |  Next  |  Last
Pages: 1 2 3 4 5
Prev: multibyte charater set in levenshtein function
Next: (9.1) btree_gist support for searching on "not equals"