timgrein commented on code in PR #3869:
URL: https://github.com/apache/calcite/pull/3869#discussion_r1687181667


##########
core/src/main/java/org/apache/calcite/sql/fun/SqlTrimFunction.java:
##########
@@ -130,6 +130,9 @@ public SqlTrimFunction(String name, SqlKind kind,
       if (operands[1] == null) {
         operands[1] = SqlLiteral.createCharString(" ", pos);
       }
+      if (operands[2] == null) {
+        throw new IllegalArgumentException("String to trim cannot be null");

Review Comment:
   > `"The second argument of trim cannot be null"`
   
   This is not 100% correct I think as the string to trim from can be `null`: 
`trim('a' from cast(null as varchar(1)))` returns `null`, which is valid. I 
think it's more about the absence of the argument, which feels to me like a 
different thing than a value/field being `null`.
   
   Ideally we should correctly fall through to the default case or handle the 
cases, where the string to trim is absent explicitly.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to