feat: support more advanced suite of LIKE expressions #5013
Conversation
There was a problem hiding this comment.
Thanks @agavra
LGTM, 'cept where does this character range stuff come from? I've not seen that in SQL before. Is that in the spec? AFAIK SQL LIKE only has _ and % with special meaning...
(I'm only putting a blocker on as I think we should think carefully about non-SQL syntax, if indeed it is).
| */ | ||
| public static boolean matches(final String val, final String pattern) { | ||
| // note that we do not yet support escape characters in the pattern | ||
| // see issue |
There was a problem hiding this comment.
missing issue link?
| {"percents", "barfoobaz", "%foo%"}, | ||
| {"percents [X]", "barbarbar", "%foo%"}, | ||
| {"percents one side", "barfoo", "%foo"}, | ||
| {"percents one side [X]", "barbarbar", "%foo"}, |
There was a problem hiding this comment.
| {"percents one side [X]", "barbarbar", "%foo"}, | |
| {"percents one side [X]", "barfoobar", "%foo"}, |
There was a problem hiding this comment.
also, worth adding case for:
- % other side and
- % in middle and
- multiple %s
- escaped %s???? Or do we not support them yet.. (hard to add?)
There was a problem hiding this comment.
the middle two are covered by c[a-c]t%[a-e][^abc]%m__w - i'll add tests for the first and last (escaping is done by [%] in MSFT-sql at least and should be supported here as well)
Huh - I looked at https://docs.microsoft.com/en-us/sql/t-sql/language-elements/like-transact-sql?view=sql-server-ver15 since they tend to have the best docs and just assumed that it was standard. But looking at postgres they don't follow the same mechanism.... I'm happy to go either way about it, but that was the hardest to code so I'd low-key be bummed about removing it 😂 EDIT: looks like OracleSQL doesn't have it either |
|
@agavra can i chime in with another "looks good but it's in the wrong function" observation here ? I definitely like getting more search functionality but that SQLServer implementation is, unfortunately, not a very representative one to model from. The more"usual" implementation is to only support '_' and '%' in the |
|
Well it seems I've learned my lesson - I will check docs other than MSFT SqlServer before implementing difficult functionality 😂 I'll remove the |
|
Can you add the escape char too? |
If you give a mouse a cookie 🍪 .... haha sure I'll add it too |
agavra
force-pushed
the
like_exps
branch
4 times, most recently
from
2a89d7b to
72e60e3
Compare
There was a problem hiding this comment.
Thanks @agavra
LGTM, 'cept for the weird exception / error message you'll be showing to users if they enter a multi-char escape
ksqldb-engine/src/test/java/io/confluent/ksql/engine/rewrite/ExpressionTreeRewriterTest.java
Outdated
Show resolved
Hide resolved
|
|
||
| if (patternString.endsWith("%")) { | ||
| if (node.getEscape().isPresent()) { |
There was a problem hiding this comment.
nit: can we not remove this if/else by having LikeEval.matches() take Optional<Char> for last param?
There was a problem hiding this comment.
somewhat - I'd still need to do:
LikeEvaluator.matches(... + "Optional.ofNullable(" + escape.map(escape -> "'" + escape + "'").getOrElse(null))And to be honest I don't think that's any cleaner.
(this needs to be generated code, I can't pass the optional itself as a string)
ksqldb-parser/src/main/java/io/confluent/ksql/parser/AstBuilder.java
Outdated
Show resolved
Hide resolved
fixes #2054
Description
Adds support for more advanced LIKE functionality by turning a LIKE predicate into a regex statement and evaluating that. I looked for existing java implementations but none seemed much better than implementing it myself and avoiding a third-party (heavyweight) dependency.
This change also allows you to reference a column now in the LIKE predicate.
Testing done
Reviewer checklist