timgrein commented on code in PR #3869:
URL: https://github.com/apache/calcite/pull/3869#discussion_r1687195732
##########
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) {
Review Comment:
Ok so I've debugged through this:
If you call `trim()` the following happens:
- Enters the block for 3 operands (first gets a default value, second and
third are `null`, not in the sense of their value, but of their absence)
- `operands[0]` gets set to `BOTH`
- `operands[1]` gets so to `" "`
- `operands[2]` is `null` (indicating absence of the arg not that it's
`null`)
So the actual method call from a user perspective was one without arguments
(`trim()`).
`trim(" a ")` works fine
`trim(both " a ")` works fine
`trim("a" from)` syntax error (string to trim is absent)
`trim(both "a" from)` syntax error (string to trim is absent)
So to handle the no-args case I think it's correct to check for `if
(operands[2] == null)` in the block with `3` operands. I'll add a comment
explaining it, otherwise it's confusing I think.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
