From: | Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> |
---|---|
To: | Takahiro Itagaki <itagaki(dot)takahiro(at)gmail(dot)com> |
Cc: | Takahiro Itagaki <itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp>, Merlin Moncure <mmoncure(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: patch (for 9.1) string functions |
Date: | 2010-07-09 07:40:48 |
Message-ID: | AANLkTinlfI8-bZctFUyplPr0-AiSKtamjlvoM-i2268S@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
>
> 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
From | Date | Subject | |
---|---|---|---|
Next Message | Pavel Stehule | 2010-07-09 09:12:19 | Re: patch (for 9.1) string functions |
Previous Message | Pavel Stehule | 2010-07-09 06:44:35 | Re: patch: preload dictionary new version |