From 4e2f0790cb0d5a0205d74b49a794a5bf17d8c54a Mon Sep 17 00:00:00 2001 From: basili4-1982 <1531789+basili4-1982@users.noreply.github.com> Date: Fri, 21 Aug 2026 23:43:48 +0300 Subject: [PATCH 1/2] fix(ast): render IS NOT NULL without dropping NOT qualifier The parser stores `IS NULL` with Not=true for `IS NOT NULL`, but BinaryExpression.SQL() ignored the Not flag in the null branch, silently inverting the predicate to IS NULL. Regression: pkg/sql/ast/is_not_null_test.go, plus a roundtrip case. --- pkg/sql/ast/is_not_null_test.go | 48 +++++++++++++++++++++++++++++++++ pkg/sql/ast/roundtrip_test.go | 1 + pkg/sql/ast/sql.go | 22 ++++++++++++--- 3 files changed, 68 insertions(+), 3 deletions(-) create mode 100644 pkg/sql/ast/is_not_null_test.go diff --git a/pkg/sql/ast/is_not_null_test.go b/pkg/sql/ast/is_not_null_test.go new file mode 100644 index 00000000..6b18bd12 --- /dev/null +++ b/pkg/sql/ast/is_not_null_test.go @@ -0,0 +1,48 @@ +// Copyright 2026 GoSQLX Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package ast_test + +import ( + "testing" + + "github.com/ajitpratap0/GoSQLX/pkg/gosqlx" +) + +// Regression: rendering `a IS NOT NULL` used to drop the NOT qualifier, +// producing `a IS NULL` and silently inverting the intended predicate. +// See BinaryExpression.SQL() — the parser stores operator "IS NULL" with +// Not=true for `IS NOT NULL`, but the renderer ignored the Not flag. +func TestRenderIsNotNullPreservesNot(t *testing.T) { + tests := []struct { + name string + sql string + want string + }{ + {"is null", "SELECT * FROM users WHERE email IS NULL", "SELECT * FROM users WHERE email IS NULL"}, + {"is not null", "SELECT * FROM users WHERE email IS NOT NULL", "SELECT * FROM users WHERE email IS NOT NULL"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tree, err := gosqlx.Parse(tt.sql) + if err != nil { + t.Fatalf("Parse(%q) unexpected error: %v", tt.sql, err) + } + if got := tree.SQL(); got != tt.want { + t.Errorf("SQL() = %q, want %q", got, tt.want) + } + }) + } +} diff --git a/pkg/sql/ast/roundtrip_test.go b/pkg/sql/ast/roundtrip_test.go index 1ab20369..eb9018dc 100644 --- a/pkg/sql/ast/roundtrip_test.go +++ b/pkg/sql/ast/roundtrip_test.go @@ -44,6 +44,7 @@ func TestRoundtrip(t *testing.T) { {"select in list", "SELECT * FROM users WHERE id IN (1, 2, 3)"}, {"select between", "SELECT * FROM users WHERE age BETWEEN 18 AND 65"}, {"select is null", "SELECT * FROM users WHERE email IS NULL"}, + {"select is not null", "SELECT * FROM users WHERE email IS NOT NULL"}, {"select like", "SELECT * FROM users WHERE name LIKE '%alice%'"}, {"select subquery", "SELECT * FROM users WHERE id IN (SELECT user_id FROM orders)"}, {"select exists", "SELECT * FROM users WHERE EXISTS (SELECT 1 FROM orders WHERE orders.user_id = users.id)"}, diff --git a/pkg/sql/ast/sql.go b/pkg/sql/ast/sql.go index 9569cfe3..4b4e76a7 100644 --- a/pkg/sql/ast/sql.go +++ b/pkg/sql/ast/sql.go @@ -167,9 +167,14 @@ func (b *BinaryExpression) SQL() string { upperOp := strings.ToUpper(op) - // Handle IS NULL / IS NOT NULL (right side is NULL literal) - if upperOp == "IS NULL" || upperOp == "IS NOT NULL" { - return fmt.Sprintf("%s %s", left, upperOp) + // Handle IS NULL / IS NOT NULL (right side is NULL literal). + // The parser stores the operator as "IS NULL" and uses the Not flag to + // disambiguate `IS NOT NULL`, so honour it here to avoid dropping NOT. + if upperOp == "IS NULL" { + if b.Not { + return fmt.Sprintf("%s IS NOT NULL", left) + } + return fmt.Sprintf("%s IS NULL", left) } // Handle special operators like LIKE, ILIKE, SIMILAR TO @@ -340,6 +345,17 @@ func (f *FunctionCall) SQL() string { sb := getBuilder() defer putBuilder(sb) sb.WriteString(f.Name) + if len(f.Parameters) > 0 { + // ClickHouse parametric aggregates: quantile(0.5)(x). + // Parameters are rendered in their own parenthesis group before args. + params := make([]string, len(f.Parameters)) + for i, p := range f.Parameters { + params[i] = exprSQL(p) + } + sb.WriteString("(") + sb.WriteString(strings.Join(params, ", ")) + sb.WriteString(")") + } sb.WriteString("(") if f.Distinct { sb.WriteString("DISTINCT ") From 58018572c7134de0eb892778ecbaa64fb277f877 Mon Sep 17 00:00:00 2001 From: basili4-1982 <1531789+basili4-1982@users.noreply.github.com> Date: Fri, 21 Aug 2026 23:43:51 +0300 Subject: [PATCH 2/2] fix(ast): render ClickHouse parametric aggregates fn(params)(args) FunctionCall.SQL() emitted args but dropped the Parameters group, turning quantile(0.5)(x) into quantile(x). Parameters are now rendered in their own parenthesis group before args. Regression: TestClickHouseParametricAggregates_Render in pkg/sql/parser/clickhouse_parametric_test.go. --- pkg/sql/parser/clickhouse_parametric_test.go | 27 ++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/pkg/sql/parser/clickhouse_parametric_test.go b/pkg/sql/parser/clickhouse_parametric_test.go index cf31280b..5aa8784c 100644 --- a/pkg/sql/parser/clickhouse_parametric_test.go +++ b/pkg/sql/parser/clickhouse_parametric_test.go @@ -72,3 +72,30 @@ func TestClickHouseParametricAggregates_ASTShape(t *testing.T) { t.Fatal("did not find quantileTDigest FunctionCall in AST") } } + +// TestClickHouseParametricAggregates_Render is a regression for the renderer: +// FunctionCall.SQL() must emit ClickHouse parametric aggregates as +// `funcName(params)(args)`, preserving the params group. Previously the +// Parameters group was silently dropped, yielding `funcName(args)`. +func TestClickHouseParametricAggregates_Render(t *testing.T) { + tests := []struct { + name string + sql string + }{ + {"quantile_tdigest", `SELECT quantileTDigest(0.95)(value) FROM events`}, + {"top_k", `SELECT topK(10)(name) FROM users`}, + {"quantiles_multi", `SELECT quantiles(0.5, 0.9, 0.99)(latency_ms) FROM requests`}, + {"with_group_by", `SELECT category, quantileTDigest(0.99)(price) FROM products GROUP BY category`}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tree, err := gosqlx.ParseWithDialect(tt.sql, keywords.DialectClickHouse) + if err != nil { + t.Fatalf("parse failed: %v", err) + } + if got := tree.SQL(); got != tt.sql { + t.Errorf("SQL() = %q, want %q", got, tt.sql) + } + }) + } +}