Skip to content

Fix aggregated projections#90288

Merged
antaljanosbenjamin merged 9 commits intomasterfrom
fix-aggregated-projections
Nov 24, 2025
Merged

Fix aggregated projections#90288
antaljanosbenjamin merged 9 commits intomasterfrom
fix-aggregated-projections

Conversation

@antaljanosbenjamin
Copy link
Copy Markdown
Member

@antaljanosbenjamin antaljanosbenjamin commented Nov 18, 2025

Changelog category (leave one):

  • Bug Fix (user-visible misbehavior in an official stable release)

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

Fix some queries with aggregated projection optimization.

Documentation entry for user-facing changes

  • Documentation is written (mandatory for new features)

@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh bot commented Nov 18, 2025

Workflow [PR], commit [833d2df]

Summary:

job_name test_name status info comment
Stateless tests (arm_asan, targeted) failure
02151_lc_prefetch FAIL cidb
01459_manual_write_to_replicas_quorum_detach_attach FAIL cidb, flaky
01459_manual_write_to_replicas_quorum_detach_attach FAIL cidb, flaky
01459_manual_write_to_replicas_quorum_detach_attach FAIL cidb, flaky
01459_manual_write_to_replicas_quorum_detach_attach FAIL cidb, flaky
Stateless tests (amd_msan, parallel) failure
03403_mv_errors FAIL cidb
00596_limit_on_expanded_ast FAIL cidb
Integration tests (amd_asan, db disk, old analyzer, 3/6) failure
test_restore_db_replica/test.py::test_restore_db_replica_with_diffrent_table_metadata[restore node1-node2] FAIL cidb, flaky
Integration tests (amd_tsan, 1/6) failure
test_storage_s3_queue/test_parallel_inserts.py::test_parallel_inserts_with_failures[1] FAIL cidb, flaky
Stress test (amd_ubsan) failure
Server died FAIL cidb
Hung check failed, possible deadlock found (see hung_check.log) FAIL cidb
Killed by signal (in clickhouse-server.log) FAIL cidb
Fatal message in clickhouse-server.log (see fatal_messages.txt) FAIL cidb
Killed by signal (output files) FAIL cidb
Found signal in gdb.log FAIL cidb
BuzzHouse (amd_debug) failure
Logical error: 'Inconsistent AST formatting: the query: FAIL cidb
BuzzHouse (amd_tsan) failure
Logical error: '(isConst() isSparse()
BuzzHouse (amd_msan) failure
Received signal 17 FAIL cidb
Performance Comparison (amd_release, master_head, 3/6) failure
Start failure

@clickhouse-gh clickhouse-gh bot added the pr-bugfix Pull request with bugfix, not backported by default label Nov 18, 2025
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a bug in aggregated projection optimization where virtual columns were being added incorrectly to the ActionsDAG, causing crashes or incorrect behavior in certain queries involving projections with joins and window functions.

Key changes:

  • Modified how virtual columns are added to the ActionsDAG in projection optimization to use name and type separately instead of passing a NameAndTypePair object
  • Added a temporary safeguard in ActionsDAG::addNode to prevent invalid column objects from being stored
  • Added a regression test with a complex query involving CTEs, window functions, and joins

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
src/Processors/QueryPlan/Optimizations/optimizeUseAggregateProjection.cpp Fixed virtual column input addition to use separate name and type parameters
src/Interpreters/ActionsDAG.cpp Added validation to clear invalid column objects in non-placeholder nodes
tests/queries/0_stateless/03721_aggregate_projection_actions_dag.sql Added regression test with complex query involving CTEs, window functions and joins
tests/queries/0_stateless/03721_aggregate_projection_actions_dag.reference Expected output showing the query plan structure

@antaljanosbenjamin
Copy link
Copy Markdown
Member Author

@antaljanosbenjamin antaljanosbenjamin added this pull request to the merge queue Nov 20, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 20, 2025
@antaljanosbenjamin
Copy link
Copy Markdown
Member Author

@antaljanosbenjamin antaljanosbenjamin added this pull request to the merge queue Nov 24, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 24, 2025
@antaljanosbenjamin
Copy link
Copy Markdown
Member Author

Test failures are not related.

@antaljanosbenjamin antaljanosbenjamin added this pull request to the merge queue Nov 24, 2025
Merged via the queue into master with commit 09af580 Nov 24, 2025
120 of 130 checks passed
@antaljanosbenjamin antaljanosbenjamin deleted the fix-aggregated-projections branch November 24, 2025 18:00
@robot-ch-test-poll1 robot-ch-test-poll1 added the pr-must-backport-synced The `*-must-backport` labels are synced into the cloud Sync PR label Nov 24, 2025
robot-ch-test-poll3 added a commit that referenced this pull request Nov 24, 2025
Cherry pick #90288 to 25.9: Fix aggregated projections
robot-ch-test-poll3 added a commit that referenced this pull request Nov 24, 2025
Cherry pick #90288 to 25.10: Fix aggregated projections
robot-ch-test-poll3 added a commit that referenced this pull request Nov 24, 2025
Cherry pick #90288 to 25.11: Fix aggregated projections
@robot-clickhouse-ci-2 robot-clickhouse-ci-2 added the pr-synced-to-cloud The PR is synced to the cloud repo label Nov 24, 2025
@robot-ch-test-poll3 robot-ch-test-poll3 added the pr-backports-created Backport PRs are successfully created, it won't be processed by CI script anymore label Nov 24, 2025
nikitamikhaylov added a commit that referenced this pull request Nov 24, 2025
Backport #90288 to 25.10: Fix aggregated projections
antaljanosbenjamin added a commit that referenced this pull request Nov 26, 2025
Backport #90288 to 25.11: Fix aggregated projections
clickhouse-gh bot added a commit that referenced this pull request Nov 26, 2025
Backport #90288 to 25.9: Fix aggregated projections
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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-synced The `*-must-backport` labels are synced into the cloud Sync PR pr-synced-to-cloud The PR is synced to the cloud repo v25.10-must-backport v25.11-must-backport

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants