-
Notifications
You must be signed in to change notification settings - Fork 447
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
[CORE][VL] Add OffloadProject to offload project having input_file_name's support considered #6200
[CORE][VL] Add OffloadProject to offload project having input_file_name's support considered #6200
Conversation
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?
See also: |
Run Gluten Clickhouse CI |
05ac11e
to
a6eb7d8
Compare
Run Gluten Clickhouse CI |
a6eb7d8
to
4d81d8a
Compare
Run Gluten Clickhouse CI |
@zhztheplayer i checked the spark code if a project node has I have follow your suggestion to modify |
backends-velox/src/main/scala/org/apache/gluten/backendsapi/velox/VeloxBackend.scala
Outdated
Show resolved
Hide resolved
gluten-core/src/main/scala/org/apache/gluten/extension/columnar/OffloadSingleNode.scala
Outdated
Show resolved
Hide resolved
gluten-core/src/main/scala/org/apache/gluten/extension/columnar/OffloadSingleNode.scala
Show resolved
Hide resolved
gluten-core/src/main/scala/org/apache/gluten/extension/columnar/OffloadSingleNode.scala
Show resolved
Hide resolved
shims/spark32/src/main/scala/org/apache/gluten/sql/shims/spark32/Spark32Shims.scala
Show resolved
Hide resolved
shims/spark33/src/main/scala/org/apache/gluten/sql/shims/spark33/Spark33Shims.scala
Show resolved
Hide resolved
gluten-core/src/main/scala/org/apache/gluten/extension/columnar/OffloadSingleNode.scala
Outdated
Show resolved
Hide resolved
gluten-core/src/main/scala/org/apache/gluten/extension/columnar/OffloadSingleNode.scala
Outdated
Show resolved
Hide resolved
gluten-core/src/main/scala/org/apache/gluten/extension/columnar/OffloadSingleNode.scala
Outdated
Show resolved
Hide resolved
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.
Almost in nice shape. Thank you for working on this.
gluten-core/src/main/scala/org/apache/gluten/extension/columnar/OffloadSingleNode.scala
Outdated
Show resolved
Hide resolved
gluten-core/src/main/scala/org/apache/gluten/extension/columnar/OffloadSingleNode.scala
Outdated
Show resolved
Hide resolved
gluten-core/src/main/scala/org/apache/gluten/extension/columnar/OffloadSingleNode.scala
Show resolved
Hide resolved
gluten-core/src/main/scala/org/apache/gluten/extension/columnar/OffloadSingleNode.scala
Outdated
Show resolved
Hide resolved
Run Gluten Clickhouse CI |
gluten-ut/spark35/src/test/scala/org/apache/spark/sql/GlutenColumnExpressionSuite.scala
Show resolved
Hide resolved
Run Gluten Clickhouse CI |
kindly ping @zhztheplayer / @xumingming |
gluten-core/src/main/scala/org/apache/gluten/extension/columnar/OffloadSingleNode.scala
Outdated
Show resolved
Hide resolved
any new comments @zhztheplayer / @xumingming thanks ? |
In case you missed the latest comments #6200 (comment) |
…xiaozhu/gluten into gayangya/offload_project
Run Gluten Clickhouse CI |
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.
Thanks. Waiting for CI.
Run Gluten Clickhouse CI |
looks CH still fail after re-trigger, i just sync the latest code to re-run the CI @PHILO-HE / @zhztheplayer |
nit: There is typo in the PR title, better fix it: |
@zhztheplayer could you help merge ? |
Run Gluten Clickhouse CI |
1 similar comment
Run Gluten Clickhouse CI |
Sure. Let's wait for remaining CI. |
@zhztheplayer the CI is passed. |
===== Performance report for TPCDS SF2000 with Velox backend, for reference only ====
|
===== Performance report for TPCDS SF2000 with Velox backend, for reference only ====
|
===== Performance report for TPCH SF2000 with Velox backend, for reference only ====
|
What changes were proposed in this pull request?
This PR is used to improve the logic to try to offload
project
withinput_file_name
to native for velox backend.It basically follow the suggestion of @zhztheplayer to extend
projectExec
offload logic.The basic full implement logic of
offloadProject
isProjectExec
is transformable, then it can be offloaded.ProjectExec
is not transformable:If it doesn't have
input_file_name
expression, then it can't be offloaded.If it has
input_file_name
:If it is still not transformable after removing
input_file_name
, then it can't be offloaded.If it is transformable after removing
input_file_name
:If it has no scan child or the scan child is not transformable, then it can't be offloaded.
If it has a scan child and the scan child is transformable, then it can be offloaded after converting
project + scan
to replaceinput_file_name
with the related_metadata
column.(Please fill in changes proposed in this fix)
(Fixes: #6157)
How was this patch tested?
manually & ut
(If this patch involves UI changes, please attach a screenshot; otherwise, remove this)