Re: patch (for 9.1) string functions

On Sun, Jul 25, 2010 at 11:29 PM, Itagaki Takahiro
<itagaki(dot)takahiro(at)gmail(dot)com> wrote:
> I think RAISE is badly designed. Using % as a placeholder has a limitation
> to format strings. For example, format() cannot work as concat():
>  SELECT format('%%', 123, 456) => ERROR

It's hard to argue with this, as far as it goes.

> So, my proposal is renaming stringfunc//sprintf() to format(),
> and moving it into the core. I think sprintf() is superior to format()
> in every aspect; '%s%s' works as concat(), and '%s%%' can append
> % without blanks.

So forget about format() and put sprintf() in contrib/stringfunc.
That's not an argument for putting anything in core. Perhaps such an
argument can be made, but this isn't it.

> Then, concat_ws() will be moved into core because contrib/stringfunc
> only has the function now. In addition, I'd like to include the function for
> the compatibility to MySQL. Also, concat() and concat_ws() can share
> the implementation.

Regardless of where this function ends up, the concat_ws documentation
should contain some mention of the fact that "ws" is intended to mean
"with separator", and that the naming is chosen for compatibility with
MySQL. As for where to put it, I see no terribly compelling reason
why it needs to be in core. You can write array_to_string(array[txt1,
txt2, txt3], sep) and get the same effect as concat_ws(sep, txt1,
txt2, txt3). I don't really want to start maintaining duplicate
functionality for things like this, especially since MySQL users will
no doubt expect that our implementation will be bug-compatible. If
the output isn't identical to what MySQL does for every set of
arguments, it'll be reported as a bug.

Come to think of it, have we checked that the behavior of LEFT, RIGHT,
REVERSE, etc. is the same on other DBs, especially as far as nulls,
empty strings, too-large or negative subscripts, etc is concerned? Is
CONCAT('foo', NULL) => 'foo' really the behavior that everyone else
implements here? And why does CONCAT() take a variadic "ANY"
argument? Shouldn't that be variadic TEXT?

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company