Subject: Re: TRIM_ARRAY
To: Dian Fay <dian.m.fay@gmail.com>, pgsql-hackers@lists.postgresql.org
Cc: Vik Fearing <vikreykja@gmail.com>
References: <fc92ce17-9655-8ff1-c62a-4dc4c8ccd815@postgresfriends.org>
 <931d94fa-9238-47ba-6f17-01600a845417@postgresfriends.org>
 <161464047991.29967.8270411748023500393.pgcf@coridan.postgresql.org>
From: Vik Fearing <vik@postgresfriends.org>
Message-ID: <b5ee1ac8-e403-6cdf-fcc3-dd5ccd5ceaf8@postgresfriends.org>
Date: Tue, 2 Mar 2021 00:53:04 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.10.0
MIME-Version: 1.0
In-Reply-To: 
 <161464047991.29967.8270411748023500393.pgcf@coridan.postgresql.org>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 3/2/21 12:14 AM, Dian Fay wrote:
> The following review has been posted through the commitfest application:
> make installcheck-world:  tested, passed
> Implements feature:       tested, passed
> Spec compliant:           tested, failed
> Documentation:            tested, failed

Thank you for looking at my patch!

> This basically does what it says, and the code looks good. The
> documentation is out of alphabetical order (trim_array should appear
> under cardinality rather than over)) but good otherwise.

Hmm.  It appears between cardinality and unnest in the source code and
also my compiled html.  Can you say more about where you're seeing the
wrong order?

> I was able to
> "break" the function with an untyped null in psql:
> 
> select trim_array(null, 2);
> ERROR:  could not determine polymorphic type because input has type unknown
> 
> I don't know whether there are any circumstances other than manual entry
> in psql where this could happen, since column values and variables will
> always be typed. I don't have access to the standard, but DB2's docs[1]
> note "if any argument is null, the result is the null value", so an
> up-front null check might be preferable to a slightly arcane user-facing
> error, even if it's a silly invocation of a silly function :)
> 
> [1] https://www.ibm.com/support/knowledgecenter/en/SSEPEK_12.0.0/sqlref/src/tpc/db2z_bif_trimarray.html

The standard also says that if either argument is null, the result is
null.  The problem here is that postgres needs to know what the return
type is and it can only determine that from the input.

If you give the function a typed null, it returns null as expected.

> The new status of this patch is: Waiting on Author

I put it back to Needs Review without a new patch because I don't know
what I would change.
-- 
Vik Fearing