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

Disable average aggregation pushdown on decimal type in ClickHouse #10703

Merged
merged 2 commits into from
Jan 20, 2022
Merged

Disable average aggregation pushdown on decimal type in ClickHouse #10703

merged 2 commits into from
Jan 20, 2022

Conversation

ebyhr
Copy link
Member

@ebyhr ebyhr commented Jan 20, 2022

Fixes #10650

I couldn't find the way to get the same result between new ClickHouse versions and Trino.

@cla-bot cla-bot bot added the cla-signed label Jan 20, 2022
@github-actions github-actions bot added the docs label Jan 20, 2022
@ebyhr ebyhr force-pushed the ebi/clickhouse-21.3 branch from 18cae88 to 37ba751 Compare January 20, 2022 08:10
ebyhr added 2 commits January 20, 2022 17:44
@ebyhr
Disable average aggregation pushdown on decimal type in ClickHouse
a29bdfe 
It returns the expected result in 20.8 (31 March 2021 EOL),
but it returns the wrong result in new versions.
@ebyhr
Upgrade minimum required ClickHouse to 21.3
d94ea35 
EOL schedule:
20.3: 31 March 2021
20.8: 31 August 2021
@ebyhr ebyhr force-pushed the ebi/clickhouse-21.3 branch from 37ba751 to d94ea35 Compare January 20, 2022 08:44
@ebyhr ebyhr requested a review from hashhar January 20, 2022 09:35
hashhar
hashhar approved these changes Jan 20, 2022
View reviewed changes
Copy link
Member

@hashhar hashhar left a comment

Choose a reason for hiding this comment

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

LGTM.

I'll investigate/ask upstream if there are functions to emulate Trino behaviour with newer versions. e.g. I see there is a roundBankers but I haven't tried.

@ebyhr
Copy link
Member Author

ebyhr commented Jan 20, 2022 •
edited
Loading

Already tried roundBankers, but it didn't work either.

@ebyhr ebyhr merged commit 77523b5 into trinodb:master Jan 20, 2022
@ebyhr ebyhr deleted the ebi/clickhouse-21.3 branch January 20, 2022 10:48
@ebyhr ebyhr changed the title Disable average aggreagtion pushdown on decimal type in ClickHouse Disable average aggregation pushdown on decimal type in ClickHouse Jan 20, 2022
@ebyhr ebyhr mentioned this pull request Jan 20, 2022
Closed
@github-actions github-actions bot added this to the 369 milestone Jan 20, 2022
@mosabua mosabua mentioned this pull request Jan 20, 2022
Merged
@tangjiangling tangjiangling mentioned this pull request Jan 31, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mosabua mosabua mosabua left review comments

@hashhar hashhar hashhar approved these changes

@wendigo wendigo wendigo approved these changes

Assignees
No one assigned
Labels
cla-signed docs
Milestone
369
Development

Successfully merging this pull request may close these issues.

Incorrect average decimal pushdown in new ClickHouse versions
4 participants
@ebyhr @wendigo @mosabua @hashhar