[BEAM-5113][SQL] Add NOT LIKE based on LIKE expression#6186
[BEAM-5113][SQL] Add NOT LIKE based on LIKE expression#6186akedin merged 1 commit intoapache:masterfrom
Conversation
| break; | ||
|
|
||
| case "NOT LIKE": | ||
| ret = new BeamSqlNotLikeExpression(subExps); |
There was a problem hiding this comment.
Can we just pass a flag into BeamSqlLikeExpression?
There was a problem hiding this comment.
I prefer this wrapper way because readability reason. Flag introduces more complexity here to understand the code. It seems to me that if true then return value else reversed value is more confusing.
There was a problem hiding this comment.
Cool, I don't mind it this way.
But I disagree that it is more readable :) It is a whole new separate class for NotLike. If I see it in the code, I would wonder how is it different from Like, and I have to read the whole class now to make sure it works as expected. And in this case I am confused because it defines operation for numbers and dates, so now I also have to read the Like class to learn that it's undefined there and throws exceptions. It also adds surface area for errors, now reading this I have to make sure that there's no ! missing anywhere. Instead I could just read 2 lines of code :) and it doesn't have to be a boolean flag though, e.g.:
enum Like { LIKE, NOT_LIKE }
...
like = compare();
return (type == NOT_LIKE) ? !like : like;
There was a problem hiding this comment.
I see. It makes sense to think from this perspective. Readers might stop reading when read the class name (which self-explains the functionality of class). However, if readers have to dig into the implementation of the separate class, it indeed reduces readability of the code.
2 participants
Add NOT LIKE based on LIKE expression.
Follow this checklist to help us incorporate your contribution quickly and easily:
[BEAM-XXX] Fixes bug in ApproximateQuantiles, where you replaceBEAM-XXXwith the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.It will help us expedite review of your Pull Request if you tag someone (e.g.
@username) to look at it.Post-Commit Tests Status (on master branch)