Fix inconsistent formatting by remembering if an expression was parenthesized by alexey-milovidov · Pull Request #92340 · ClickHouse/ClickHouse

alexey-milovidov · 2025-12-17T03:10:07Z

Changelog category (leave one):

CI Fix or Improvement (changelog entry is not required)

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

Fix inconsistent formatting by remembering if an expression was parenthesized.

See examples: https://pastila.nl/?012beb19/a57b360b2732ae0c828c9c364a9d67ce#wVKsIzKfqrLVizUOWpVuNw==

…hesed

…hesized

clickhouse-gh · 2025-12-17T03:10:36Z

Workflow [PR], commit [451882c]

Summary: ❌

job_name	test_name	status	info
Fast test		failure
	Build ClickHouse	failure
Build (arm_tidy)		failure
	Build ClickHouse	failure	cidb
Build (amd_debug)		dropped
Build (amd_asan_ubsan)		dropped
Build (amd_tsan)		dropped
Build (amd_msan)		dropped
Build (amd_binary)		dropped
Build (arm_debug)		dropped
Build (arm_asan_ubsan)		dropped
Build (arm_tsan)		dropped

AI Review

Summary

The PR makes formatting round-tripping more consistent by preserving whether expressions were explicitly parenthesized in IAST and propagating that through QueryTree conversion paths, with broad reference updates. The core implementation looks consistent after the follow-up fixes (need_parens restoration and IAST copy/assignment propagation). I found one remaining process-quality issue in PR metadata: the selected changelog category does not match this user-visible behavior fix.

PR Metadata

Changelog category is not semantically correct for this change. CI Fix or Improvement should be used for CI-only changes, while this PR changes user-visible SQL formatting behavior.
Changelog entry is present and specific enough.
Exact replacement text for category:
- - Bug Fix (user-visible misbehavior in an official stable release)

Findings

💡 Nits

[PR description / Changelog category] Category mismatch: this PR is a user-visible formatter behavior fix, not a CI-only change.
- Suggested fix: change Changelog category to Bug Fix (user-visible misbehavior in an official stable release).

ClickHouse Rules

Item	Status	Notes
Deletion logging	➖
Serialization versioning	➖
Core-area scrutiny	✅
No test removal	✅
Experimental gate	➖
No magic constants	✅
Backward compatibility	✅
`SettingsChangesHistory.cpp`	➖
PR metadata quality	⚠️	`Changelog category` should be `Bug Fix (user-visible misbehavior in an official stable release)` instead of `CI Fix or Improvement`.
Safe rollout	✅
Compilation time	✅

Final Verdict

Status: ⚠️ Request changes
Minimum required actions:
- Update PR Changelog category to Bug Fix (user-visible misbehavior in an official stable release).

…-an-expression-was-parenthesed

…lag through QueryTree This commit extends the parentheses preservation to work with EXPLAIN SYNTAX, which uses the Analyzer and converts AST -> QueryTree -> AST. Changes: - Add `parenthesized` flag to `IQueryTreeNode` base class with getter/setter - Propagate the flag when building QueryTree from AST in `QueryTreeBuilder` - Set the flag on converted AST in `IQueryTreeNode::toAST` - Copy the flag during `cloneAndReplace` - Add space between consecutive negate operators to avoid `--` ambiguity Now EXPLAIN SYNTAX preserves parentheses just like `clickhouse-format` and client echoing do. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

…-an-expression-was-parenthesed

…rsing-by-remembering-if-an-expression-was-parenthesed # Conflicts: # src/Parsers/ASTAssignment.h # src/Parsers/ASTFunction.cpp # src/Parsers/ASTSelectWithUnionQuery.cpp # tests/queries/0_stateless/03773_join_on_formatting.reference # tests/queries/0_stateless/03786_AST_formatting_inconsistencies_in_debug_check.reference

…ssion-was-parenthesed

…rsing-by-remembering-if-an-expression-was-parenthesed Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

clickhouse-gh · 2026-03-28T23:08:15Z

src/Parsers/IAST.cpp

+
+void IAST::formatImpl(WriteBuffer & ostr, const FormatSettings & settings, FormatState & state, FormatStateStacked frame) const
+{
+    if (parenthesized)


IAST::IAST(const IAST & other) does not copy the new parenthesized field. Since many AST nodes clone via make_intrusive<...>(*this), this drops parentheses information on clone paths and can reintroduce inconsistent formatting/round-tripping after transformations.

Please copy parenthesized in both the copy constructor and assignment operator:

add , parenthesized(other.parenthesized) in the constructor initializer list;

add parenthesized = other.parenthesized; in operator=.

…rsing-by-remembering-if-an-expression-was-parenthesed # Conflicts: # tests/queries/0_stateless/03701_optimize_inverse_dictionary_lookup_basic.reference

The field was accidentally removed from the struct definition while it is still referenced by 34 call sites across 10 files (ASTFunction, ASTCreateQuery, ASTTTLElement, etc.), causing the build to fail. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…ignment operator Without this, AST clones (via `make_intrusive<...>(*this)`) would lose the parenthesized flag, reintroducing inconsistent formatting. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

clickhouse-gh · 2026-03-30T08:41:45Z

src/Parsers/IAST.h

 public:

+    /// If the element has extra parentheses around it, e.g., in "a + (b)", b has extra parentheses.
+    bool parenthesized = false;


This change is user-visible (SELECT ... FORMAT SQL output and AST round-tripping behavior change), so the PR metadata should not use CI Fix or Improvement. Please switch Changelog category to Bug Fix (user-visible misbehavior in an official stable release) so users can discover this fix in release notes.

…-an-expression-was-parenthesed

alexey-milovidov added 2 commits December 17, 2025 04:08

Fix inconsistent parsing by remembering when an expression was parent…

c942545

…hesed

Fix inconsistent parsing by remembering when an expression was parent…

a80983d

…hesized

clickhouse-gh bot added the pr-ci label Dec 17, 2025

alexey-milovidov changed the title ~~Fix inconsistent parsing by remembering if an expression was parenthesized~~ Fix inconsistent formatting by remembering if an expression was parenthesized Dec 17, 2025

azat mentioned this pull request Dec 17, 2025

Fix inconsistent AST formatting of arrayElement with negate literal #92293

Merged

1 task

alexey-milovidov and others added 8 commits December 20, 2025 10:54

Merge branch 'master' into fix-inconsistent-parsing-by-remembering-if…

b88dfdb

…-an-expression-was-parenthesed

Fix an error

7bb3ca2

Fix an error

fabbe33

Merge branch 'master' into fix-inconsistent-parsing-by-remembering-if…

8e80602

…-an-expression-was-parenthesed

Fix build

4709410

Merge branch 'master' into fix-inconsistent-parsing-by-remembering-if…

a57ab85

…-an-expression-was-parenthesed

Merge branch 'master' into fix-inconsistent-parsing-by-remembering-if…

5ceaa6f

…-an-expression-was-parenthesed

alexey-milovidov mentioned this pull request Jan 19, 2026

EXPLAIN SYNTAX and IQueryTreeNode::toAST should format all operators as functions #94603

Open

alexey-milovidov and others added 5 commits January 19, 2026 16:44

Update references

d2bd59f

Merge master into fix-inconsistent-parsing-by-remembering-if-an-expre…

c3971bb

…ssion-was-parenthesed

Merge master into fix-inconsistent-parsing-by-remembering-if-an-expre…

2f53b9c

…ssion-was-parenthesed

Merge remote-tracking branch 'origin/master' into fix-inconsistent-pa…

b527e93

…rsing-by-remembering-if-an-expression-was-parenthesed Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

clickhouse-gh bot reviewed Mar 28, 2026

View reviewed changes

alexey-milovidov and others added 3 commits March 30, 2026 08:12

Merge remote-tracking branch 'origin/master' into fix-inconsistent-pa…

27f34ca

…rsing-by-remembering-if-an-expression-was-parenthesed # Conflicts: # tests/queries/0_stateless/03701_optimize_inverse_dictionary_lookup_basic.reference

clickhouse-gh bot reviewed Mar 30, 2026

View reviewed changes

Merge branch 'master' into fix-inconsistent-parsing-by-remembering-if…

451882c

…-an-expression-was-parenthesed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix inconsistent formatting by remembering if an expression was parenthesized#92340

Fix inconsistent formatting by remembering if an expression was parenthesized#92340
alexey-milovidov wants to merge 19 commits intomasterfrom
fix-inconsistent-parsing-by-remembering-if-an-expression-was-parenthesed

alexey-milovidov commented Dec 17, 2025 •

edited

Loading

Uh oh!

clickhouse-gh bot commented Dec 17, 2025 •

edited

Loading

Uh oh!

clickhouse-gh bot Mar 28, 2026

Uh oh!

clickhouse-gh bot Mar 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

alexey-milovidov commented Dec 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changelog category (leave one):

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

Uh oh!

clickhouse-gh bot commented Dec 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

AI Review

Summary

PR Metadata

Findings

ClickHouse Rules

Final Verdict

Uh oh!

clickhouse-gh bot Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

clickhouse-gh bot Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

alexey-milovidov commented Dec 17, 2025 •

edited

Loading

clickhouse-gh bot commented Dec 17, 2025 •

edited

Loading