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

From: Bruce Momjian <bruce(at)momjian(dot)us>
To: amutu(at)amutu(dot)com
Cc: pgsql-bugs(at)postgresql(dot)org
Subject: Re: BUG #8335: trim() un-document behaviour
Date: 2013-08-07 16:19:53
Message-ID: 20130807161953.GZ11189@momjian.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs pgsql-hackers

On Fri, Jul 26, 2013 at 02:23:10AM +0000, amutu(at)amutu(dot)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(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

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

Attachment Content-Type Size
trim.diff text/x-diff 710 bytes

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Sergey Konoplev 2013-08-08 01:29:30 Re: How to avoid Force Autovacuum
Previous Message Alvaro Herrera 2013-08-07 15:35:52 Re: BUG #8373: can create database with long name , but can't connect

Browse pgsql-hackers by date

  From Date Subject
Next Message Thom Brown 2013-08-07 16:21:01 9.4 regression
Previous Message Bruce Momjian 2013-08-07 15:08:34 Re: Unsafe GUCs and ALTER SYSTEM WAS: Re: ALTER SYSTEM SET