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][Data skipping] Add the STARTS_WITH expression and support data skipping for it #2539

Open
2 of 8 tasks
allisonport-db opened this issue Jan 18, 2024 · 6 comments
Open
2 of 8 tasks

[Kernel][Data skipping] Add the STARTS_WITH expression and support data skipping for it #2539

allisonport-db opened this issue Jan 18, 2024 · 6 comments
Assignees
@krishnanravi
Labels
enhancement New feature or request good first issue Good for newcomers kernel
Milestone
3.4.0

Comments

@allisonport-db
Copy link
Collaborator

Feature request

Which Delta project/connector is this regarding?

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

Overview

Currently Kernel supports a limited set of expressions. We should 1) add the STARTS_WITH expression and 2) use file statistics to prune files based on the expression.

Motivation

Better file pruning.

Further details

This means we should

  1. add STARTS_WITH to the Kernel Predicate and support it in the kernel-defaults project
  2. Generate a data skipping filter according to the same rules we use in delta-spark

Willingness to contribute

The Delta Lake Community encourages new feature contributions. Would you or another member of your organization be willing to contribute an implementation of this feature?

  • Yes. I can contribute this feature independently.
  • Yes. I would be willing to contribute this feature with guidance from the Delta Lake community.
  • No. I cannot contribute this feature at this time.
@allisonport-db allisonport-db added enhancement New feature or request kernel labels Jan 18, 2024
@vkorukanti vkorukanti added the good first issue Good for newcomers label Feb 19, 2024
@vkorukanti vkorukanti added this to the 3.2.0 milestone Feb 27, 2024
@vkorukanti vkorukanti modified the milestones: 3.2.0, 3.3.0 Apr 24, 2024
@krishnanravi
Copy link
Contributor

@allisonport-db @vkorukanti I would like to work on this.

@vkorukanti
Copy link
Collaborator

@krishnanravi Sounds good. I assigned the issue to you. Thank you!

@vkorukanti
Copy link
Collaborator

Here is an example PR that adds the IS_NULL expression. STARTS_WITH also requires similar changes. Feel free to ping us for any questions.

@krishnanravi
Copy link
Contributor

krishnanravi commented May 11, 2024 •
edited
Loading

@vkorukanti qq - what data types do we want to support for starts with in kernel defaults?
I just finished an implementation that enforces left side data type and right are always string which otherwise would raise an unsupported operation exception. Is this enough for the kernel defaults?
in contrast, looking at support for starts with in spark, any expression of any data type is supported on both sides. do we want an implementation as comprehensive as spark?

This was referenced May 14, 2024
Closed
Merged
vkorukanti pushed a commit that referenced this issue May 23, 2024
@krishnanravi
[Kernel][Expressions] Add support for LIKE expression (#3103)
0deef04 
## Description
Add SQL `LIKE` expression support in Kernel list of supported expressions and a default implementation.

Addresses part of #2539 (where `STARTS_WITH` as `LIKE 'str%'`)

## How was this patch tested?
added unit tests

Signed-off-by: Krishnan Paranji Ravi <[email protected]>
longvu-db pushed a commit to longvu-db/delta that referenced this issue May 28, 2024
@krishnanravi @longvu-db
[Kernel][Expressions] Add support for LIKE expression (delta-io#3103)
8b58e90 
## 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 <[email protected]>
@vkorukanti
Copy link
Collaborator

The LIKE expression is available. We can write the STARTS_WITH as LIKE 'str% and add the remaining data skipping part of the work.

@huan233usc huan233usc mentioned this issue Dec 19, 2024
Closed
5 tasks
@allisonport-db allisonport-db modified the milestones: 3.3.0, 3.4.0 Dec 20, 2024
This was referenced Dec 23, 2024
Merged
Merged
allisonport-db pushed a commit that referenced this issue Jan 6, 2025
@huan233usc
[Kernel] Add STARTS_WITH expression and default impl (#4007)
b6745bb 
<!--
Thanks for sending a pull request!  Here are some tips for you:
1. If this is your first time, please read our contributor guidelines:
https://github.com/delta-io/delta/blob/master/CONTRIBUTING.md
2. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP]
Your PR title ...'.
  3. Be sure to keep the PR description updated to reflect all changes.
  4. Please write your PR title to summarize what this PR proposes.
5. If possible, provide a concise example to reproduce the issue for a
faster review.
6. If applicable, include the corresponding issue number in the PR title
and link it in the body.
-->

#### Which Delta project/connector is this regarding?
<!--
Please add the component selected below to the beginning of the pull
request title
For example: [Spark] Title of my pull request
-->

- [ ] Spark
- [ ] Standalone
- [ ] Flink
- [x] Kernel
- [ ] Other (fill in here)

## Description

<!--
- Describe what this PR changes.
- Describe why we need the change.
 
If this PR resolves an issue be sure to include "Resolves #XXX" to
correctly link and close the issue upon merge.
-->
Initial implementation of STARTS_WITH expression at this moment, we only
support b as literal expression.

This is 1/n for addressing
#2539, the logic of data
skipping will be done in the following PRs

## How was this patch tested?

<!--
If tests were added, say they were added here. Please make sure to test
the changes thoroughly including negative and positive cases if
possible.
If the changes were tested in any way other than unit tests, please
clarify how you tested step by step (ideally copy and paste-able, so
that other reviewers can test and check, and descendants can verify in
the future).
If the changes were not tested, please explain why.
-->
Added test cases in DefaultExpressionEvaluatorSuite.scala

## Does this PR introduce _any_ user-facing changes?

<!--
If yes, please clarify the previous behavior and the change this PR
proposes - provide the console output, description and/or an example to
show the behavior difference if possible.
If possible, please also clarify if this is a user-facing change
compared to the released Delta Lake versions or within the unreleased
branches such as master.
If no, write 'No'.
-->
No

---------

Co-authored-by: Xin Huang <[email protected]>
vkorukanti pushed a commit that referenced this issue Jan 7, 2025
@huan233usc
[Kernel] Add SUBSTRING expression and default impl (#4001)
7381d5a 
<!--
Thanks for sending a pull request!  Here are some tips for you:
1. If this is your first time, please read our contributor guidelines:
https://github.com/delta-io/delta/blob/master/CONTRIBUTING.md
2. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP]
Your PR title ...'.
  3. Be sure to keep the PR description updated to reflect all changes.
  4. Please write your PR title to summarize what this PR proposes.
5. If possible, provide a concise example to reproduce the issue for a
faster review.
6. If applicable, include the corresponding issue number in the PR title
and link it in the body.
-->

#### Which Delta project/connector is this regarding?
<!--
Please add the component selected below to the beginning of the pull
request title
For example: [Spark] Title of my pull request
-->

- [ ] Spark
- [ ] Standalone
- [ ] Flink
- [x] Kernel
- [ ] Other (fill in here)

## Description
<!--
- Describe what this PR changes.
- Describe why we need the change.
 
If this PR resolves an issue be sure to include "Resolves #XXX" to
correctly link and close the issue upon merge.
-->

Implements substring expression. This is 2/n for addressing
#2539 (i.e. data skipping of
startwith depends on substring expression, see[1] for spark's code
pointer), the first part is implemented in
#4007.


[1]https://github.com/delta-io/delta/blob/master/spark/src/main/scala/org/apache/spark/sql/delta/stats/DataSkippingReader.scala#L557-L561

## How was this patch tested?

<!--
If tests were added, say they were added here. Please make sure to test
the changes thoroughly including negative and positive cases if
possible.
If the changes were tested in any way other than unit tests, please
clarify how you tested step by step (ideally copy and paste-able, so
that other reviewers can test and check, and descendants can verify in
the future).
If the changes were not tested, please explain why.
-->

Added e2e test cases, the expected result are generated using spark,
e.g.

WITH t(x) AS 
(SELECT null union all select "one" union all select "two" union all
select "three" union all select "four" union all select null union all
select null union all select "seven" union all select "eight")
select substring(x,-100,98) from t

substring(x,-100,98)
null
o
t
thr
fo
null
null
sev
eig

## Does this PR introduce _any_ user-facing changes?

<!--
If yes, please clarify the previous behavior and the change this PR
proposes - provide the console output, description and/or an example to
show the behavior difference if possible.
If possible, please also clarify if this is a user-facing change
compared to the released Delta Lake versions or within the unreleased
branches such as master.
If no, write 'No'.
-->
No

---------

Co-authored-by: Xin Huang <[email protected]>
huan233usc added a commit to huan233usc/delta that referenced this issue Jan 17, 2025
@huan233usc
[Kernel] Add STARTS_WITH expression and default impl (delta-io#4007)
a380216 
<!--
Thanks for sending a pull request!  Here are some tips for you:
1. If this is your first time, please read our contributor guidelines:
https://github.com/delta-io/delta/blob/master/CONTRIBUTING.md
2. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP]
Your PR title ...'.
  3. Be sure to keep the PR description updated to reflect all changes.
  4. Please write your PR title to summarize what this PR proposes.
5. If possible, provide a concise example to reproduce the issue for a
faster review.
6. If applicable, include the corresponding issue number in the PR title
and link it in the body.
-->

#### Which Delta project/connector is this regarding?
<!--
Please add the component selected below to the beginning of the pull
request title
For example: [Spark] Title of my pull request
-->

- [ ] Spark
- [ ] Standalone
- [ ] Flink
- [x] Kernel
- [ ] Other (fill in here)

## Description

<!--
- Describe what this PR changes.
- Describe why we need the change.
 
If this PR resolves an issue be sure to include "Resolves #XXX" to
correctly link and close the issue upon merge.
-->
Initial implementation of STARTS_WITH expression at this moment, we only
support b as literal expression.

This is 1/n for addressing
delta-io#2539, the logic of data
skipping will be done in the following PRs

## How was this patch tested?

<!--
If tests were added, say they were added here. Please make sure to test
the changes thoroughly including negative and positive cases if
possible.
If the changes were tested in any way other than unit tests, please
clarify how you tested step by step (ideally copy and paste-able, so
that other reviewers can test and check, and descendants can verify in
the future).
If the changes were not tested, please explain why.
-->
Added test cases in DefaultExpressionEvaluatorSuite.scala

## Does this PR introduce _any_ user-facing changes?

<!--
If yes, please clarify the previous behavior and the change this PR
proposes - provide the console output, description and/or an example to
show the behavior difference if possible.
If possible, please also clarify if this is a user-facing change
compared to the released Delta Lake versions or within the unreleased
branches such as master.
If no, write 'No'.
-->
No

---------

Co-authored-by: Xin Huang <[email protected]>
huan233usc added a commit to huan233usc/delta that referenced this issue Jan 17, 2025
@huan233usc
[Kernel] Add SUBSTRING expression and default impl (delta-io#4001)
2a5f9d2 
<!--
Thanks for sending a pull request!  Here are some tips for you:
1. If this is your first time, please read our contributor guidelines:
https://github.com/delta-io/delta/blob/master/CONTRIBUTING.md
2. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP]
Your PR title ...'.
  3. Be sure to keep the PR description updated to reflect all changes.
  4. Please write your PR title to summarize what this PR proposes.
5. If possible, provide a concise example to reproduce the issue for a
faster review.
6. If applicable, include the corresponding issue number in the PR title
and link it in the body.
-->

#### Which Delta project/connector is this regarding?
<!--
Please add the component selected below to the beginning of the pull
request title
For example: [Spark] Title of my pull request
-->

- [ ] Spark
- [ ] Standalone
- [ ] Flink
- [x] Kernel
- [ ] Other (fill in here)

## Description
<!--
- Describe what this PR changes.
- Describe why we need the change.
 
If this PR resolves an issue be sure to include "Resolves #XXX" to
correctly link and close the issue upon merge.
-->

Implements substring expression. This is 2/n for addressing
delta-io#2539 (i.e. data skipping of
startwith depends on substring expression, see[1] for spark's code
pointer), the first part is implemented in
delta-io#4007.


[1]https://github.com/delta-io/delta/blob/master/spark/src/main/scala/org/apache/spark/sql/delta/stats/DataSkippingReader.scala#L557-L561

## How was this patch tested?

<!--
If tests were added, say they were added here. Please make sure to test
the changes thoroughly including negative and positive cases if
possible.
If the changes were tested in any way other than unit tests, please
clarify how you tested step by step (ideally copy and paste-able, so
that other reviewers can test and check, and descendants can verify in
the future).
If the changes were not tested, please explain why.
-->

Added e2e test cases, the expected result are generated using spark,
e.g.

WITH t(x) AS 
(SELECT null union all select "one" union all select "two" union all
select "three" union all select "four" union all select null union all
select null union all select "seven" union all select "eight")
select substring(x,-100,98) from t

substring(x,-100,98)
null
o
t
thr
fo
null
null
sev
eig

## Does this PR introduce _any_ user-facing changes?

<!--
If yes, please clarify the previous behavior and the change this PR
proposes - provide the console output, description and/or an example to
show the behavior difference if possible.
If possible, please also clarify if this is a user-facing change
compared to the released Delta Lake versions or within the unreleased
branches such as master.
If no, write 'No'.
-->
No

---------

Co-authored-by: Xin Huang <[email protected]>
@allisonport-db
Copy link
Collaborator Author

Expression support added in #4007 but (2) is still pending

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@krishnanravi krishnanravi

Labels
enhancement New feature or request good first issue Good for newcomers kernel
Projects
None yet
Milestone
3.4.0
Development

No branches or pull requests
3 participants
@vkorukanti @krishnanravi @allisonport-db