[ajitpratap0]

Summary

Fixes #854.

BigQuery's capabilities set SupportsPivotUnpivot: true, but pivotDialectAllowed() checked a hard-coded list of SQL Server, Oracle and Snowflake. As a result, gosqlx validate --dialect bigquery "SELECT * FROM sales PIVOT (SUM(amount) FOR quarter IN ('Q1', 'Q2'))" failed with E2002.

• Gate: the check now uses p.Capabilities().SupportsPivotUnpivot, the same pattern isQualifyKeyword uses for SupportsQualify. I checked every dialect: only SQL Server, Oracle, Snowflake and BigQuery claim the capability. The default (Unknown), Generic, PostgreSQL, MySQL, MariaDB, SQLite, ClickHouse and Redshift all return false, so behaviour there is unchanged. Tests pin this. No capability flags changed.
• BigQuery grammar (checked against the BigQuery query-syntax reference). Each extension is gated to the dialects whose grammar has it:

Syntax	BigQuery	Oracle	Snowflake	SQL Server
Aggregate alias SUM(x) [AS] total and multiple aggregates	✅	✅	–	–
PIVOT IN value alias without AS ('Q1' q1)	✅	✅	✅	–
UNPIVOT INCLUDE NULLS / EXCLUDE NULLS	✅	✅	✅	–
UNPIVOT row-value alias without AS (string or INT64)	✅	–	–	–
Numeric UNPIVOT row-value alias with AS	✅	✅	–	–
Multi-column UNPIVOT ((a, b) FOR k IN ((c1, c2) AS 'x', ...))	✅	✅	–	–

• AST (additive fields; parsed quoted aliases and UNPIVOT value names retain source delimiters):
  • PivotClause.AggregateAlias and PivotClause.ExtraAggregates []PivotAggregate (the 2nd and later aggregates; Children() includes them, and pool release frees them)
  • UnpivotClause.NullHandling and UnpivotClause.ValueColumns (multi-column form; ValueColumn is empty in that form)
  • ast.UnpivotNullHandling
• Rendering: both SQL() and pkg/formatter render the new fields. QuotePivotItem now accepts a well-formed (atom, atom) [AS atom] column set and still double-quotes anything else.
• Quoted names: PIVOT aggregate aliases and UNPIVOT single/multi-column value names preserve their source delimiters in both renderers, including BigQuery backticks and Oracle case-sensitive double quotes. Malformed hand-built values still use the normal quoting fallback.
• Backtick detection: the tokenizer leaves Token.Quote at zero for backticks. renderCurrentIdent compares the token span width with len(Token.Value) (bytes), or detects a multiline span; it does not read source text. Tokenizer tests pin byte columns for 日本 and doubled backticks, and parser/formatter round trips cover both forms. The existing tokenizer metadata contract remains unchanged.

Scoped out

• Constant expressions as PIVOT IN values (IN (1 + 1), IN (DATE '2024-01-01')); only literals and names are parsed.
• Oracle tuple row-value aliases ((c1, c2) AS ('a', 'b')).

Test Plan

• pkg/sql/parser/bigquery_pivot_test.go covers:
  • BigQuery PIVOT (issue repro, IN aliases with and without AS, multiple aliased aggregates, backtick names, subquery source)
  • BigQuery UNPIVOT (basic, INCLUDE/EXCLUDE NULLS, string and INT64 row aliases, multi-column)
  • malformed input
  • dialect gating: default, Generic, PostgreSQL, MySQL, MariaDB, SQLite, ClickHouse and Redshift reject PIVOT/UNPIVOT; pivot still works as an alias in PostgreSQL; the extensions are rejected where the dialect lacks them and accepted for Oracle/Snowflake where it has them
• pkg/formatter/bigquery_pivot_render_test.go checks FormatAST and SQL() output and reparsing, including BigQuery backtick aggregate aliases, single/multi-column UNPIVOT value names, non-ASCII and doubled backtick names, and Oracle quoted aliases. Hand-built malformed names retain safe fallback quoting.
• Tokenizer tests assert the existing Quote == 0 contract and exact byte spans for Unicode and doubled-backtick identifiers through both tokenization entry points.
• A numeric AS 1 UNPIVOT alias is accepted for Oracle/BigQuery and rejected for Snowflake/SQL Server; quoted UNPIVOT value and input-column sets with spaces round-trip through both renderers.
• task check passes; go test -race ./pkg/sql/parser/ ./pkg/formatter/ ./pkg/sql/ast/ ./pkg/sql/tokenizer/ passes.
• Repository-wide benchmark smoke passes with CGO enabled, unit tests separated, and one benchmark iteration. Standalone TestPerformanceRegression also passes.
• BenchmarkPerformanceBaseline before review fixes (2810b973) vs final head (e4aab414): saved test binaries, 10 samples each, interleaved with alternating order on Apple M4 / Go 1.27.2. Pinned benchstat (golang.org/x/perf/cmd/benchstat@v0.0.0-20260908200009-22c9c6c9d4da) shows no significant timing change in any case (all p >= 0.148; geomean -0.22%); bytes/op and allocations/op are identical. No speedup is claimed.

Breaking Changes

The AST fields are additive. Parsed quoted aggregate aliases and UNPIVOT value columns now retain source delimiters so rendering preserves the identifier. Two input-validation changes are listed in CHANGELOG:

• PIVOT/UNPIVOT IN lists require commas between entries. Where bare aliases are supported, IN (a b) is one entry (a AS b); otherwise the missing comma is rejected.
• AS in an IN list must be followed by an alias. IN ('a' AS) used to drop the AS silently.

Also, dialect aliases that dialect.Parse maps (e.g. mssql, bq) now enable PIVOT, because the gate uses the typed dialect rather than an exact string match.

🤖 Generated with Claude Code

https://claude.ai/code/session_01RkzVu3JY9Y6BRWfjesWts9

[vercel]

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

Project	Deployment	Actions	Updated
gosqlx-website	Ready	Preview	Oct 22, 2026 6:55pm UTC

[github-actions]

Claude finished @ajitpratap0's task in 39s —— View job

---

PR review: BigQuery PIVOT/UNPIVOT (#857)

I read the diff in pivot.go, ast_clauses.go, sql_table_hints_pivot.go, render.go and the pool release code. I couldn't run the tests in this environment because the shell commands needed approval. Everything below comes from reading the code, not from running it.

Overall: this looks good. Using Capabilities().SupportsPivotUnpivot is the right fix, and it follows the same pattern as isQualifyKeyword. The AST changes only add fields. Children(), pool release, SQL() and the formatter all cover the new fields. Each syntax extension is gated per dialect.

Things to consider

1. Changelog wording on IN (a b) (pivot.go, around line 245). For Oracle, Snowflake and BigQuery, IN (a b) now parses as one entry, a AS b, because the bare-alias branch takes it. Only SQL Server gets the "expected , or )" error. The PR text and CHANGELOG say commas are now required in general, which is only true for SQL Server and for entries that can't take an alias. Suggest rewording to match.

2. Backtick detection is a heuristic (renderCurrentIdent). It infers backticks from End.Column - Start.Column > len(Value). That works because the span is always wider than the value when the source has backticks. Two risks:

   • It depends on the tokenizer's column accounting: whether columns count runes or bytes, and how tabs are handled. A comment or test pinning that assumption would help.
   • It would misfire if the tokenizer ever reports a token's span wider than its text for another reason, such as an escape sequence. A short comment on why Quote == 0 is left as is would help.

   The follow-up of setting Quote properly in the tokenizer, once its goldens can change, would remove this workaround.

3. Unquoted aggregate aliases and value columns. parsePivotAggregateAlias and parseUnpivotNameList(..., false) store the bare Token.Value. Backtick or quoted names are re-quoted only if QuoteTableAlias / QuotePivotColumn decide they need quoting. A name like `my alias` should be handled, but I don't see a test for an aggregate alias or UNPIVOT value column that needs quoting. Please add one for PIVOT and one for multi-column UNPIVOT.

4. Multi-column UNPIVOT gating. multiColumn reuses pivotMultiAggregateAllowed(). The coupling is documented, but a dedicated unpivotMultiColumnAllowed() would make a future dialect change harder to get wrong.

5. parseUnpivotNameList and NULLS lookahead. INCLUDE/EXCLUDE is detected by value plus peekToken being NULLS. That is safe. One edge case: in Oracle or BigQuery, UNPIVOT include (...) with no NULLS doesn't consume anything and then fails on the ( check, which is the right outcome.

6. Performance and pooling. The new slices are only allocated when the clause uses them, which matches your benchstat result of no change. releaseTableReference correctly clears ExtraAggregates.

7. Security. I found no new concerns. UnpivotNullHandling normalizes to a fixed set, so a hand-built AST can't write raw text. QuotePivotItem still double-quotes anything malformed.

Tests

Coverage is broad: positive and negative cases, a dialect-gating matrix, and round trips through both the formatter and SQL(). The gaps are the quoted-alias cases in point 3, and a case showing that backtick names inside a multi-column set (a, b) round-trip.

None of the points above blocks merging. Point 1 is wording and point 3 is a small test gap.
· branch fix/bigquery-pivot-854

[github-actions]

Claude finished @ajitpratap0's task in 16s —— View job

---

PR review: BigQuery PIVOT/UNPIVOT (#857)

I read the diff in pkg/sql/parser/pivot.go. I didn't run the tests. This review comes from reading the code only.

Overall: the PR is in good shape. Capabilities().SupportsPivotUnpivot is the right gate and matches isQualifyKeyword. The AST changes only add fields. Each syntax extension is gated per dialect, and the new slices are only allocated when the clause uses them.

Things to consider

1. Changelog wording on IN (a b). In Oracle, Snowflake and BigQuery, IN (a b) now parses as one entry, a AS b, because the bare-alias branch takes it. Only SQL Server gets the expected , or ) error. Saying commas are now required everywhere overstates it. Please reword the CHANGELOG and PR text.

2. Backtick detection is a heuristic (renderCurrentIdent, pivot.go ~line 56). It infers backticks when End.Column - Start.Column > len(tok.Value). This depends on the tokenizer counting columns in the same units as len(), which is bytes. A non-ASCII backticked name such as `café` could be miscounted if columns count runes. For example, a backticked é has rune width 1 but byte length 2, so a span of 7 runes against len 5 still reads as wider, but an unquoted é name would have rune width 4 against len 5, which is fine. The failure mode to check is a multi-byte name where the rune-based span is no wider than the byte length, so the backticks go undetected. Please add a test with a non-ASCII backticked name and a comment that pins the column-unit assumption.

3. Aggregate aliases and value columns aren't stored in source form. parsePivotAggregateAlias and parseUnpivotNameList(..., false) store the bare Token.Value. Rendering then relies on QuoteTableAlias / QuotePivotColumn to re-quote. I don't see a test for an aggregate alias or UNPIVOT value column that needs quoting, such as `my alias`. Please add one for PIVOT and one for multi-column UNPIVOT.

4. Multi-column UNPIVOT gating. multiColumn reuses pivotMultiAggregateAllowed(). A dedicated unpivotMultiColumnAllowed() would make a future dialect change harder to get wrong.

5. Bare PIVOT aggregate alias. parsePivotAggregateAlias accepts any identifier as a bare alias. That is correct for SUM(x) total. Please confirm the next token, FOR, can never tokenize as an identifier. The tests suggest it can't.

6. Security and performance. I found nothing new. QuotePivotItem still quotes anything malformed, and the benchstat result is clean.

Tests

Coverage is broad: positive and negative cases, a dialect-gating matrix, and round trips through both the formatter and SQL(). The gaps are the quoted-alias cases in point 3 and the non-ASCII backtick case in point 2. A case with backtick names inside a multi-column set would also help.

None of these block merging. Point 1 is wording, and points 2 and 3 are small test gaps.
· branch fix/bigquery-pivot-854

[github-actions]

Claude finished @ajitpratap0's task in 15s —— View job

---

PR review: BigQuery PIVOT/UNPIVOT (#857)

I read the parser changes in pkg/sql/parser/pivot.go. I didn't run the tests, so this comes from reading the code only.

Overall: this is in good shape. Gating on Capabilities().SupportsPivotUnpivot is the right fix and matches isQualifyKeyword. The AST changes only add fields, and each syntax extension is gated per dialect. The new slices are only allocated when the clause uses them, which fits your clean benchstat result.

Things to consider

1. CHANGELOG and PR text on IN (a b). Only SQL Server rejects IN (a b) now. In Oracle, Snowflake and BigQuery the bare-alias branch reads it as one entry, a AS b. "Commas are now required" is therefore only true for SQL Server and for entries that can't take an alias. Please reword it.

2. Backtick detection (renderCurrentIdent). It is a span-width heuristic, so it depends on the tokenizer counting columns in the same units as len(Value).

   • Please add a test with a non-ASCII backticked name, such as `café`.
   • Please add a comment that pins the column-unit assumption and says why Token.Quote is left at zero.

3. Quoted-name tests. Aliases and value columns do go through renderCurrentIdent, so the parser keeps the source form. That means the gap is in tests, not in the code. Please add cases for:

   • a PIVOT aggregate alias that needs quoting, such as `my alias`
   • a multi-column UNPIVOT value set and an IN column set with backtick names, such as (`a`, `b`)

4. Multi-column UNPIVOT gating. multiColumn reuses pivotMultiAggregateAllowed(). The doc comment records the coupling, but a dedicated unpivotMultiColumnAllowed() would keep a future dialect change from silently altering both behaviours.

5. Bare aggregate alias. parsePivotAggregateAlias accepts any identifier as a bare alias. This is safe only because FOR tokenizes as TokenTypeFor. A negative test for PIVOT (SUM(x) FOR ...) with no alias would pin that.

6. Security and performance. I found no new concerns. QuotePivotItem still double-quotes malformed input, and the pool release clears the new slices.

Tests

Coverage is broad: positive and negative cases, a dialect-gating matrix, and round trips through both the formatter and SQL(). The gaps are the ones in points 2, 3 and 5.

None of these block merging. Point 1 is wording and the rest are small test or hardening items.
· branch fix/bigquery-pivot-854

[github-actions]

Claude finished @ajitpratap0's task in 24s —— View job

---

PR review: BigQuery PIVOT/UNPIVOT (#857)

I read the pivot.go diff. I did not run the tests, so this comes from reading the code only.

Overall: the PR is in good shape and I see no blockers. Capabilities().SupportsPivotUnpivot is the right gate. The per-dialect extension gates are narrow, and the new AST fields are additions only.

Corrections to the earlier automated reviews

• Aliases and value columns keep source quoting. parsePivotAggregateAlias, parseUnpivotNameList and the single-column UNPIVOT value path all call renderCurrentIdent(). They do not store the bare Token.Value. The earlier concern that rendering relies on re-quoting is therefore mostly moot.
• FOR cannot be taken as a bare aggregate alias. parsePivotAggregateAlias only accepts isIdentifier(), and FOR tokenizes as TokenTypeFor.

Things to consider

1. Possible double quoting on render. Aliases are stored with their delimiters, for example `my alias`. If SQL() or the formatter then runs QuoteTableAlias or QuotePivotColumn on the stored value, it could quote twice or mangle it. Please confirm this with a round-trip test using a backticked alias that contains a space. Cover a PIVOT aggregate alias, a single-column UNPIVOT value column, and a multi-column set such as (`a b`, c).
2. Backtick detection heuristic (renderCurrentIdent, pivot.go ~line 56). It treats an identifier as backticked when the span is wider than len(tok.Value) or crosses lines.
   • len counts bytes. If the tokenizer's columns count runes, a non-ASCII name such as `café` could be misjudged. Please add one non-ASCII backtick test.
   • Please add a short comment pinning the column-unit assumption, and note that this goes away once the tokenizer sets Quote for backticks.
3. CHANGELOG and PR wording on IN (a b). In Oracle, Snowflake and BigQuery, IN (a b) now parses as the single entry a AS b. Only SQL Server gets the "expected , or )" error. "IN lists now require commas" overstates it, so please reword.
4. Gate naming. The multi-column UNPIVOT form reuses pivotMultiAggregateAllowed(). A dedicated unpivotMultiColumnAllowed() would stop a later change to one feature from silently changing the other. This is optional.
5. Minor: UNPIVOT IN alias branch. The AS branch accepts a Number alias for every dialect that allows AS, but the bare form is BigQuery only. That matches the table in the PR description, but I did not see a negative test for AS 1 in SQL Server or Snowflake. If a number alias is meant to be BigQuery-only, gate it.

Performance and security

• The new slices are allocated only when the clause uses them, and the benchstat result is clean.
• releaseTableReference clears ExtraAggregates.
• UnpivotNullHandling normalizes to a fixed set, and QuotePivotItem still double-quotes anything malformed. I found nothing new.

Tests

Coverage is broad: positive and negative cases, a dialect-gating matrix, and round trips through both the formatter and SQL(). The gaps are the quoted-alias cases in item 1 and the non-ASCII backtick case in item 2.
· branch fix/bigquery-pivot-854

[github-actions]

Claude finished @ajitpratap0's task in 17s —— View job

---

PR review: BigQuery PIVOT/UNPIVOT (#857)

I read the gating code in pkg/sql/parser/pivot.go and the earlier review comments. I did not run the tests or read the whole diff, so this review is partial.

Overall: no blockers. Gating on Capabilities().SupportsPivotUnpivot is the right fix and matches isQualifyKeyword. The AST changes only add fields, and each syntax extension is gated per dialect.

Earlier review points that look already handled

Going by the PR description and the latest commits (accbfe5, e4aab41), I believe these are done, but I have not re-checked each against the test files:

• Quoted aliases and value columns: they keep their source delimiters, and the PR describes round-trip tests for both renderers.
• Non-ASCII and doubled-backtick names: the PR describes tokenizer byte-span tests and round trips.
• Numeric AS 1 alias: the PR describes it as gated by dialect, accepted for Oracle and BigQuery and rejected for Snowflake and SQL Server.

Remaining suggestions

1. Multi-column UNPIVOT gating (pivot.go:114). The multi-column UNPIVOT form reuses pivotMultiAggregateAllowed(). The doc comment records the coupling. A dedicated unpivotMultiColumnAllowed() would stop a later change to one feature from silently changing the other. This is optional.
2. Changelog wording on IN (a b). In dialects with bare aliases (Oracle, Snowflake, BigQuery), IN (a b) parses as one entry, a AS b. Only SQL Server and entries that can't take an alias reject it. The PR text says commas are now required in general, which overstates it. Please reword the PR text and CHANGELOG.
3. Backtick heuristic (renderCurrentIdent). It compares span width with len(Value). Please make sure a comment says the width is in bytes, that Quote is left at zero on purpose, and that the heuristic can go once the tokenizer sets Quote for backticks.

Performance and security

• The new slices are allocated only when the clause uses them, and releaseTableReference clears them.
• QuotePivotItem still double-quotes malformed hand-built input, so I found no new injection surface.
  · branch fix/bigquery-pivot-854