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.

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

[GLUTEN-6951][CORE][CH] Move CustomerExpressionTransformer to CH backend #6993

Merged
merged 5 commits into from
Aug 26, 2024

Conversation

zhztheplayer
Copy link
Member

@zhztheplayer zhztheplayer commented Aug 23, 2024

CustomerExpressionTransformer was added to core module in #1937 however is not used by Velox backend for long time. Propose to move its code and planner rules to CH backend.

Changes:

  1. Drop support of config option a.g.s.c.extended.expressions.transformer from Velox backend
  2. Move code related to CustomerExpressionTransformer from core to CH backend
  3. Essential code refactors

@github-actions github-actions bot added CORE works for Gluten Core CLICKHOUSE labels Aug 23, 2024
Copy link

#6951

Copy link

Run Gluten Clickhouse CI

@zhztheplayer
Copy link
Member Author

@liujiayi771 @Yohahaha @ulysses-you @kecookier @zhli1142015 (and all Velox backend users/developers)

Please let me know if you rely on the feature. Otherwise we'll remove then.

@zhztheplayer zhztheplayer requested a review from zzcclp August 23, 2024 07:46
@zhztheplayer
Copy link
Member Author

@zzcclp What do you think about this? Thanks.

@Yohahaha
Copy link
Contributor

We are not rely on this functionality for now.

Copy link

Run Gluten Clickhouse CI

@ulysses-you
Copy link
Contributor

+1

Copy link

Run Gluten Clickhouse CI

@baibaichen
Copy link
Contributor

let me move test to backends clickhouse first.

@liujiayi771
Copy link
Contributor

+1

@zzcclp
Copy link
Contributor

zzcclp commented Aug 26, 2024

please move spark32/src/test/scala/org/apache/spark/sql/extension/CustomerExpressionTransformer.scala, spark32/src/test/scala/org/apache/spark/sql/extension/GlutenCustomerExpressionTransformerSuite.scala to CH backend module too.

fixup

fixup
Copy link

Run Gluten Clickhouse CI

@zhztheplayer
Copy link
Member Author

please move spark32/src/test/scala/org/apache/spark/sql/extension/CustomerExpressionTransformer.scala, spark32/src/test/scala/org/apache/spark/sql/extension/GlutenCustomerExpressionTransformerSuite.scala to CH backend module too.

I am moving it. It caused Velox's test failure.

Copy link

Run Gluten Clickhouse CI

Copy link

Run Gluten Clickhouse CI

@zhztheplayer zhztheplayer marked this pull request as ready for review August 26, 2024 05:17
@zhztheplayer
Copy link
Member Author

Run Gluten Clickhouse CI

Copy link
Contributor

@zzcclp zzcclp left a comment

Choose a reason for hiding this comment

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

LGTM

@zhztheplayer zhztheplayer merged commit 9bb4b28 into apache:main Aug 26, 2024
41 of 43 checks passed
@zhztheplayer
Copy link
Member Author

@zzcclp Thanks!

sharkdtu pushed a commit to sharkdtu/gluten that referenced this pull request Nov 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLICKHOUSE CORE works for Gluten Core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants