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