Skip to content

Fix expression precedence while preserving explicit parentheses - #326

Merged
git-hulk merged 4 commits into
masterfrom
codex/fix-expression-precedence
Sep 14, 2026
Merged

git-hulk merged 4 commits into
masterfrom
codex/fix-expression-precedence

Conversation

@git-hulk

@git-hulk git-hulk commented Sep 14, 2026

Copy link
Copy Markdown
Member

Problem

Unary signs bind before postfix operations, comparisons and membership bind at different levels, and cast targets consume following array access. These produce expression trees that disagree with ClickHouse. BETWEEN bounds also reject comparisons that ClickHouse accepts.

Reproduction

SELECT -tuple(1, 2).1;
-- Previously: (-tuple(1, 2)).1; ClickHouse: -(tuple(1, 2).1)
SELECT a = b IN (1);
-- Previously: a = (b IN (1)); ClickHouse: (a = b) IN (1)
SELECT a::Int64[1];
-- Previously: indexing attached to the type; ClickHouse: (a::Int64)[1]
SELECT a BETWEEN b = c AND d = e;
-- Comparisons must be accepted inside both bounds.

Fix

Centralize operator binding levels and correct unary/postfix, comparison/membership, cast, and BETWEEN parsing. Preserve public AST types and existing precedence constant values.

Formatting retains explicit parentheses from the SQL without inferring additional parentheses from the expression tree. Callers constructing or editing an AST must supply explicit grouping nodes when needed. Unary signs are written directly beside their operands, retaining a space between consecutive minus signs to avoid a line comment. Keyword operators retain their separating space.

Test

  • Add parser regressions for unary/postfix binding, comparison/BETWEEN binding, and cast-before-postfix access; correct existing membership expectations against ClickHouse.
  • Add TestFormatter_ExplicitExpressionParentheses to check source parentheses and unary-sign spacing in compact and beautified formatting, including nested negation and signed numeric operands, and TestFormatter_EditedExpressionRequiresExplicitParentheses to verify the caller supplies grouping.
  • Add a focused SQL fixture with AST, compact, and beautified golden outputs. Existing golden outputs are unchanged.
  • Validate 45 expression cases against ClickHouse Local 26.7.1.1315, comparing original SQL with explicitly grouped parsed trees and both formatting modes: 119 distinct SQL variants, zero mismatches.
  • Pass make test (race and compatibility checks) and make lint (CI-pinned golangci-lint v1.53.3) with Go 1.21.13.

Bind unary signs after postfix access and casts, associate comparisons
and membership at the same level, and allow comparisons in BETWEEN
bounds. Keep postfix access outside cast targets.

Share operator precedence with the formatter and add parentheses where
needed to preserve constructed or edited expression trees in both modes.
Keep existing public AST types and precedence constant values.

Add parser, formatter, edited-AST, and golden regression tests. Verify 45
expression cases against ClickHouse 26.7.1.1315. Pass make test, make lint,
and make with Go 1.21.13.
@git-hulk git-hulk changed the title Fix expression precedence and preserve formatting grouping Fix expression precedence while preserving explicit parentheses Sep 14, 2026
Comment thread parser/testdata/query/format/beautify/select_expression_precedence.sql Outdated
@git-hulk
git-hulk marked this pull request as ready for review September 14, 2026 06:10
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 14, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-14T06:19:28.075913Z 179fe73 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@git-hulk
git-hulk merged commit 236dc12 into master Sep 14, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant