Re: BUG #8335: trim() un-document behaviour

On Fri, Jul 26, 2013 at 02:23:10AM +0000, amutu@amutu.com wrote:
> in the postgresql doc 9.4,I find the trim() function like this:
>
>
> trim([leading | trailing | both] [characters] from string)
>
>
> so the trim should be pass only one argument with some optional prefix --- but I
> find the following calls with two argument is successfull but the results is
> unexpected and wired:
>
>
> ##first call
> postgres=# select trim(trailing '/, 'fasd/');
> rtrim
> ------
>
>
> (1 row)
> -----!!!note: it return titile is rtrim----
>
>
> ## second call
> postgres=# select trim('/', 'fasd/')
> ;
> btrim
> -----
>
>
> (1 row)
> -----!!!note: it return titile is btrim----
>
>
> it seems trim is transform to rtrim internal but the above call should
> return error or it may produce un-expect results

(I have cleaned up this posting because single-quotes were converted to
Unicode forward-backward quotes):

What is happening is that TRIM() is converted by the parser to calls to
base functions, e.g.

    \df *trim*
                              List of functions
       Schema   | Name  | Result data type | Argument data types |  Type
    ------------+-------+------------------+---------------------+--------
     pg_catalog | btrim | bytea            | bytea, bytea        | normal
     pg_catalog | btrim | text             | text                | normal
     pg_catalog | btrim | text             | text, text          | normal
     pg_catalog | ltrim | text             | text                | normal
     pg_catalog | ltrim | text             | text, text          | normal
     pg_catalog | rtrim | text             | text                | normal
     pg_catalog | rtrim | text             | text, text          | normal

That is why the headings don't say 'trim', but 'btrim', or similar ---
not sure we can easily improve that, and you can change the label with
AS.

The larger problem is the use of ',' instead of FROM, and the backwards
interpretation of the arguments.  The query:

    SELECT trim('/' FROM 'fasd/')

is internally converted to:

    SELECT btrim('fasd/', '/')

Note the arguments are reversed.  The comma syntax does not reverse the
arguments:

    SELECT trim('/', 'fasd/')

is internally converted to:

    SELECT btrim('/', 'fasd/')

You can even use modifiers like TRAILING with comma syntax:

    SELECT trim(TRAILING '/', 'fasd/');

and that uses 'rtrim', but of course the behavior is still reverse of
expected.

Basically the odd comma behavior is because without a FROM, the
arguments are passed directly to btrim/rtrim/ltrim, and these functions
take the origin string first, then the string of characters to remove.
You are right this is undocumented.

The attached patch swaps the arguments in the parser, and allows your
expected behavior:

    SELECT trim('x', 'xfasdx');
     btrim
    -------
     fasd

Another option would be to change the C API for the b/r/ltrim functions,
or disallow the use of the comma TRIM syntax in the parser.

I am a little worried people might be relying on the trim/comma syntax
somewhere.

--
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + It's impossible for everything to be true. +