-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Kernel][Expressions] Add support for LIKE expression #3103
[Kernel][Expressions] Add support for LIKE expression #3103
Conversation
Signed-off-by: Krishnan Paranji Ravi <krishna.pr@gmail.com>
Signed-off-by: Krishnan Paranji Ravi <krishna.pr@gmail.com>
Signed-off-by: Krishnan Paranji Ravi <krishna.pr@gmail.com>
@vkorukanti this is ready for review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Look great! Thanks for the PR. Few minor comments. Let me know if you have any questions.
kernel/kernel-api/src/main/java/io/delta/kernel/expressions/Like.java
Outdated
Show resolved
Hide resolved
.../src/main/java/io/delta/kernel/defaults/internal/expressions/DefaultExpressionEvaluator.java
Outdated
Show resolved
Hide resolved
.../src/main/java/io/delta/kernel/defaults/internal/expressions/DefaultExpressionEvaluator.java
Outdated
Show resolved
Hide resolved
.../src/main/java/io/delta/kernel/defaults/internal/expressions/DefaultExpressionEvaluator.java
Outdated
Show resolved
Hide resolved
.../src/main/java/io/delta/kernel/defaults/internal/expressions/DefaultExpressionEvaluator.java
Outdated
Show resolved
Hide resolved
...st/scala/io/delta/kernel/defaults/internal/expressions/DefaultExpressionEvaluatorSuite.scala
Outdated
Show resolved
Hide resolved
created special eval for Like expression Signed-off-by: Krishnan Paranji Ravi <krishna.pr@gmail.com>
Signed-off-by: Krishnan Paranji Ravi <krishna.pr@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Few minor comments. Otherwise looks good to go.
kernel/kernel-defaults/src/main/java/io/delta/kernel/defaults/internal/DefaultEngineErrors.java
Outdated
Show resolved
Hide resolved
.../src/main/java/io/delta/kernel/defaults/internal/expressions/DefaultExpressionEvaluator.java
Outdated
Show resolved
Hide resolved
.../src/main/java/io/delta/kernel/defaults/internal/expressions/DefaultExpressionEvaluator.java
Outdated
Show resolved
Hide resolved
.../src/main/java/io/delta/kernel/defaults/internal/expressions/DefaultExpressionEvaluator.java
Outdated
Show resolved
Hide resolved
.../src/main/java/io/delta/kernel/defaults/internal/expressions/DefaultExpressionEvaluator.java
Outdated
Show resolved
Hide resolved
...lts/src/main/java/io/delta/kernel/defaults/internal/expressions/LikeExpressionEvaluator.java
Outdated
Show resolved
Hide resolved
...st/scala/io/delta/kernel/defaults/internal/expressions/DefaultExpressionEvaluatorSuite.scala
Outdated
Show resolved
Hide resolved
...st/scala/io/delta/kernel/defaults/internal/expressions/DefaultExpressionEvaluatorSuite.scala
Outdated
Show resolved
Hide resolved
...st/scala/io/delta/kernel/defaults/internal/expressions/DefaultExpressionEvaluatorSuite.scala
Show resolved
Hide resolved
...st/scala/io/delta/kernel/defaults/internal/expressions/DefaultExpressionEvaluatorSuite.scala
Show resolved
Hide resolved
...lts/src/main/java/io/delta/kernel/defaults/internal/expressions/LikeExpressionEvaluator.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Krishnan Paranji Ravi <krishna.pr@gmail.com>
Signed-off-by: Krishnan Paranji Ravi <krishna.pr@gmail.com>
Signed-off-by: Krishnan Paranji Ravi <krishna.pr@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@krishnanravi LGTM. Very close to merging. Just one comment about the empty batch handling and few more test cases. Once those done, then this PR is good to merge.
.../src/main/java/io/delta/kernel/defaults/internal/expressions/DefaultExpressionEvaluator.java
Outdated
Show resolved
Hide resolved
.../src/main/java/io/delta/kernel/defaults/internal/expressions/DefaultExpressionEvaluator.java
Outdated
Show resolved
Hide resolved
...lts/src/main/java/io/delta/kernel/defaults/internal/expressions/LikeExpressionEvaluator.java
Outdated
Show resolved
Hide resolved
...lts/src/main/java/io/delta/kernel/defaults/internal/expressions/LikeExpressionEvaluator.java
Outdated
Show resolved
Hide resolved
...lts/src/main/java/io/delta/kernel/defaults/internal/expressions/LikeExpressionEvaluator.java
Outdated
Show resolved
Hide resolved
...st/scala/io/delta/kernel/defaults/internal/expressions/DefaultExpressionEvaluatorSuite.scala
Show resolved
Hide resolved
...st/scala/io/delta/kernel/defaults/internal/expressions/DefaultExpressionEvaluatorSuite.scala
Outdated
Show resolved
Hide resolved
...st/scala/io/delta/kernel/defaults/internal/expressions/DefaultExpressionEvaluatorSuite.scala
Outdated
Show resolved
Hide resolved
...lts/src/main/java/io/delta/kernel/defaults/internal/expressions/LikeExpressionEvaluator.java
Show resolved
Hide resolved
...st/scala/io/delta/kernel/defaults/internal/expressions/DefaultExpressionEvaluatorSuite.scala
Show resolved
Hide resolved
Signed-off-by: Krishnan Paranji Ravi <krishna.pr@gmail.com>
Signed-off-by: Krishnan Paranji Ravi <krishna.pr@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. thank you @krishnanravi!
## Description Add SQL `LIKE` expression support in Kernel list of supported expressions and a default implementation. Addresses part of delta-io#2539 (where `STARTS_WITH` as `LIKE 'str%'`) ## How was this patch tested? added unit tests Signed-off-by: Krishnan Paranji Ravi <krishna.pr@gmail.com>
Which Delta project/connector is this regarding?
Description
Add SQL
LIKE
expression support in Kernel list of supported expressions and a default implementation.Addresses part of #2539 (where
STARTS_WITH
asLIKE 'str%'
)How was this patch tested?
added unit tests