Skip to content

Fix rows_before_aggregation for aggregation in order#88117

Merged
yariks5s merged 1 commit intoClickHouse:masterfrom
korowa:fix-gby-in-order-rc
Oct 14, 2025
Merged

Fix rows_before_aggregation for aggregation in order#88117
yariks5s merged 1 commit intoClickHouse:masterfrom
korowa:fix-gby-in-order-rc

Conversation

@korowa
Copy link
Copy Markdown
Contributor

@korowa korowa commented Oct 6, 2025

Closes #87927

Current implementation sets rows_before_aggregation to consumed chunk number of rows. In cases when in order aggregation block in limited, and transformation splits the chunk to process it in more than one iteration, the counter starts to double account input rows.

Changelog category (leave one):

  • Not for changelog (changelog entry is not required)

@yariks5s yariks5s added the can be tested Allows running workflows for external contributors label Oct 6, 2025
@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh bot commented Oct 6, 2025

Workflow [PR], commit [faf10fa]

Summary:

job_name test_name status info comment
Stateless tests (amd_tsan, parallel, 2/2) failure
03212_variant_dynamic_cast_or_default FAIL

@clickhouse-gh clickhouse-gh bot added the pr-not-for-changelog This PR should not be mentioned in the changelog label Oct 6, 2025
@yariks5s yariks5s self-assigned this Oct 6, 2025
Current implementation sets rows_before_aggregation to consumed
chunk number of rows. In cases when in order aggregation block
in limited, and transformation splits the chunk to process it in
more than one iteration, the counter starts to double account
input rows.
@korowa korowa force-pushed the fix-gby-in-order-rc branch from 7651934 to faf10fa Compare October 7, 2025 08:44
Copy link
Copy Markdown
Member

@yariks5s yariks5s left a comment

Choose a reason for hiding this comment

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

Looks good, looks like we can do the same with src_bytes, but we can do it later

@yariks5s
Copy link
Copy Markdown
Member

03212_variant_dynamic_cast_or_default was flaky before: #87583

@yariks5s yariks5s added this pull request to the merge queue Oct 14, 2025
Merged via the queue into ClickHouse:master with commit d134479 Oct 14, 2025
121 of 123 checks passed
@robot-clickhouse-ci-1 robot-clickhouse-ci-1 added the pr-synced-to-cloud The PR is synced to the cloud repo label Oct 14, 2025
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-not-for-changelog This PR should not be mentioned in the changelog pr-synced-to-cloud The PR is synced to the cloud repo

Projects

None yet

Development

Successfully merging this pull request may close these issues.

03174_exact_rows_before_aggregation is flaky

3 participants