kbendick commented on a change in pull request #2062:
URL: https://github.com/apache/iceberg/pull/2062#discussion_r762686988
##########
File path:
api/src/main/java/org/apache/iceberg/expressions/ExpressionVisitors.java
##########
@@ -242,6 +248,10 @@ public R or(R leftResult, R rightResult) {
throw new UnsupportedOperationException("Unsupported operation.");
}
+ public <T> R notStartsWith(Bound<T> expr, Literal<T> lit) {
+ throw new UnsupportedOperationException("Unsupported operation.");
Review comment:
I think generally we try to touch as few lines as needed in some cases
(or avoid unnecessary changes), as that makes it easier on people who maintain
forks.
I don't disagree with you that the error message should be more descriptive,
but given what I know about the API and the implemented classes, I don't think
these specific base class for `BoundVisitor` actually get called anywhere.
Notice how many of them return `null` as well.
As for the specific error message, it's just to be consistent with the
existing one for `startsWith`.
Given the size of the PR already, I think it's best to keep unrelated
changes out and in a separate PR (particularly updating `startsWith` - but
until then, let's be consistent).
Large PRs like this already face a hurdle.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
