Re: patch (for 9.1) string functions

From: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
To: Takahiro Itagaki <itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp>
Cc: 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-08 11:09:41
Message-ID: AANLkTimI35B5Z6cUTfqF8CvYdNhyCJR_2Z4zPP_h1PZX@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hello

2010/7/8 Takahiro Itagaki <itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp>:
>
> Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
>
>> updated version, concat function doesn't use separator
>
> BTW, didn't you forget stringfunc.sql.in for contrib/stringfunc ?
> So, I have not check stringfunc module yet.

sorry, attached fixed patch
>
> I reviewed your patch, and format() in the core is almost ok. It's very cool!
> On the other hand, contrib/stringfunc tries to implement safe-sprintf. It's
> very complex, and I have questions about multi-byte character handling in it.
>

I use same mechanism as RAISE statement does. And it working for mer

postgres=# select sprintf('žlutý%dkůň',10);
sprintf
------------
žlutý10kůň
(1 row)

Time: 0,647 ms

postgres=# select sprintf('%s žlutý kůň','příliš');
sprintf
------------------
příliš žlutý kůň
(1 row)

Time: 11,017 ms
postgres=# select sprintf('%10s žlutý kůň','příliš');
sprintf
----------------------
příliš žlutý kůň
(1 row)

Time: 0,439 ms

> * How to print NULL value.
> format() function prints NULL as "NULL", but RAISE statement in PL/pgSQL
> does as "<NULL>". Do we need the same result for them?

I prefer just NULL. You can add "<" and ">" simple if you want. But
removing is little bit dificult.

postgres=# select sprintf('%s', coalesce(NULL, '<NULL>'));
sprintf
---------
<NULL>
(1 row)

maybe some GUC variable

stringfunc.null_string = '<NULL>' in future??

>
>    postgres=# SELECT format('% vs %', 'NULL', NULL);
>        format
>    --------------
>     NULL vs NULL
>    (1 row)
>
>    postgres=# DO $$ BEGIN RAISE NOTICE '% vs %', 'NULL', NULL; END; $$;
>    NOTICE:  NULL vs <NULL>
>    DO
>
> * Error messages: "too few/many parameters"
>  For the same reason, "too few/many parameters specified for format()"
>  might be better for the messages.
>
>  For RAISE in PL/pgSQL:
>    ERROR:  too few parameters specified for RAISE
>    ERROR:  too many parameters specified for RAISE

ook, I agree

>
> * Why do you need convert multi-byte characters to wide char?
> Length specifier in stringfunc_sprintf() means "character length".
> But is pg_encoding_mbcliplen() enough for the purpose?
>

No, I need it. I use a swprintf function - for output of formated
strings - and there are not some sprintf function for multibyte chars
:(. Without this function I don't need a multibyte->widechars
conversion, but sprintf function will be much more larger and complex.

> * Character-length vs. disp-length in length specifier for sprintf()
> For example, '%10s' for sprintf() means "10 characters" in the code.
> But there might be usages to format text values for display. In such
> case, display length might be better for the length specifier.
> How about having both "s" and "S"?
>    "%10s" -- 10 characters
>    "%10S" -- 10 disp length; we could use pg_dsplen() for the purpse.

it is independent, because I use swprintf function

postgres=# select length(sprintf('%5s', 'ščř'));
length
--------
5
(1 row)

Time: 45,485 ms
postgres=# select length(sprintf('%5s', 'abc'));
length
--------
5
(1 row)

Time: 0,499 ms

so it is equal to using a pg_dsplen()

probably original one byte behave have sense for bytea data type. But
I am not sure if we would to complicate this function for binary data.

>
> Regards,

Thank you very much for review

Pavel Stehule
> ---
> Takahiro Itagaki
> NTT Open Source Software Center
>
>
>

Attachment Content-Type Size
stringfunc.diff application/octet-stream 41.8 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2010-07-08 11:53:28 Re: patch: preload dictionary new version
Previous Message Pavel Stehule 2010-07-08 11:03:03 Re: patch: preload dictionary new version