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

Choose correct aggregation method for lc128 and lc256 #42342

Merged
merged 7 commits into from Oct 18, 2022

Conversation

canhld94
Copy link
Contributor

@canhld94 canhld94 commented Oct 16, 2022

Changelog category (leave one):

  • Bug Fix

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):

  • Choose correct aggregation method for LowCardinality with BigInt

Close #42330

Information about CI checks: https://clickhouse.com/docs/en/development/continuous-integration/

@robot-clickhouse robot-clickhouse added the pr-improvement Pull request with some product improvements label Oct 16, 2022
@vdimir vdimir added can be tested Allows running workflows for external contributors pr-bugfix Pull request with bugfix, not backported by default and removed pr-improvement Pull request with some product improvements labels Oct 16, 2022
Copy link
Member

@vdimir vdimir left a comment

Choose a reason for hiding this comment

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

👍

@vdimir vdimir self-assigned this Oct 16, 2022
@alexey-milovidov alexey-milovidov added the pr-must-backport Pull request should be backported intentionally. Use this label with great care! label Oct 16, 2022
@alexey-milovidov
Copy link
Member

@azat @vdimir any idea why it didn't simply use AggregatedDataVariants::Type::serialized?

@alexey-milovidov alexey-milovidov changed the title Choose correct aggregation method for lc128 and lc512 Choose correct aggregation method for lc128 and lc256 Oct 16, 2022
@azat
Copy link
Collaborator

azat commented Oct 16, 2022

any idea why it didn't simply use AggregatedDataVariants::Type::serialized?

There is optimization for LowCardinality , that had been added in #3138

@canhld94
Copy link
Contributor Author

Fail cases:

  • Stateless: 00463_long_sessions_in_http_interface
  • msan: OOM kill, not sure...
  • tsan: backward compatibility,
  • ubsan: backward compatibility, cannot attach table

robot-clickhouse pushed a commit that referenced this pull request Oct 18, 2022
robot-clickhouse pushed a commit that referenced this pull request Oct 18, 2022
@robot-clickhouse robot-clickhouse added the pr-backports-created Backport PRs are successfully created, it won't be processed by CI script anymore label Oct 18, 2022
alexey-milovidov added a commit that referenced this pull request Oct 18, 2022
Backport #42342 to 22.8: Choose correct aggregation method for lc128 and lc256
alexey-milovidov added a commit that referenced this pull request Oct 18, 2022
Backport #42342 to 22.9: Choose correct aggregation method for lc128 and lc256
alexey-milovidov added a commit that referenced this pull request Oct 18, 2022
Backport #42342 to 22.7: Choose correct aggregation method for lc128 and lc256
alexey-milovidov added a commit that referenced this pull request Oct 18, 2022
Backport #42342 to 22.3: Choose correct aggregation method for lc128 and lc256
ucasfl pushed a commit to ucasfl/ClickHouse that referenced this pull request Nov 30, 2022
ucasfl pushed a commit to ucasfl/ClickHouse that referenced this pull request Nov 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
can be tested Allows running workflows for external contributors pr-backports-created Backport PRs are successfully created, it won't be processed by CI script anymore pr-bugfix Pull request with bugfix, not backported by default pr-must-backport Pull request should be backported intentionally. Use this label with great care!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

use-of-uninitialized-value in DB::Aggregator
5 participants