Re: patch (for 9.1) string functions

Hello

I am sending a actualised patch

* removed concat_json
* renamed function rvsr to reverse
* functions format, sprintf and concat* are stable now (as to_char for example)

2010/7/9 Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>:
> hello
>
> 2010/7/9 Takahiro Itagaki <itagaki(dot)takahiro(at)gmail(dot)com>:
>> 2010/7/8 Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>:
>>> sorry, attached fixed patch
>>
>> Make installcheck for contrib/stringfunc is broken.
>> Please run regression test with --enable-cassert build.
>>  test stringfunc           ... TRAP:
>> FailedAssertion("!(!lc_ctype_is_c())", File: "mbutils.c", Line: 715)
>>  LOG:  server process (PID 15121) was terminated by signal 6: Aborted

it worked on my station :( - Fedora 64bit

can you send a backtrace, please

Regards

Pavel Stehule

>>
>
>
>> This patch contains several functions.
>> - format(fmt text, VARIADIC args "any")
>> - sprintf(fmt text, VARIADIC args "any")
>> - concat(VARIADIC args "any")
>> - concat_ws(separator text, VARIADIC args "any")
>> - concat_json(VARIADIC args "any")
>> - concat_sql(VARIADIC args "any")
>> - rvrs(str text)
>> - left(str text, n int)
>> - right(str text, n int)
>>
>> The first one is in the core, and others are in contrib/stringfunc.
>> But I think almost
>> all of them should be in the core, because users want to write portable SQLs.
>> Contrib modules are not always available.  Note that concat() is
>> supported by Oracle,
>> MySQL, and DB2. Also left() and right() are supported by MySQL, DB2,
>> and SQL Server.
>>
>> Functions that depend on GUC settings should be marked as VOLATILE
>> instead of IMMUTABLE. I think format(), sprintf(), and all of
>> concat()s should be
>> volatile because at least timestamp depends on datestyle parameter.
>>
>
> ok, I'll fix it
>
>> concat_ws() and rvrs() should be renamed to non-abbreviated forms.
>> How about concat_with_sep() and reverse() ?
>>
>
> I used a well known names - concat_ws (MySQL) and rvrs (Oracle rdbms),
> I like concat_ws - concat_with_sep is maybe too long. rvrs is too
> short, so I'll rename it to reverse - ok?
>
>> I think we should avoid concat_json() at the moment because there is another
>> development project for JSON support. The result type will be JOIN type rather
>> than text then.
>>
>
> ok
>
>> I'm not sure usefulness of concat_sql(). Why don't you just quote all values
>> with quotes and separate them with comma?
>>
>
> concat_xxx functions are helpers to serialisation. So when when you
> would to generate INSERT statements for some export, and you cannot
> use a COPY statement, you can do
>
> FOR r IN
>  SELECT ....
> LOOP
>  RETURN NEXT 'INSERT INTO tab(..) VALUES (' || concat_sql(r.a, r.b, r.c, ... )
> END LOOP;
> RETURN;
>
> you don't need to solve anything and output is well formated SQL. Some
> databases dislike quoted numeric values - and quoted nums can be
> sonfusing
>
>
>>>> format() function prints NULL as "NULL", but RAISE statement in PL/pgSQL
>>>> does as "<NULL>".
>>> I prefer just NULL.
>>> maybe some GUC variable
>>> stringfunc.null_string = '<NULL>' in future??
>>
>> We have some choices for NULL representation. For example, empty string,
>> NULL, <NULL>, or (null) . What will be our choice?   Each of them looks
>> equally reasonable for me. GUC idea is also good because we need to
>> mark format() as VOLATILE anyway. We have nothing to lose.
>>
>
> Can ve to solve it other patch? I know to aversion core hackers to new
> GUC. Now I propose just "NULL". The GUC for NULL representation has
> bigger consequences - probably have to related to RAISE statement, and
> to proposed functions to_string, to_array.
>
>> ---
>> Takahiro Itagaki
>>
>
> Thank You very much, I'do fix it
>
> Pavel
>