Skip to content

[GLUTEN-6666][VL] Use custom SparkExprToSubfieldFilterParser#6754

Merged
rui-mo merged 1 commit intoapache:mainfrom
rui-mo:wip_separate
Nov 13, 2024
Merged

[GLUTEN-6666][VL] Use custom SparkExprToSubfieldFilterParser#6754
rui-mo merged 1 commit intoapache:mainfrom
rui-mo:wip_separate

Conversation

@rui-mo
Copy link
Copy Markdown
Contributor

@rui-mo rui-mo commented Aug 8, 2024 •
edited
Loading

What changes were proposed in this pull request?

Removes separating filter relevant code from Gluten. With a custom filter parser registered, we are able to use Velox provided filter extraction.
Fixes #6666.

How was this patch tested?

Verified the functionality and performance on TPC-DS, TPC-H queries.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Aug 8, 2024

Thanks for opening a pull request!

Could you open an issue for this pull request on Github Issues?

https://github.com/apache/incubator-gluten/issues

Then could you also rename commit message and pull request title in the following format?

[GLUTEN-${ISSUES_ID}][COMPONENT]feat/fix: ${detailed message}

See also:

@FelixYBW
Copy link
Copy Markdown
Contributor

FelixYBW commented Aug 9, 2024

@zhli1142015

@rui-mo how many PRs we need to submit to Velox? since there is a gap between Gluten's

@rui-mo
Copy link
Copy Markdown
Contributor Author

rui-mo commented Aug 15, 2024

@rui-mo how many PRs we need to submit to Velox? since there is a gap between Gluten's

@FelixYBW I'm still evaluating the gap here. Will update once it's figured out. Thanks.

@rui-mo rui-mo force-pushed the wip_separate branch 3 times, most recently from fcaacf7 to 857d4c4 Compare August 20, 2024 06:11
@zhztheplayer zhztheplayer mentioned this pull request Sep 18, 2024
Closed
@rui-mo rui-mo force-pushed the wip_separate branch 3 times, most recently from 4ab8080 to 7a18f66 Compare October 12, 2024 02:16
@rui-mo rui-mo force-pushed the wip_separate branch 2 times, most recently from d728c2d to f7cda2a Compare October 23, 2024 08:25
@rui-mo rui-mo marked this pull request as ready for review October 29, 2024 01:54
@rui-mo
Copy link
Copy Markdown
Contributor Author

rui-mo commented Nov 6, 2024

@GlutenPerfBot benchmark

@FelixYBW
Copy link
Copy Markdown
Contributor

FelixYBW commented Nov 6, 2024

@FelixYBW I'm still evaluating the gap here. Will update once it's figured out. Thanks.

Any update?

@rui-mo
Copy link
Copy Markdown
Contributor Author

rui-mo commented Nov 8, 2024

Any update?

@FelixYBW The needed changes on Velox are not much, mainly including the below two commits, and the PRs for them were both opened to Velox. To clean-up Gluten code sooner, we might merge this PR to Gluten first. Thanks.
https://github.com/rui-mo/velox/commit/8f9a34144f9836d2f8cfbb60fd12b5492a36d8a3
https://github.com/rui-mo/velox/commit/faac5d08a8736f96f718b4f37deb70ba6d4a43d9

@FelixYBW
Copy link
Copy Markdown
Contributor

FelixYBW commented Nov 8, 2024

@FelixYBW The needed changes on Velox are not much, mainly including the below two commits, and the PRs for them were both opened to Velox. To clean-up Gluten code sooner, we might merge this PR to Gluten first. Thanks. rui-mo/velox@8f9a341 rui-mo/velox@faac5d0

can you put the velox PR ID here?

@rui-mo rui-mo changed the title [VL] Use extractFiltersFromRemainingFilter provided by Velox [GLUTEN-6666][VL] Use extractFiltersFromRemainingFilter provided by Velox Nov 12, 2024
@github-actions
Copy link
Copy Markdown

github-actions bot commented Nov 12, 2024

#6666

@rui-mo rui-mo force-pushed the wip_separate branch 3 times, most recently from 039124b to 3ca491f Compare November 13, 2024 02:28
@rui-mo rui-mo changed the title [GLUTEN-6666][VL] Use extractFiltersFromRemainingFilter provided by Velox [GLUTEN-6666][VL] Use custom SparkExprToSubfieldFilterParser Nov 13, 2024
zhli1142015
zhli1142015 reviewed Nov 13, 2024
View reviewed changes
ep/build-velox/src/get_velox.sh Outdated

VELOX_REPO=https://github.com/oap-project/velox.git
VELOX_BRANCH=2024_11_12
VELOX_BRANCH=2024_11_12_1
Copy link
Copy Markdown
Contributor

@zhli1142015 zhli1142015 Nov 13, 2024

Choose a reason for hiding this comment

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

Is this intended change?

Copy link
Copy Markdown
Contributor Author

@rui-mo rui-mo Nov 13, 2024

Choose a reason for hiding this comment

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

Yes, it avoids pushing to the previous branch.

@rui-mo
Copy link
Copy Markdown
Contributor Author

rui-mo commented Nov 13, 2024

can you put the velox PR ID here?

Added the PR ID.

@rui-mo rui-mo merged commit d6326f0 into apache:main Nov 13, 2024
@zml1206 zml1206 mentioned this pull request Nov 14, 2024
Merged
@rui-mo rui-mo mentioned this pull request Mar 4, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@zhli1142015 zhli1142015 zhli1142015 left review comments

@philo-he philo-he philo-he approved these changes

Assignees

No one assigned

Labels

BUILD VELOX

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[VL] Switch to use extractFiltersFromRemainingFilter in Velox

4 participants

@rui-mo @FelixYBW @zhli1142015 @philo-he