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