Re: patch (for 9.1) string functions

Hello

2010/7/13 Itagaki Takahiro <itagaki.takahiro@gmail.com>:
> 2010/7/13 Pavel Stehule <pavel.stehule@gmail.com>:
>> so this is actualised patch:
>> * concat_sql removed
>> * left, right, reverse and concat are in core
>> * printf and concat_ws are in contrib
>> * format show "<NULL>" as NULL string
>> * removed an using of wide chars
>
> I think function codes in the core (concat, format, left, right,
> and reverse) are ready for committers. They also have docs, but
> the names are not listed in Index page (bookindex.html).
> Please add
>   <indexterm>
>    <primary>funcname</primary>
>   </indexterm>
> in func.sgml for each new function.
>

fixed
> However, I have a couple of comments to stringfunc module. sprintf()
> and concat_ws() are not installed by default, but provided by the module.
>
>> todo:
>> NULL handling for printf function
>
> I like <NULL> for null arguments. It is just same as format() and RAISE.

done

>
> === Questions ===
> * concat_ws() transforms NULLs into empty strings.
> Is it an intended behavior and compatible with MySQL?
> Note that string_agg() doesn't add separators to NULLs.
>

no I was  wrong - original concat_ws just ignore NULL - fixed, now
concat_ws has same behave like original.

>  =# SELECT coalesce(concat_ws(',', 'A', NULL, 'B'), '(null)');
>   coalesce
>  ----------
>   A,,B
>  (1 row)
>
> * concat_ws() returns NULL when the separator is NULL.
> Is it an intended behavior and compatible with MySQL?
>
>  =# SELECT coalesce(concat_ws(NULL, 'A', NULL, 'B'), '(null)');
>   coalesce
>  ----------
>   (null)
>  (1 row)
>
> === Trivial issues ===
> * Some function prototypes are declared but not used.
>  We can just remove them.
>  - mb_string_info()
>  - stringfunc_concat(PG_FUNCTION_ARGS);
>  - stringfunc_left(PG_FUNCTION_ARGS);
>  - stringfunc_right(PG_FUNCTION_ARGS);
>  - stringfunc_reverse(PG_FUNCTION_ARGS);
>
> * Some error messages need to be improved.
>  For example, "1th" is wrong.
>    =# select sprintf('>>>%*s<<<', NULL, 'abcdef');
>    ERROR:  null value not allowed
>    HINT:  width (1th) arguments is NULL

have you a some idea about it?

>
> * sprintf() has some typos in error messages
>  For example, "sprinf".
>

fixed

> --
> Itagaki Takahiro
>

Regards

Pavel