Re: patch (for 9.1) string functions

hello

2010/7/9 Takahiro Itagaki <itagaki.takahiro@gmail.com>:
> 2010/7/8 Pavel Stehule <pavel.stehule@gmail.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
>


> 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