Re: patch (for 9.1) string functions

From: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
To: Itagaki Takahiro <itagaki(dot)takahiro(at)gmail(dot)com>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Merlin Moncure <mmoncure(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Erik Rijkers <er(at)xs4all(dot)nl>
Subject: Re: patch (for 9.1) string functions
Date: 2010-07-26 03:44:28
Message-ID: AANLkTinLDc1YT5hHcDiueY+Eg8vR6pN22ZBqFEqF6LY-@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

2010/7/26 Itagaki Takahiro <itagaki(dot)takahiro(at)gmail(dot)com>:
> I merged and enhanced some part of your patch:
>  - contrib/stringfunc are merged in the core patch
>  - Old format() is replaced with sprintf(), but the function name is
> still format().
>  - Support %q as alias for %iq.
>
> 2010/7/25 Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>:
>> fixed - it depends on INT64_FORMAT now.
> I modified the code a bit not to expect 'll' or 'l'.
>
>> %lq ... literal quoted
>> %iq ... ident quoted
> I also modified 'q' without specifier, i.e, %q is handled as same as %lq.
>
>>> But I found there is a design issue in format() :
>> I prefer a current behave - RAISE statement uses same and it is not
>> reported as bug for ten years
>
> 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
>
> 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.
>

Sorry, I am strong against. Using a format just for string string
concation is bad idea - there are a concat function, there are ||
operator. Look on complexity of format/RAISE and sprintf. More - it
can be strange, when we have a "format" function and it is almost
"sprintf". I still prefer simplicity of format - you have a true - it
has a issue, but there are simply workaround

format('%', 123||345)

format is very simple - but usually you don't need to specify a with,
a precision.

sprintf has some issue based on common sprintf implementation and
expecting too. For example a precision is used very dynamically - it
has a different sense for integers and for floats, so I wouldn't have
a sprintf in core.

> 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.
>
> Comments?

I disagree - please, return to prev variant

Regards

Pavel Stehule
>
> --
> Itagaki Takahiro
>

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Itagaki Takahiro 2010-07-26 04:00:50 Re: patch (for 9.1) string functions
Previous Message Itagaki Takahiro 2010-07-26 03:29:32 Re: patch (for 9.1) string functions