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-3644][CH] Revert the logic to support the custom aggregate functions #3648

Merged
merged 1 commit into from
Nov 8, 2023

Conversation

zzcclp
Copy link
Contributor

@zzcclp zzcclp commented Nov 8, 2023

What changes were proposed in this pull request?

In PR #3629, it removes the logic to support the custom aggregate functions, must be reverted.

Close #3644.

(Fixes: #3644)

How was this patch tested?

(Please explain how this patch was tested. E.g. unit tests, integration tests, manual tests)

(If this patch involves UI changes, please attach a screenshot; otherwise, remove this)

Copy link

github-actions bot commented Nov 8, 2023

#3644

Copy link

github-actions bot commented Nov 8, 2023

Run Gluten Clickhouse CI

Copy link
Contributor

@rui-mo rui-mo left a comment

Choose a reason for hiding this comment

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

Is it feasible to add some unit test to ensure the functionality?

@ulysses-you
Copy link
Contributor

It seems only affect CH backend. yeah, one more ut is better to avoid regression in future.

…nctions

In PR apache#3629, it removes the logic to support the custom aggregate functions, must be reverted.

Close apache#3644.
Copy link

github-actions bot commented Nov 8, 2023

Run Gluten Clickhouse CI

@zzcclp
Copy link
Contributor Author

zzcclp commented Nov 8, 2023

Add a ut to make sure that it must use custome agg transformer to run. @rui-mo @ulysses-you

@zzcclp
Copy link
Contributor Author

zzcclp commented Nov 8, 2023

ping @ulysses-you @rui-mo

@zzcclp zzcclp merged commit a090556 into apache:main Nov 8, 2023
17 checks passed
@GlutenPerfBot
Copy link
Contributor

===== Performance report for TPCH SF2000 with Velox backend, for reference only ====

query log/native_3648_time.csv log/native_master_11_07_2023_e3eff1d8f_time.csv difference percentage
q1 33.50 34.38 0.877 102.62%
q2 24.89 25.03 0.135 100.54%
q3 38.33 38.14 -0.182 99.52%
q4 36.22 37.57 1.351 103.73%
q5 71.53 71.50 -0.026 99.96%
q6 7.95 6.26 -1.693 78.71%
q7 85.68 82.22 -3.452 95.97%
q8 88.04 86.95 -1.097 98.75%
q9 121.66 119.81 -1.848 98.48%
q10 52.71 51.26 -1.445 97.26%
q11 19.45 19.73 0.283 101.46%
q12 26.22 24.39 -1.834 93.01%
q13 49.22 50.30 1.074 102.18%
q14 18.82 17.67 -1.158 93.85%
q15 30.76 30.35 -0.401 98.70%
q16 16.28 16.20 -0.083 99.49%
q17 100.46 101.51 1.043 101.04%
q18 148.06 148.26 0.201 100.14%
q19 14.82 16.17 1.353 109.13%
q20 30.07 30.31 0.241 100.80%
q21 226.34 224.88 -1.462 99.35%
q22 13.19 14.08 0.892 106.76%
total 1254.22 1246.98 -7.234 99.42%

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CH] Revert the logic to support the custom aggregate functions
4 participants