Pavel Stehule <pavel.stehule@gmail.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.
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.
* 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?
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
bebetter for the messages.
For RAISE in PL/pgSQL: ERROR: too few parameters specified for RAISE ERROR: too many parameters specified for
RAISE
* 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?
* 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
thepurpse.
Regards,
---
Takahiro Itagaki
NTT Open Source Software Center
