Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[Kernel][Predicate] - WIP - Add support for STARTS_WITH expression #3095

Closed
wants to merge 5 commits into from
Closed

[Kernel][Predicate] - WIP - Add support for STARTS_WITH expression #3095

wants to merge 5 commits into from

Conversation

krishnanravi
Copy link
Contributor

Which Delta project/connector is this regarding?

  • Spark
  • Standalone
  • Flink
  • Kernel
  • Other (fill in here)

Description

addresses #2539

How was this patch tested?

added unit tests

Does this PR introduce any user-facing changes?

No

vkorukanti
vkorukanti requested changes May 14, 2024
View reviewed changes
Copy link
Collaborator

@vkorukanti vkorukanti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @krishnanravi for the PR.

Wondering if we can directly support LIKE. STARTS_WITH is a special form of LIKE.

E.g. STARTS_WITH(column, 'city') is equals to column LIKE 'city%'

kernel/kernel-api/src/main/java/io/delta/kernel/expressions/Predicate.java Outdated
* <li>Name: <code>STARTS_WITH</code>
* <ul>
* <li>SQL semantic: <code>expr STARTS WITH expr</code></li>
* <li>Since version: 3.2.0</li>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change it to 3.3.0 (next release)

kernel/kernel-api/src/main/java/io/delta/kernel/expressions/StartsWith.java Outdated
* Definition:
* <p><ul>
* <li>{@code expr1} STARTS WITH {@code expr2} on two inputs.</li>
* <li>Requires both left and right input expressions of type {@link Predicate}.</li>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

left and right need to be strings, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes. updated.

kernel/kernel-api/src/main/java/io/delta/kernel/expressions/StartsWith.java Outdated
* Note that matches are case insensitive.</li>
* </ul>
*
* @since 3.0.0
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change the version to 3.3.0

@krishnanravi
updates to release version and api docs
6aa1e19 
Signed-off-by: Krishnan Paranji Ravi <[email protected]>
@krishnanravi
Copy link
Contributor Author

krishnanravi commented May 14, 2024 •
edited
Loading

Thank you @krishnanravi for the PR.

Wondering if we can directly support LIKE. STARTS_WITH is a special form of LIKE.

E.g. STARTS_WITH(column, 'city') is equals to column LIKE 'city%'

@vkorukanti yes makes sense to me. are you suggesting we replace STARTS_WITH with LIKE support instead, and as part of this change/PR?

@vkorukanti
Copy link
Collaborator

@vkorukanti yes makes sense to me. are you suggesting we replace STARTS_WITH with LIKE support instead, and as part of this change/PR?

Yep, lets add the LIKE as part of this PR. Basically, replace the STARTS_WITH like and the eval to use the regex. There was a similar PR for standalone that you can refer to. The rest of the changes in this PR are still the same.

@krishnanravi krishnanravi marked this pull request as draft May 14, 2024 23:10
@krishnanravi krishnanravi changed the title [Kernel][Predicate] Add support for STARTS_WITH expression [Kernel][Predicate] - WIP - Add support for STARTS_WITH expression May 14, 2024
@krishnanravi
Copy link
Contributor Author

krishnanravi commented May 15, 2024 •
edited
Loading

@vkorukanti yes makes sense to me. are you suggesting we replace STARTS_WITH with LIKE support instead, and as part of this change/PR?

Yep, lets add the LIKE as part of this PR. Basically, replace the STARTS_WITH like and the eval to use the regex. There was a similar PR for standalone that you can refer to. The rest of the changes in this PR are still the same.

@vkorukanti assume we will need support for a custom escape char in input.
e.g. col1 LIKE '%\%%' ESCAPE '\' should escape the second % token and the escape char per se can be provided for LiKE predicates.

@vkorukanti
Copy link
Collaborator

@vkorukanti assume we will need support for a custom escape char in input. e.g. col1 LIKE '%\%%' ESCAPE '\' should escape the second % token and the escape char per se can be provided for LiKE predicates.
Let's start with the simple like with no escape and you can follow up the escape in a separate PR. Thanks again for picking this up.

@krishnanravi krishnanravi mentioned this pull request May 15, 2024
Merged
5 tasks
@krishnanravi krishnanravi deleted the startsWith-predicate-support branch May 31, 2024 01:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vkorukanti vkorukanti vkorukanti requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@krishnanravi @vkorukanti