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

[GLUTEN-8492][CH] Offload RangeExec #8518

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

[GLUTEN-8492][CH] Offload RangeExec #8518

wants to merge 12 commits into from

Conversation

taiyang-li
Copy link
Contributor

@taiyang-li taiyang-li commented Jan 13, 2025 •
edited
Loading

What changes were proposed in this pull request?

(Please fill in changes proposed in this fix)

(Fixes: #8492)

How was this patch tested?

All existing uts using range function. Below ut is excluded because assertion that plan contains RangeExec operator would fail after RangeExec is offloaded to gluten.

  • "SPARK-27439: Explain result should match collected result after view change"

@github-actions github-actions bot added CORE works for Gluten Core VELOX CLICKHOUSE labels Jan 13, 2025
@github-actions GitHub Actions
Copy link

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:

@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI on x86

1 similar comment
@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI on x86

@taiyang-li taiyang-li changed the title [WIP] Support range functions [CH] Support range functions Jan 13, 2025
@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI on x86

1 similar comment
@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI on x86

@taiyang-li taiyang-li changed the title [CH] Support range functions [GLUTEN-8492][CH] Support range functions Jan 14, 2025
@taiyang-li taiyang-li marked this pull request as ready for review January 14, 2025 04:14
@github-actions GitHub Actions
Copy link

#8492

@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI on x86

@taiyang-li taiyang-li requested a review from baibaichen January 14, 2025 05:18
@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI on x86

2 similar comments
@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI on x86

@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI on x86

@taiyang-li taiyang-li changed the title [GLUTEN-8492][CH] Support range functions [GLUTEN-8492][CH] Offload RangeExec Jan 14, 2025
@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI on x86

1 similar comment
@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI on x86

@zhouyuan zhouyuan mentioned this pull request Jan 16, 2025
Merged
@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI on x86

2 similar comments
@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI on x86

@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI on x86

@taiyang-li taiyang-li requested review from zhztheplayer and PHILO-HE and removed request for PHILO-HE February 11, 2025 04:18
@taiyang-li
Copy link
Contributor Author

For performance comparison refer to #8492 (comment)

@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI on x86

@CodiumAI-Agent
Copy link

Persistent review updated to latest commit febd663

@taiyang-li
Copy link
Contributor Author

Run Gluten Clickhouse CI on x86

PHILO-HE
PHILO-HE reviewed Feb 12, 2025
View reviewed changes
Copy link
Contributor

@PHILO-HE PHILO-HE left a comment

Choose a reason for hiding this comment

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

Mainly reviewed the common code. Basically looks good!

backends-clickhouse/src/main/scala/org/apache/gluten/backendsapi/clickhouse/CHIteratorApi.scala Outdated Show resolved Hide resolved
backends-velox/src/main/scala/org/apache/gluten/backendsapi/velox/VeloxIteratorApi.scala Outdated Show resolved Hide resolved
backends-clickhouse/src/main/scala/org/apache/gluten/execution/CHRangeExecTransformer.scala Show resolved Hide resolved
gluten-substrait/src/main/scala/org/apache/gluten/backendsapi/IteratorApi.scala Outdated Show resolved Hide resolved
backends-clickhouse/src/main/scala/org/apache/gluten/execution/CHRangeExecTransformer.scala Outdated Show resolved Hide resolved
cpp-ch/local-engine/local_engine_jni.cpp Outdated Show resolved Hide resolved
gluten-substrait/src/main/scala/org/apache/gluten/execution/GlutenWholeStageColumnarRDD.scala Show resolved Hide resolved
gluten-substrait/src/main/scala/org/apache/gluten/execution/WholeStageTransformer.scala Outdated Show resolved Hide resolved
gluten-substrait/src/main/scala/org/apache/gluten/execution/WholeStageTransformer.scala Outdated Show resolved Hide resolved
@PHILO-HE
Copy link
Contributor

Maybe, we can move the existing GlutenSQLRangeExecSuite.scala from spark33 ut folder to gluten-ut/test folder, which can get it tested for all Spark versions and both backends.

@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI on x86

@taiyang-li
Copy link
Contributor Author

Maybe, we can move the existing GlutenSQLRangeExecSuite.scala from spark33 ut folder to gluten-ut/test folder, which can get it tested for all Spark versions and both backends.

done.

@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI on x86

2 similar comments
@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI on x86

@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI on x86

@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI on x86

@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI on x86

@github-actions GitHub Actions
Copy link

Run Gluten Clickhouse CI on x86

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

@PHILO-HE PHILO-HE PHILO-HE left review comments

@baibaichen baibaichen Awaiting requested review from baibaichen

@zhztheplayer zhztheplayer Awaiting requested review from zhztheplayer

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
CLICKHOUSE CORE works for Gluten Core VELOX
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[CH] sum with filter bad performance compared to vanilla spark
3 participants
@taiyang-li @CodiumAI-Agent @PHILO-HE