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
