From: "Kevin Grittner" on
Itagaki Takahiro <itagaki.takahiro(a)gmail.com> wrote:

> I'd like to move all proposed functions into the core, and not to
> add contrib/stringfunc.

> Still failed :-( I used UTF8 database with *locale=C* on 64bit
> Linux.
> char2wchar() doesn't seem to work on C locale. We should avoid
> using the function and converting mb chars to wide chars.
>
> select sprintf('>>>%10s %10d<<<', 'hello', 10);
> ! server closed the connection unexpectedly
> TRAP: FailedAssertion("!(!lc_ctype_is_c())", File: "mbutils.c",
> Line: 715)
>
> #0 0x00000038c0c332f5 in raise () from /lib64/libc.so.6
> #1 0x00000038c0c34b20 in abort () from /lib64/libc.so.6
> #2 0x00000000006e951d in ExceptionalCondition
> (conditionName=<value optimized out>, errorType=<value optimized
> out>, fileName=<value optimized out>, lineNumber=<value optimized
> out>) at assert.c:57
> #3 0x00000000006fa8bf in char2wchar (to=0x1daf188 L"", tolen=16,
> from=0x1da95b0 "%*s", fromlen=3) at mbutils.c:715
> #4 0x00007f8e8c436d37 in stringfunc_sprintf
> (fcinfo=0x7fff9bdcd4b0)
> at stringfunc.c:463

Based on this and subsequent posts, I've changed this patch's status
to "Waiting on Author".

-Kevin

--
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: Pavel Stehule on
2010/7/12 Kevin Grittner <Kevin.Grittner(a)wicourts.gov>:
> Itagaki Takahiro <itagaki.takahiro(a)gmail.com> wrote:
>
>> I'd like to move all proposed functions into the core, and not to
>> add contrib/stringfunc.
>
>> Still failed :-(  I used UTF8 database with *locale=C* on 64bit
>> Linux.
>> char2wchar() doesn't seem to work on C locale. We should avoid
>> using the function and converting mb chars to wide chars.
>>
>>   select sprintf('>>>%10s %10d<<<', 'hello', 10);
>> ! server closed the connection unexpectedly
>> TRAP: FailedAssertion("!(!lc_ctype_is_c())", File: "mbutils.c",
>> Line: 715)
>>
>> #0  0x00000038c0c332f5 in raise () from /lib64/libc.so.6
>> #1  0x00000038c0c34b20 in abort () from /lib64/libc.so.6
>> #2  0x00000000006e951d in ExceptionalCondition
>> (conditionName=<value optimized out>, errorType=<value optimized
>> out>, fileName=<value optimized out>, lineNumber=<value optimized
>> out>) at assert.c:57
>> #3  0x00000000006fa8bf in char2wchar (to=0x1daf188 L"", tolen=16,
>> from=0x1da95b0 "%*s", fromlen=3) at mbutils.c:715
>> #4  0x00007f8e8c436d37 in stringfunc_sprintf
>> (fcinfo=0x7fff9bdcd4b0)
>> at stringfunc.c:463
>
> Based on this and subsequent posts, I've changed this patch's status
> to "Waiting on Author".

ook - I'll send actualised version tomorrow

Pavel

>
> -Kevin
>

--
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: Pavel Stehule on
Hello

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

todo:

NULL handling for printf function

Query:
what is corect result for

* printf(">>%3.2d<<", NULL) ??
* printf(">>%3.2s", NULL) ??

Regards

Pavel Stehule


2010/7/12 Itagaki Takahiro <itagaki.takahiro(a)gmail.com>:
> 2010/7/12 Robert Haas <robertmhaas(a)gmail.com>:
>> I'm all in favor of putting such things in core as are supported by
>> multiple competing products, but is that really true for all of these?
>
> - concat() : MySQL, Oracle, DB2
> - concat_ws() : MySQL,
> - left(), right() : MySQL, SQL Server, DB2
> - reverse() : MySQL, SQL Server, Oracle (as utl_raw.reverse)
>
> concat_sql(), format(), and sprintf() will be our unique features.
>
> --
> Itagaki Takahiro
>
From: Itagaki Takahiro on
2010/7/13 Pavel Stehule <pavel.stehule(a)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.

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.

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

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

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

--
Itagaki Takahiro

--
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: Pavel Stehule on
Hello

2010/7/13 Itagaki Takahiro <itagaki.takahiro(a)gmail.com>:
> 2010/7/13 Pavel Stehule <pavel.stehule(a)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