{{ message }}
Self-deduplication for ActionsDAG#106113
Merged
Merged
Conversation
Contributor
clickhouse-gh
Bot
added
the
pr-improvement
![clickhouse-gh[bot]](https://avatars.githubusercontent.com/in/928874?s=60&v=4){kind=small}
george-larionov
approved these changes
May 29, 2026
george-larionov
left a comment
george-larionov
left a comment
Member
There was a problem hiding this comment.
It looks good to me overall (I saw you fixed the final (hopefully) issue that the bot found). However I think it is worth it to add a performance test to show some perf gains for this improvement.
Some functions consume their argument names (e.g. `formatRowNoNewline` puts column names into JSON keys, `toTypeName` returns names). Looking through ALIAS chains for those would falsely equate two calls over different aliases of the same input. Add an opt-in `isNameInsensitive()` on `IFunctionBase`. Default is false (conservative). Overridden to true in `FunctionsComparison`, `FunctionsLogical` and `FunctionBinaryArithmetic` since these depend only on argument values. `buildStructuralEquivalenceClasses` now looks through aliases only when the parent function declares `isNameInsensitive() = true`. Recovers the nested SELECT optimization while keeping name-sensitive functions safe. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Resolved conflicts in favor of the new `isNameInsensitive` API while keeping the remote's `query_plan_merge_filters=1` setting in the test. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
COLUMN nodes with the same value but different names are not interchangeable for name-sensitive parents (e.g. `formatRowNoNewline` uses argument names as JSON keys). Defensive fix: include `result_name` in the COLUMN dedup key so `SELECT 1 AS a, 1 AS b` stays as two distinct nodes. Auto-named consts (like `0_UInt8` from `WHERE x > 0`) keep deduplicating as before since their names also match. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 task
| }; | ||
|
|
||
| using FunctionOverloadResolverPtr = std::shared_ptr<IFunctionOverloadResolver>; | ||
| /// mutable beacuse it's set after construction by FunctionFactory, resolvers themselves are otherwise immutable |
Member
There was a problem hiding this comment.
As an option, we can change buildImpl to return std::shared_ptr<IFunctionBase> (or, even better, std::unique_ptr), and move it into FunctionBasePtr = std::shared_ptrduringbuild`.
I found about 35 places by searching. So let's do it in post-refactoring if needed.
KochetovNicolai
approved these changes
Jun 16, 2026
Contributor
LLVM Coverage ReportChanged lines: Changed C/C++ lines covered by tests: 206/213 (96.71%) | Lost baseline coverage (was covered on master, now uncovered in this PR): 5 line(s) · Uncovered code |
robot-clickhouse-ci-2
added
the
pr-synced-to-cloud
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):
Provides the logic to remove duplicate calculations during query execution in different scenarios.
Version info
26.6.1.982