@jay23606 is attempting to deploy a commit to the coolajitpratapsingh0-9932's projects Team on Vercel.

A member of the Team first needs to authorize it.

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Pre-merge review: #591 fix(cli): format NOT EXISTS without a nil dereference

Thanks @jay23606. This is a small, correct fix. I ran it against a set of inputs built to break it and against the whole testdata/ corpus, comparing origin/main with this branch.

Verdict: MERGE. Nothing blocks. The suggestions below are cheap and optional. The larger problems I found all exist on main already, so they should get their own issues rather than grow this PR.

What the PR claims, and whether it holds

Adversarial cases (CLI formatSQL, compact and readable, then re-parse and re-format to check idempotence)

On main, 26 of these runs panicked. On this branch, none panic.

Corpus totals for testdata/**/*.sql (170 files, compact and readable, 340 runs):

• main: 238 ok, 4 panics, 24 re-parse failures, 74 parse errors on input.
• PR: 242 ok, 0 panics, 24 re-parse failures (the same ones), 74 parse errors on input.

No regressions.

Should fix before merge (optional, cheap)

1. cmd/gosqlx/cmd/sql_formatter.go:851: the existing comment // Handle EXISTS (subquery) - NOT EXISTS is handled via UnaryExpression is wrong, and this PR is the right place to fix it. Suggested text: // Handle EXISTS (subquery). NOT EXISTS arrives as BinaryExpression{Operator:"NOT", Right:nil}; see the BinaryExpression case.

2. Test coverage (sql_formatter_test.go, TestSQLFormatter_NotExistsDoesNotPanic): the test covers one shape, readable mode only, and checks a substring. Two changes would make it much stronger:

   • Make it a table test over a few shapes: WHERE NOT EXISTS, SELECT NOT EXISTS (…) AS x, CASE WHEN NOT EXISTS, HAVING NOT EXISTS, and nested.
   • For each shape, run both Compact: true and Compact: false, then assert that the output re-parses and that formatting it again gives the same output.

   Also, the test never calls p.Release() and never releases the parsed result AST; only the separate astObj is released. The newer pattern in TestSQLFormatter_ParenthesizedExpression (defer p.Release() plus defer ast.ReleaseAST(tree)) is cleaner. Calling (&Formatter{Opts: CLIFormatterOptions{…}}).formatSQL(sql) would go further and test the real CLI path.

3. Optional hardening, item 3 of the fix proposed in [BUG] gosqlx format panics (nil pointer) on NOT EXISTS; CLI formatter lacks ExistsExpression case #587: add if expr == nil { return nil } at the top of formatExpression (sql_formatter.go:619). With it, an unhandled nil sub-expression can no longer crash the CLI through expr.TokenLiteral() in the default: branch (:894). I found no other nil shape that the parser produces today, so this is defensive only.

Deferred: real bugs, pre-existing on main, out of scope here (I recommend filing issues)

1. ast.SQL() and pkg/formatter render NOT EXISTS as SQL that does not parse. [BUG] gosqlx format panics (nil pointer) on NOT EXISTS; CLI formatter lacks ExistsExpression case #587 says the library renderers are fine. They are not.

   • Input: SELECT a FROM t WHERE NOT EXISTS (SELECT 1 FROM u WHERE u.id = t.id).
   • tree.SQL() returns SELECT a FROM t WHERE NOT (EXISTS (SELECT 1 FROM u WHERE u.id = t.id) NOT ). formatter.FormatString gives the same text with ;.
   • Re-parsing either output fails with E2002 … expected ), got NOT.
   • Cause: in pkg/sql/ast/sql.go, BinaryExpression.writeSQL takes the generic b.Not path, NOT ( + Left + + op + + Right + ), which writes op = "NOT" and an empty Right. It needs a branch for Operator == "NOT" && Right == nil that writes NOT followed by Left.
   • This bug was already there before the perf(ast): make SQL() serialization allocation-light (drop fmt/ToUpper, fix builderPool, writer-based rendering) (#558) #575 rewrite. It affects every NOT EXISTS going through SQL(), pkg/formatter, and anything else that round-trips through them. I recommend a new high-priority issue.

2. The CLI formatter drops set-operation subqueries. EXISTS (…), IN (…) and the scalar (…) subquery only format *ast.SelectStatement. A *ast.SetOperation body (UNION/EXCEPT/INTERSECT) is silently dropped:

   • WHERE EXISTS (SELECT 1 FROM u UNION ALL SELECT 2 FROM v) becomes WHERE EXISTS ().
   • a IN (SELECT … UNION SELECT …) becomes a IN ().

   Neither output parses. pkg/formatter handles both correctly. I recommend a new issue.

3. Many expression types are missing from the CLI formatExpression switch. None of them panic. They fall back to TokenLiteral() and corrupt the SQL without any error. These account for the 24 corpus re-parse failures:

   • CastExpression: CAST(a AS INT) and a::int both become CAST.
   • ExtractExpression: becomes EXTRACT.
   • IntervalExpression: becomes INTERVAL.
   • AnyExpression / AllExpression: a = ANY (subq) becomes ANY, and the left operand is lost too.
   • TupleExpression: (a, b) IN ((1, 2), (3, 4)) becomes TUPLE IN (TUPLE, TUPLE).
   • ArrayConstructorExpression: becomes ARRAY.
   • ArraySubscriptExpression / ArraySliceExpression: become [] and [:].
   • RollupExpression / CubeExpression / GroupingSetsExpression: the argument list is dropped.
   • NamedArgument: f(x => 1) becomes f(x). pkg/formatter has the same bug.

   pkg/formatter renders all of these correctly except NamedArgument. I recommend one umbrella issue. The durable fix is to make gosqlx format delegate to pkg/formatter instead of keeping a second renderer. Divergence between the two renderers is behind [BUG] GoSQLX does not support parentheses #519, [BUG] ClickHouse parametric aggregates drop params group on render (quantile(0.5)(x) → quantile(x)) #531, [BUG] insert with CTE #537 and [BUG] gosqlx format panics (nil pointer) on NOT EXISTS; CLI formatter lacks ExistsExpression case #587.

Overlap with in-flight PRs #583 and #584

• sql_formatter.go: no textual overlap.

  • This PR edits the top of the *ast.BinaryExpression case (around line 665).
  • fix(cli): render ClickHouse parametric aggregate parameters in gosqlx format (#531) #583 edits the *ast.FunctionCall case (around line 699).
  • fix(parser): accept WITH ... SELECT as the source of INSERT and keep leading CTEs in gosqlx format (#537) #584 edits formatInsert (around line 259).

  Applied in sequence onto origin/main (fix(cli): format NOT EXISTS without a nil dereference #591, then fix(cli): render ClickHouse parametric aggregate parameters in gosqlx format (#531) #583, then fix(parser): accept WITH ... SELECT as the source of INSERT and keep leading CTEs in gosqlx format (#537) #584), all three apply cleanly to this file.

• sql_formatter_test.go: this PR and fix(parser): accept WITH ... SELECT as the source of INSERT and keep leading CTEs in gosqlx format (#537) #584 both append new tests right after TestSQLFormatter_ParenthesizedExpression, at the end of the file. Whichever merges second gets an append/append conflict. It is trivial to resolve: keep both tests. fix(cli): render ClickHouse parametric aggregate parameters in gosqlx format (#531) #583 adds its own test file, so it does not overlap.

Conventions

• Commit and PR title follow Conventional Commits (fix(cli): …), and the commit says Fixes #587.
• There is no CHANGELOG entry. CONTRIBUTING says "CHANGELOG.md updated (if applicable)", and fix(cli): render ClickHouse parametric aggregate parameters in gosqlx format (#531) #583 and fix(parser): accept WITH ... SELECT as the source of INSERT and keep leading CTEs in gosqlx format (#537) #584 don't add one either. A one-line ### Fixed entry under [Unreleased] would be nice to have: "gosqlx format no longer panics on NOT EXISTS (subquery) ([BUG] gosqlx format panics (nil pointer) on NOT EXISTS; CLI formatter lacks ExistsExpression case #587)". It does not block; a maintainer can add it at release time.

Security and supply chain (fork PR)

No concerns. Safe to run CI: yes.

• Only two files change: cmd/gosqlx/cmd/sql_formatter.go (+6) and cmd/gosqlx/cmd/sql_formatter_test.go (+30).
• Nothing touches .github/, scripts, the Taskfile, go.mod or go.sum, build tags or init().
• No network, file or exec calls. The test does no I/O beyond in-memory parsing.
• No non-ASCII or hidden characters in the diff. One commit, authored by the contributor's GitHub noreply address.
• On input handling, the change removes a crash an attacker could trigger. A CLI user or CI job formatting untrusted SQL could abort the process with any NOT EXISTS. The new code adds one bounded recursion into Left, which is no deeper than existing cases and already falls under the parser's recursion limits.

Evidence

• go test -race -count=1 ./cmd/gosqlx/...: all ok.
• go vet ./cmd/gosqlx/...: clean. gofmt -l cmd/gosqlx/cmd: clean. golangci-lint run ./cmd/gosqlx/cmd/...: 0 issues.
• go test -run TestSQLFormatter_NotExistsDoesNotPanic: PASS on the PR. The issue's reproduction panics on main with a nil-pointer dereference at sql_formatter.go:889.
• Differential corpus run (CLI formatSQL, both modes, re-parse and idempotence), main vs PR: the only differences are mysql/19_not_exists.sql and corpus/tpch/q21_suppliers_kept_waiting.sql, which go from PANIC to ok.
• Library check on main: tree.SQL() returns SELECT a FROM t WHERE NOT (EXISTS (SELECT 1 FROM u WHERE u.id = t.id) NOT ), and re-parsing it fails with E2002 expected ), got NOT.

Independent pre-merge review (adversarial correctness + security/supply-chain), run because the automated claude-review workflow skips fork PRs. Generated with Claude Code.

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated
gosqlx-website	Ready	Preview	Oct 13, 2026 11:12am UTC