From 77f03c21116bac9a1126c3bb2ab687ce2b33151d Mon Sep 17 00:00:00 2001 From: Ajit Pratap Singh Date: Thu, 22 Oct 2026 23:57:22 +0530 Subject: [PATCH 1/3] fix(parser): BigQuery PIVOT/UNPIVOT (#854) Gate PIVOT/UNPIVOT on Capabilities().SupportsPivotUnpivot instead of a hard-coded SQL Server/Oracle/Snowflake list, so BigQuery (which advertises the capability) no longer fails with E2002. The capability is true for exactly SQL Server, Oracle, Snowflake and BigQuery; the default, Generic and all other dialects keep treating pivot/unpivot as identifiers. Add the BigQuery grammar extensions, gated per dialect: - aggregate aliases and multiple aggregates (Oracle, BigQuery) - IN value aliases without AS (all but SQL Server) - UNPIVOT INCLUDE/EXCLUDE NULLS (Oracle, Snowflake, BigQuery) - row-value aliases without AS and INT64 aliases (BigQuery) - multi-column UNPIVOT ((a, b) FOR k IN ((c1, c2) AS 'x')) (Oracle, BigQuery) AST: PivotClause.AggregateAlias/ExtraAggregates, UnpivotClause.NullHandling/ ValueColumns, ast.PivotAggregate and ast.UnpivotNullHandling, rendered by SQL() and pkg/formatter. IN lists now require commas and an alias after AS. Backtick-quoted IN names keep their backticks when rendered. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RkzVu3JY9Y6BRWfjesWts9 --- CHANGELOG.md | 28 ++ README.md | 2 +- docs/SQL_COMPATIBILITY.md | 12 +- pkg/formatter/bigquery_pivot_render_test.go | 124 +++++++ pkg/formatter/render.go | 35 +- pkg/sql/ast/ast_clauses.go | 55 ++- pkg/sql/ast/pool_statement_release.go | 7 +- pkg/sql/ast/sql_table_hints_pivot.go | 91 ++++- pkg/sql/parser/bigquery_pivot_test.go | 361 ++++++++++++++++++++ pkg/sql/parser/pivot.go | 243 +++++++++++-- 10 files changed, 895 insertions(+), 63 deletions(-) create mode 100644 pkg/formatter/bigquery_pivot_render_test.go create mode 100644 pkg/sql/parser/bigquery_pivot_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index a1f66f83a..84cdb68da 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,34 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed +- **BigQuery PIVOT / UNPIVOT** (#854): the parser gated PIVOT/UNPIVOT on a + hard-coded SQL Server / Oracle / Snowflake list, so BigQuery rejected them + with E2002 although its capabilities advertise `SupportsPivotUnpivot`. The + gate is now `Capabilities().SupportsPivotUnpivot` (true for exactly SQL + Server, Oracle, Snowflake and BigQuery; the default, Generic, PostgreSQL, + MySQL, MariaDB, SQLite, ClickHouse and Redshift dialects still treat `pivot` + / `unpivot` as identifiers). Supported BigQuery forms: aggregate aliases and + multiple aggregates, `IN` value aliases with or without `AS`, + `UNPIVOT INCLUDE NULLS` / `EXCLUDE NULLS`, row-value aliases with or without + `AS` (string or INT64) and multi-column UNPIVOT. The extensions follow each + dialect's grammar: Oracle also gets aliased/multiple aggregates, the NULLS + modifier and multi-column UNPIVOT; Snowflake gets the NULLS modifier and + bare `IN` value aliases; SQL Server is unchanged. +- PIVOT/UNPIVOT backtick-quoted `IN` names and aliases keep their backticks + when rendered (they were written bare, which did not re-parse). + +### Added +- `ast.PivotClause.AggregateAlias`, `ast.PivotClause.ExtraAggregates` + (`[]ast.PivotAggregate`), `ast.UnpivotClause.NullHandling`, + `ast.UnpivotClause.ValueColumns` and `ast.UnpivotNullHandling`; rendered by + `SQL()` and `pkg/formatter`. + +### Behavior changes +- PIVOT / UNPIVOT `IN` lists now require commas between entries and an alias + after `AS` in every PIVOT dialect (`IN (a b)` used to parse as two entries + and `IN ('a' AS)` silently dropped the `AS`). + ## [1.16.0] - 2026-10-22 — Performance Audit, SQL Highlighting, Dialect-Aware LSP, Claude Code Plugin & ChatGPT App, Correctness Sweep Covers everything merged since v1.15.0: the October 2026 performance audit diff --git a/README.md b/README.md index 013ca6af5..5179376fa 100644 --- a/README.md +++ b/README.md @@ -166,7 +166,7 @@ Formatting turns one-line SQL into a readable query: - **[Analysis](https://gosqlx.dev/docs/reference/linting-rules/)** - SQL injection scanner, 30 lint rules (L001-L030), 20 optimizer rules and metadata extraction. - **[Tooling](https://gosqlx.dev/docs/cli-guide/)** - AST-based formatter, query transforms API, VS Code extension and GitHub Action. - **[Syntax highlighting](docs/HIGHLIGHTING.md)** - dialect-aware highlighting for terminals (ANSI), HTML/CSS, JSON spans, LSP semantic tokens and WASM; `gosqlx highlight query.sql` or `gosqlx.HighlightANSI(sql)`. -- **[Multi-dialect](https://gosqlx.dev/docs/reference/sql-compatibility/)** - 8 dialects with dedicated grammar support: PostgreSQL, MySQL, MariaDB, SQL Server, Oracle, SQLite, Snowflake, ClickHouse. The `--dialect` flag and `WithDialect` accept 11 names: those 8 (`postgresql`, `mysql`, `mariadb`, `sqlserver`, `oracle`, `sqlite`, `snowflake`, `clickhouse`) plus `generic`, `bigquery` and `redshift`. BigQuery and Redshift have no dialect-specific keyword set (they use the generic keywords) and only capability-gated grammar, such as BigQuery's `QUALIFY`, `UNNEST` in `FROM` and `(struct).field` access. +- **[Multi-dialect](https://gosqlx.dev/docs/reference/sql-compatibility/)** - 8 dialects with dedicated grammar support: PostgreSQL, MySQL, MariaDB, SQL Server, Oracle, SQLite, Snowflake, ClickHouse. The `--dialect` flag and `WithDialect` accept 11 names: those 8 (`postgresql`, `mysql`, `mariadb`, `sqlserver`, `oracle`, `sqlite`, `snowflake`, `clickhouse`) plus `generic`, `bigquery` and `redshift`. BigQuery and Redshift have no dialect-specific keyword set (they use the generic keywords) and only capability-gated grammar, such as BigQuery's `QUALIFY`, `PIVOT`/`UNPIVOT`, `UNNEST` in `FROM` and `(struct).field` access. - **[AI-ready](https://gosqlx.dev/docs/integrations/mcp/)** - MCP server with 7 tools, public remote endpoint, Streamable HTTP. - **[Tested](https://gosqlx.dev/docs/reference/sql-compatibility/)** - race-detector clean in CI, ~80-85% SQL-99 (estimated). diff --git a/docs/SQL_COMPATIBILITY.md b/docs/SQL_COMPATIBILITY.md index 269c75795..c2ae72095 100644 --- a/docs/SQL_COMPATIBILITY.md +++ b/docs/SQL_COMPATIBILITY.md @@ -407,7 +407,7 @@ MariaDB inherits all MySQL features (SHOW, DESCRIBE, REPLACE INTO, ON DUPLICATE | Feature | Support Level | GoSQLX Parser | Test Coverage | Notes | |---------|---------------|---------------|---------------|-------| | **MERGE** | ✅ Full | ✅ Full | 95% | MERGE statements with WHEN clauses | -| **PIVOT/UNPIVOT** | ✅ Full | ✅ Full | — | Parsed and rendered since v1.14.0 (`FROM s PIVOT (...) AS p`) | +| **PIVOT/UNPIVOT** | ✅ Full | ✅ Full | — | Parsed and rendered since v1.14.0 (`FROM s PIVOT (...) AS p`). Also parsed for Oracle, Snowflake and BigQuery: PIVOT/UNPIVOT is gated on the dialect capability `SupportsPivotUnpivot` | | **CROSS/OUTER APPLY** | ✅ Full | 🔧 Syntax | 10% | Keywords reserved, no parsing logic | | **HierarchyID** | ✅ Full | 🔧 Syntax | 20% | Data type recognition | | **T-SQL functions** | ✅ Full | ⚠️ Partial | 40% | Subset of T-SQL functions | @@ -636,7 +636,7 @@ GoSQLX provides a first-class dialect mode engine that threads the SQL dialect t | **Snowflake** | `"snowflake"` | Snowflake keywords | MATCH_RECOGNIZE, @stage, SAMPLE, QUALIFY, VARIANT paths, time-travel, MINUS, LATERAL FLATTEN, TRY_CAST, IGNORE/RESPECT NULLS, LIKE ANY/ALL, CREATE STAGE/STREAM/TASK/PIPE stubs | ✅ 87/87 QA pass | | **ClickHouse** | `"clickhouse"` | ClickHouse keywords | PREWHERE, FINAL, GLOBAL IN/NOT IN, nested types, parametric aggregates, bare-bracket arrays, ORDER BY WITH FILL, CODEC, WITH TOTALS, LIMIT BY, ANY/ALL JOIN, SETTINGS, TTL, INSERT FORMAT | ✅ 83/83 QA pass (was 69/83 in v1.14.0) | | **MariaDB** | `"mariadb"` | MariaDB keywords (superset of MySQL) | All MySQL features + SEQUENCE DDL, FOR SYSTEM_TIME, WITH SYSTEM VERSIONING, PERIOD FOR, CONNECT BY | ✅ | -| **BigQuery** | `"bigquery"` | BigQuery keywords | Composite field access `(expr).field`, table functions | ⚠️ Keywords + basic parsing | +| **BigQuery** | `"bigquery"` | BigQuery keywords | Composite field access `(expr).field`, table functions, QUALIFY, PIVOT/UNPIVOT | ⚠️ Keywords + basic parsing | | **Redshift** | `"redshift"` | Redshift keywords | Keyword recognition | ⚠️ Keywords + basic parsing | | **Generic** | `"generic"` | Standard SQL keywords | No dialect-specific syntax; composite field access allowed | ✅ | @@ -715,6 +715,14 @@ gosqlx format --dialect mysql query.sql - Table functions in FROM: `numbers(N)`, `remote(...)`, `file(...)`, `url(...)`, `merge(...)`, `s3(...)`, `values(...)`, etc. - 30+ ClickHouse-specific keywords: TTL, CODEC, FORMAT, SETTINGS, DISTRIBUTED, etc. +#### BigQuery +- PIVOT / UNPIVOT (unreleased, #854): aggregate aliases and multiple aggregates (`PIVOT (SUM(s) AS total, COUNT(*) AS n FOR q IN ('Q1' AS q1, 'Q2' q2))`), `UNPIVOT INCLUDE NULLS` / `EXCLUDE NULLS`, row-value aliases with or without `AS` (string or INT64), and multi-column UNPIVOT (`UNPIVOT ((a, b) FOR k IN ((c1, c2) AS 'x', (c3, c4) AS 'y'))`) +- PIVOT `IN` values must be literals or names; constant expressions such as `IN (1 + 1)` or `IN (DATE '2024-01-01')` are not parsed +- QUALIFY, `UNNEST` in FROM and `(struct).field` access are capability-gated; BigQuery has no dedicated keyword set +- Aggregate aliases with characters that need quoting are re-rendered in ANSI double quotes, as all GoSQLX aliases are + +Oracle also accepts aggregate aliases, multiple aggregates, `INCLUDE`/`EXCLUDE NULLS` and multi-column UNPIVOT; Snowflake accepts `INCLUDE`/`EXCLUDE NULLS`. SQL Server keeps the single-aggregate form without aliases or a NULLS modifier. Oracle's tuple row-value aliases (`(c1, c2) AS ('a', 'b')`) are not parsed. + ## SQL Standards Compliance Summary ### Overall Compliance diff --git a/pkg/formatter/bigquery_pivot_render_test.go b/pkg/formatter/bigquery_pivot_render_test.go new file mode 100644 index 000000000..e0fc89c06 --- /dev/null +++ b/pkg/formatter/bigquery_pivot_render_test.go @@ -0,0 +1,124 @@ +// 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 formatter_test + +import ( + "testing" + + "github.com/ajitpratap0/GoSQLX/pkg/formatter" + "github.com/ajitpratap0/GoSQLX/pkg/gosqlx" + "github.com/ajitpratap0/GoSQLX/pkg/sql/ast" + "github.com/ajitpratap0/GoSQLX/pkg/sql/keywords" +) + +// TestBigQueryPivot_FormatRoundTrip_Issue854 verifies BigQuery PIVOT / UNPIVOT +// (including aggregate aliases, multiple aggregates, INCLUDE/EXCLUDE NULLS and +// the multi-column UNPIVOT form) render through both pkg/formatter and +// ast SQL(), and that the output re-parses to the same rendering. +func TestBigQueryPivot_FormatRoundTrip_Issue854(t *testing.T) { + bq := keywords.DialectBigQuery + cases := []struct { + name string + input string + want string + }{ + { + "issue_repro", + "SELECT * FROM sales PIVOT (SUM(amount) FOR quarter IN ('Q1', 'Q2'))", + "SELECT * FROM sales PIVOT (SUM(amount) FOR quarter IN ('Q1', 'Q2'))", + }, + { + "aliases_and_multiple_aggregates", + "SELECT * FROM produce PIVOT (SUM(sales) total_sales, COUNT(*) AS num_records FOR quarter IN ('Q1' AS q1, 'Q2' q2)) p", + "SELECT * FROM produce PIVOT (SUM(sales) AS total_sales, COUNT(*) AS num_records FOR quarter IN ('Q1' AS q1, 'Q2' AS q2)) AS p", + }, + { + "backtick_alias", + "SELECT * FROM sales PIVOT (SUM(amount) AS `total amount` FOR quarter IN ('Q1' AS `q 1`))", + "SELECT * FROM sales PIVOT (SUM(amount) AS \"total amount\" FOR quarter IN ('Q1' AS `q 1`))", + }, + { + "unpivot_include_nulls", + "SELECT * FROM produce UNPIVOT INCLUDE NULLS (sales FOR quarter IN (Q1 'first', Q2 AS 2)) AS u", + "SELECT * FROM produce UNPIVOT INCLUDE NULLS (sales FOR quarter IN (Q1 AS 'first', Q2 AS 2)) AS u", + }, + { + "unpivot_multi_column", + "SELECT * FROM produce UNPIVOT EXCLUDE NULLS ((h1, h2) FOR semesters IN ((Q1, Q2) AS 'semester_1', (Q3, Q4) AS 'semester_2'))", + "SELECT * FROM produce UNPIVOT EXCLUDE NULLS ((h1, h2) FOR semesters IN ((Q1, Q2) AS 'semester_1', (Q3, Q4) AS 'semester_2'))", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + tree, err := gosqlx.ParseWithDialect(tc.input, bq) + if err != nil { + t.Fatalf("parse %q: %v", tc.input, err) + } + defer ast.ReleaseAST(tree) + + got := formatter.FormatAST(tree, ast.CompactStyle()) + if got != tc.want { + t.Errorf("FormatAST(compact) mismatch\n got: %s\nwant: %s", got, tc.want) + } + if len(tree.Statements) != 1 { + t.Fatalf("expected 1 statement, got %d", len(tree.Statements)) + } + sel, ok := tree.Statements[0].(*ast.SelectStatement) + if !ok { + t.Fatalf("expected *ast.SelectStatement, got %T", tree.Statements[0]) + } + if sqlOut := sel.SQL(); sqlOut != tc.want { + t.Errorf("SQL() mismatch\n got: %s\nwant: %s", sqlOut, tc.want) + } + + // Re-parse the formatted output (compact and readable styles) + // and check it renders identically. + for _, style := range []ast.FormatOptions{ast.CompactStyle(), ast.ReadableStyle()} { + out := formatter.FormatAST(tree, style) + tree2, err := gosqlx.ParseWithDialect(out, bq) + if err != nil { + t.Fatalf("re-parse %q: %v", out, err) + } + again := formatter.FormatAST(tree2, ast.CompactStyle()) + ast.ReleaseAST(tree2) + if again != tc.want { + t.Errorf("round trip mismatch\n got: %s\nwant: %s", again, tc.want) + } + } + }) + } +} + +// TestPivotItem_ColumnSetQuoting_Issue854 verifies a hand-built UNPIVOT IN +// column set is written verbatim only when well formed. +func TestPivotItem_ColumnSetQuoting_Issue854(t *testing.T) { + cases := []struct { + in, want string + }{ + {"(a, b)", "(a, b)"}, + {"(a, `b c`) AS 'k'", "(a, `b c`) AS 'k'"}, + {"(a, b) AS 1", "(a, b) AS 1"}, + {"(a, (b))", `"(a, (b))"`}, + {"(a, b", `"(a, b"`}, + {"(a,b)", `"(a,b)"`}, + {"(a, b)) --", `"(a, b)) --"`}, + {"()", `"()"`}, + } + for _, tc := range cases { + if got := ast.QuotePivotItem(tc.in); got != tc.want { + t.Errorf("QuotePivotItem(%q) = %q, want %q", tc.in, got, tc.want) + } + } +} diff --git a/pkg/formatter/render.go b/pkg/formatter/render.go index 2b95047b1..69abc0057 100644 --- a/pkg/formatter/render.go +++ b/pkg/formatter/render.go @@ -1432,6 +1432,12 @@ func tableRefSQL(t *ast.TableReference, f *nodeFormatter) string { sb.WriteString(f.kw("PIVOT")) sb.WriteString(" (") sb.WriteString(exprSQL(t.Pivot.AggregateFunction)) + writePivotAggregateAlias(&sb, f, t.Pivot.AggregateAlias) + for _, a := range t.Pivot.ExtraAggregates { + sb.WriteString(", ") + sb.WriteString(exprSQL(a.Function)) + writePivotAggregateAlias(&sb, f, a.Alias) + } sb.WriteString(" ") sb.WriteString(f.kw("FOR")) sb.WriteString(" ") @@ -1445,8 +1451,23 @@ func tableRefSQL(t *ast.TableReference, f *nodeFormatter) string { if t.Unpivot != nil { sb.WriteString(" ") sb.WriteString(f.kw("UNPIVOT")) + if nh := ast.UnpivotNullHandling(t.Unpivot.NullHandling); nh != "" { + sb.WriteString(" ") + sb.WriteString(f.kw(nh)) + } sb.WriteString(" (") - sb.WriteString(ast.QuotePivotColumn(t.Unpivot.ValueColumn)) + if len(t.Unpivot.ValueColumns) > 0 { + sb.WriteString("(") + for i, c := range t.Unpivot.ValueColumns { + if i > 0 { + sb.WriteString(", ") + } + sb.WriteString(ast.QuotePivotColumn(c)) + } + sb.WriteString(")") + } else { + sb.WriteString(ast.QuotePivotColumn(t.Unpivot.ValueColumn)) + } sb.WriteString(" ") sb.WriteString(f.kw("FOR")) sb.WriteString(" ") @@ -1468,6 +1489,18 @@ func tableRefSQL(t *ast.TableReference, f *nodeFormatter) string { return sb.String() } +// writePivotAggregateAlias writes " AS alias" for a PIVOT aggregate alias, +// quoting the alias like a table alias; an empty alias writes nothing. +func writePivotAggregateAlias(sb *strings.Builder, f *nodeFormatter, alias string) { + if alias == "" { + return + } + sb.WriteString(" ") + sb.WriteString(f.kw("AS")) + sb.WriteString(" ") + sb.WriteString(ast.QuoteTableAlias(alias)) +} + // writePivotItems writes PIVOT IN values or UNPIVOT IN columns separated by // ", ", each in its stored source form when well formed and double-quoted // otherwise (see ast.QuotePivotItem). diff --git a/pkg/sql/ast/ast_clauses.go b/pkg/sql/ast/ast_clauses.go index 30f9cacc3..a3efcbc40 100644 --- a/pkg/sql/ast/ast_clauses.go +++ b/pkg/sql/ast/ast_clauses.go @@ -736,35 +736,62 @@ func (c TimeTravelClause) Children() []Node { return nodes } -// PivotClause represents the SQL Server / Oracle PIVOT operator for row-to-column -// transformation in a FROM clause. +// PivotClause represents the SQL Server / Oracle / Snowflake / BigQuery PIVOT +// operator for row-to-column transformation in a FROM clause. // // PIVOT (SUM(sales) FOR region IN ([North], [South], [East], [West])) AS pvt +// PIVOT (SUM(sales) AS total, COUNT(*) AS n FOR quarter IN ('Q1' AS q1, 'Q2' AS q2)) +// +// The first aggregate is AggregateFunction (with optional AggregateAlias); +// Oracle / BigQuery allow further aggregates, which are kept in +// ExtraAggregates in source order. type PivotClause struct { - AggregateFunction Expression // The aggregate function, e.g. SUM(sales) - PivotColumn string // The column used for pivoting, e.g. region - InValues []string // The values to pivot on, e.g. [North], [South] - Pos models.Location // Source position of the PIVOT keyword + AggregateFunction Expression // The aggregate function, e.g. SUM(sales) + AggregateAlias string // Optional alias of AggregateFunction (BigQuery prefix / Oracle alias) + ExtraAggregates []PivotAggregate // Second and later aggregates (Oracle / BigQuery), in source order + PivotColumn string // The column used for pivoting, e.g. region + InValues []string // The values to pivot on, e.g. [North], 'Q1' AS q1 + Pos models.Location // Source position of the PIVOT keyword +} + +// PivotAggregate is an additional aggregate in a multi-aggregate PIVOT, e.g. +// the "COUNT(*) AS n" in PIVOT (SUM(x) AS total, COUNT(*) AS n FOR ...). +type PivotAggregate struct { + Function Expression // The aggregate function call + Alias string // Optional alias; empty when none was given } func (p *PivotClause) expressionNode() {} func (p PivotClause) TokenLiteral() string { return "PIVOT" } func (p PivotClause) Children() []Node { + var nodes []Node if p.AggregateFunction != nil { - return []Node{p.AggregateFunction} + nodes = append(nodes, p.AggregateFunction) } - return nil + for _, a := range p.ExtraAggregates { + if a.Function != nil { + nodes = append(nodes, a.Function) + } + } + return nodes } -// UnpivotClause represents the SQL Server / Oracle UNPIVOT operator for column-to-row -// transformation in a FROM clause. +// UnpivotClause represents the SQL Server / Oracle / Snowflake / BigQuery +// UNPIVOT operator for column-to-row transformation in a FROM clause. // // UNPIVOT (sales FOR region IN (north_sales, south_sales, east_sales)) AS unpvt +// UNPIVOT INCLUDE NULLS ((h1, h2) FOR half IN ((q1, q2) AS 'H1', (q3, q4) AS 'H2')) +// +// In the multi-column form (Oracle / BigQuery) ValueColumns holds the value +// column set and ValueColumn is empty; each InColumns entry is then a +// parenthesized column set such as "(q1, q2) AS 'H1'". type UnpivotClause struct { - ValueColumn string // The target value column, e.g. sales - NameColumn string // The target name column, e.g. region - InColumns []string // The source columns to unpivot, e.g. north_sales, south_sales - Pos models.Location // Source position of the UNPIVOT keyword + NullHandling string // "", "INCLUDE NULLS" or "EXCLUDE NULLS" (Oracle / Snowflake / BigQuery) + ValueColumn string // The target value column, e.g. sales (single-column form) + ValueColumns []string // The target value columns (multi-column form), e.g. h1, h2 + NameColumn string // The target name column, e.g. region + InColumns []string // The source columns to unpivot, e.g. north_sales, south_sales + Pos models.Location // Source position of the UNPIVOT keyword } func (u *UnpivotClause) expressionNode() {} diff --git a/pkg/sql/ast/pool_statement_release.go b/pkg/sql/ast/pool_statement_release.go index ae6980c39..ec09bbede 100644 --- a/pkg/sql/ast/pool_statement_release.go +++ b/pkg/sql/ast/pool_statement_release.go @@ -520,10 +520,15 @@ func releaseTableReference(tr *TableReference, mode releaseMode) { releaseExpression(tr.TableFunc, mode) tr.TableFunc = nil } - // Pivot.AggregateFunction is an Expression. + // Pivot.AggregateFunction and ExtraAggregates hold Expressions. if tr.Pivot != nil { releaseExpression(tr.Pivot.AggregateFunction, mode) tr.Pivot.AggregateFunction = nil + for i := range tr.Pivot.ExtraAggregates { + releaseExpression(tr.Pivot.ExtraAggregates[i].Function, mode) + tr.Pivot.ExtraAggregates[i].Function = nil + } + tr.Pivot.ExtraAggregates = nil tr.Pivot = nil } // Unpivot holds only strings — drop the struct. diff --git a/pkg/sql/ast/sql_table_hints_pivot.go b/pkg/sql/ast/sql_table_hints_pivot.go index 9029e5df2..650a24af0 100644 --- a/pkg/sql/ast/sql_table_hints_pivot.go +++ b/pkg/sql/ast/sql_table_hints_pivot.go @@ -16,6 +16,7 @@ package ast import ( "bytes" + "strings" "unicode" "unicode/utf8" ) @@ -131,9 +132,13 @@ func writePivotItem(buf *bytes.Buffer, item string) { } // isWellFormedPivotItem reports whether item is "atom" or "atom AS atom" -// (see QuotePivotItem). +// (see QuotePivotItem), where the first atom may also be a parenthesized +// column set "(atom, atom, ...)" as in a multi-column UNPIVOT. func isWellFormedPivotItem(item string) bool { n := pivotAtomLen(item) + if n == 0 { + n = pivotColumnSetLen(item) + } if n == 0 { return false } @@ -149,6 +154,30 @@ func isWellFormedPivotItem(item string) bool { return pivotAtomLen(rest) == len(rest) } +// pivotColumnSetLen returns the length of the "(atom, atom, ...)" column set +// at the start of s, or 0 if s does not start with one. Sets do not nest. +func pivotColumnSetLen(s string) int { + if s == "" || s[0] != '(' { + return 0 + } + i := 1 + for { + n := pivotAtomLen(s[i:]) + if n == 0 { + return 0 + } + i += n + switch { + case i < len(s) && s[i] == ')': + return i + 1 + case len(s)-i > 2 && s[i] == ',' && s[i+1] == ' ': + i += 2 + default: + return 0 + } + } +} + // pivotAtomLen returns the length of the atom at the start of s, or 0 if s // does not start with a well-formed atom. A delimited atom must close with // its delimiter; a delimiter inside it must be doubled. In a 'string' a @@ -207,16 +236,23 @@ func pivotAtomLen(s string) int { return i } -// writePivotClause writes " PIVOT (agg FOR col IN (v1, v2))". The pivot -// column is quoted when it is not a plain name, and each IN value is written -// in its stored source form (e.g. [Q1], 'JAN', 1 AS one) when well formed, -// otherwise double-quoted (see QuotePivotItem). +// writePivotClause writes " PIVOT (agg [AS alias][, ...] FOR col IN (v1, v2))". +// The pivot column is quoted when it is not a plain name, aggregate aliases +// are quoted like table aliases, and each IN value is written in its stored +// source form (e.g. [Q1], 'JAN', 1 AS one) when well formed, otherwise +// double-quoted (see QuotePivotItem). func writePivotClause(buf *bytes.Buffer, p *PivotClause) { if p == nil { return } buf.WriteString(" PIVOT (") writeExpr(buf, p.AggregateFunction) + writePivotAggregateAlias(buf, p.AggregateAlias) + for _, a := range p.ExtraAggregates { + buf.WriteString(", ") + writeExpr(buf, a.Function) + writePivotAggregateAlias(buf, a.Alias) + } buf.WriteString(" FOR ") writePivotColumn(buf, p.PivotColumn) buf.WriteString(" IN (") @@ -229,14 +265,40 @@ func writePivotClause(buf *bytes.Buffer, p *PivotClause) { buf.WriteString("))") } -// writeUnpivotClause writes " UNPIVOT (val FOR name IN (c1, c2))", quoting +func writePivotAggregateAlias(buf *bytes.Buffer, alias string) { + if alias == "" { + return + } + buf.WriteString(" AS ") + writeTableAlias(buf, alias) +} + +// writeUnpivotClause writes +// " UNPIVOT [INCLUDE NULLS | EXCLUDE NULLS] (val FOR name IN (c1, c2))", or +// with a parenthesized value column set in the multi-column form, quoting // names and IN columns the same way as writePivotClause. func writeUnpivotClause(buf *bytes.Buffer, u *UnpivotClause) { if u == nil { return } - buf.WriteString(" UNPIVOT (") - writePivotColumn(buf, u.ValueColumn) + buf.WriteString(" UNPIVOT ") + if nh := UnpivotNullHandling(u.NullHandling); nh != "" { + buf.WriteString(nh) + buf.WriteByte(' ') + } + buf.WriteByte('(') + if len(u.ValueColumns) > 0 { + buf.WriteByte('(') + for i, c := range u.ValueColumns { + if i > 0 { + buf.WriteString(", ") + } + writePivotColumn(buf, c) + } + buf.WriteByte(')') + } else { + writePivotColumn(buf, u.ValueColumn) + } buf.WriteString(" FOR ") writePivotColumn(buf, u.NameColumn) buf.WriteString(" IN (") @@ -248,3 +310,16 @@ func writeUnpivotClause(buf *bytes.Buffer, u *UnpivotClause) { } buf.WriteString("))") } + +// UnpivotNullHandling normalizes an UnpivotClause.NullHandling value to +// "INCLUDE NULLS" or "EXCLUDE NULLS". Any other value (only possible in a +// hand-built AST) yields "" so it is never written raw. +func UnpivotNullHandling(s string) string { + switch { + case strings.EqualFold(s, "INCLUDE NULLS"), strings.EqualFold(s, "INCLUDE"): + return "INCLUDE NULLS" + case strings.EqualFold(s, "EXCLUDE NULLS"), strings.EqualFold(s, "EXCLUDE"): + return "EXCLUDE NULLS" + } + return "" +} diff --git a/pkg/sql/parser/bigquery_pivot_test.go b/pkg/sql/parser/bigquery_pivot_test.go new file mode 100644 index 000000000..8324720ec --- /dev/null +++ b/pkg/sql/parser/bigquery_pivot_test.go @@ -0,0 +1,361 @@ +// 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 parser_test + +import ( + "reflect" + "testing" + + "github.com/ajitpratap0/GoSQLX/pkg/gosqlx" + "github.com/ajitpratap0/GoSQLX/pkg/sql/ast" + "github.com/ajitpratap0/GoSQLX/pkg/sql/keywords" +) + +// firstTableRef parses sql in dialect d and returns the first FROM table +// reference of the single SELECT statement. +func firstTableRef(t *testing.T, sql string, d keywords.SQLDialect) ast.TableReference { + t.Helper() + tree, err := gosqlx.ParseWithDialect(sql, d) + if err != nil { + t.Fatalf("ParseWithDialect(%s) %q: %v", d, sql, err) + } + t.Cleanup(func() { ast.ReleaseAST(tree) }) + if len(tree.Statements) != 1 { + t.Fatalf("expected 1 statement, got %d", len(tree.Statements)) + } + sel, ok := tree.Statements[0].(*ast.SelectStatement) + if !ok { + t.Fatalf("expected *ast.SelectStatement, got %T", tree.Statements[0]) + } + if len(sel.From) == 0 { + t.Fatal("expected a FROM table reference") + } + return sel.From[0] +} + +// TestBigQueryPivot_Issue854 verifies BigQuery PIVOT is parsed now that the +// parser gates PIVOT/UNPIVOT on Capabilities().SupportsPivotUnpivot. +func TestBigQueryPivot_Issue854(t *testing.T) { + bq := keywords.DialectBigQuery + tests := []struct { + name string + sql string + aggAlias string + extraAggs []string // aliases of the extra aggregates + pivotCol string + inValues []string + tableAlias string + }{ + { + name: "issue repro", + sql: "SELECT * FROM sales PIVOT (SUM(amount) FOR quarter IN ('Q1', 'Q2'))", + pivotCol: "quarter", + inValues: []string{"'Q1'", "'Q2'"}, + }, + { + name: "IN value aliases with and without AS, table alias", + sql: "SELECT * FROM sales PIVOT (SUM(amount) FOR quarter IN ('Q1' AS first_quarter, 'Q2' second_quarter)) AS pivoted", + pivotCol: "quarter", + inValues: []string{"'Q1' AS first_quarter", "'Q2' AS second_quarter"}, + tableAlias: "pivoted", + }, + { + name: "multiple aliased aggregates", + sql: "SELECT * FROM produce PIVOT (SUM(sales) AS total_sales, COUNT(*) AS num_records FOR quarter IN ('Q1', 'Q2'))", + aggAlias: "total_sales", + extraAggs: []string{"num_records"}, + pivotCol: "quarter", + inValues: []string{"'Q1'", "'Q2'"}, + }, + { + name: "aggregate alias without AS, unaliased extra aggregate", + sql: "SELECT * FROM produce PIVOT (SUM(sales) s, AVG(sales) FOR quarter IN (1, 2)) p", + aggAlias: "s", + extraAggs: []string{""}, + pivotCol: "quarter", + inValues: []string{"1", "2"}, + tableAlias: "p", + }, + { + name: "backtick quoted names", + sql: "SELECT * FROM `proj.ds.sales` PIVOT (SUM(amount) FOR `quarter` IN ('Q1' AS `q 1`))", + pivotCol: "quarter", + inValues: []string{"'Q1' AS `q 1`"}, + }, + { + name: "subquery source", + sql: "SELECT * FROM (SELECT product, sales, quarter FROM produce) PIVOT (SUM(sales) FOR quarter IN ('Q1', 'Q2'))", + pivotCol: "quarter", + inValues: []string{"'Q1'", "'Q2'"}, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + ref := firstTableRef(t, tc.sql, bq) + if ref.Pivot == nil { + t.Fatal("expected Pivot clause") + } + pv := ref.Pivot + if pv.AggregateFunction == nil { + t.Fatal("expected AggregateFunction") + } + if pv.AggregateAlias != tc.aggAlias { + t.Errorf("AggregateAlias = %q, want %q", pv.AggregateAlias, tc.aggAlias) + } + if len(pv.ExtraAggregates) != len(tc.extraAggs) { + t.Fatalf("ExtraAggregates = %d, want %d", len(pv.ExtraAggregates), len(tc.extraAggs)) + } + for i, a := range pv.ExtraAggregates { + if a.Function == nil { + t.Errorf("ExtraAggregates[%d].Function is nil", i) + } + if a.Alias != tc.extraAggs[i] { + t.Errorf("ExtraAggregates[%d].Alias = %q, want %q", i, a.Alias, tc.extraAggs[i]) + } + } + if pv.PivotColumn != tc.pivotCol { + t.Errorf("PivotColumn = %q, want %q", pv.PivotColumn, tc.pivotCol) + } + if !reflect.DeepEqual(pv.InValues, tc.inValues) { + t.Errorf("InValues = %q, want %q", pv.InValues, tc.inValues) + } + if ref.Alias != tc.tableAlias { + t.Errorf("Alias = %q, want %q", ref.Alias, tc.tableAlias) + } + if got := len(pv.Children()); got != 1+len(tc.extraAggs) { + t.Errorf("Children() = %d nodes, want %d", got, 1+len(tc.extraAggs)) + } + }) + } +} + +// TestBigQueryUnpivot_Issue854 verifies BigQuery UNPIVOT, including the +// INCLUDE/EXCLUDE NULLS modifier, row-value aliases with and without AS and +// the multi-column form. +func TestBigQueryUnpivot_Issue854(t *testing.T) { + bq := keywords.DialectBigQuery + tests := []struct { + name string + sql string + nullHandling string + valueCol string + valueCols []string + nameCol string + inColumns []string + tableAlias string + }{ + { + name: "basic", + sql: "SELECT * FROM produce UNPIVOT (sales FOR quarter IN (Q1, Q2, Q3, Q4))", + valueCol: "sales", + nameCol: "quarter", + inColumns: []string{"Q1", "Q2", "Q3", "Q4"}, + }, + { + name: "INCLUDE NULLS with row value aliases and table alias", + sql: "SELECT * FROM produce UNPIVOT INCLUDE NULLS (sales FOR quarter IN (Q1 AS 'first', Q2 'second')) AS u", + nullHandling: "INCLUDE NULLS", + valueCol: "sales", + nameCol: "quarter", + inColumns: []string{"Q1 AS 'first'", "Q2 AS 'second'"}, + tableAlias: "u", + }, + { + name: "EXCLUDE NULLS with INT64 row value aliases", + sql: "SELECT * FROM produce UNPIVOT exclude nulls (sales FOR quarter IN (Q1 AS 1, Q2 2))", + nullHandling: "EXCLUDE NULLS", + valueCol: "sales", + nameCol: "quarter", + inColumns: []string{"Q1 AS 1", "Q2 AS 2"}, + }, + { + name: "multi-column", + sql: "SELECT * FROM produce UNPIVOT ((first_half_sales, second_half_sales) FOR semesters IN ((Q1, Q2) AS 'semester_1', (Q3, Q4) AS 'semester_2'))", + valueCols: []string{"first_half_sales", "second_half_sales"}, + nameCol: "semesters", + inColumns: []string{"(Q1, Q2) AS 'semester_1'", "(Q3, Q4) AS 'semester_2'"}, + }, + { + name: "multi-column with NULLS modifier, unaliased sets and alias", + sql: "SELECT * FROM produce UNPIVOT INCLUDE NULLS ((a, b) FOR k IN ((Q1, Q2), (Q3, Q4))) unp", + nullHandling: "INCLUDE NULLS", + valueCols: []string{"a", "b"}, + nameCol: "k", + inColumns: []string{"(Q1, Q2)", "(Q3, Q4)"}, + tableAlias: "unp", + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + ref := firstTableRef(t, tc.sql, bq) + if ref.Unpivot == nil { + t.Fatal("expected Unpivot clause") + } + u := ref.Unpivot + if u.NullHandling != tc.nullHandling { + t.Errorf("NullHandling = %q, want %q", u.NullHandling, tc.nullHandling) + } + if u.ValueColumn != tc.valueCol { + t.Errorf("ValueColumn = %q, want %q", u.ValueColumn, tc.valueCol) + } + if !reflect.DeepEqual(u.ValueColumns, tc.valueCols) { + t.Errorf("ValueColumns = %q, want %q", u.ValueColumns, tc.valueCols) + } + if u.NameColumn != tc.nameCol { + t.Errorf("NameColumn = %q, want %q", u.NameColumn, tc.nameCol) + } + if !reflect.DeepEqual(u.InColumns, tc.inColumns) { + t.Errorf("InColumns = %q, want %q", u.InColumns, tc.inColumns) + } + if ref.Alias != tc.tableAlias { + t.Errorf("Alias = %q, want %q", ref.Alias, tc.tableAlias) + } + }) + } +} + +// TestBigQueryPivot_Invalid_Issue854 verifies malformed BigQuery PIVOT / +// UNPIVOT is still rejected. +func TestBigQueryPivot_Invalid_Issue854(t *testing.T) { + bad := []string{ + "SELECT * FROM t PIVOT (SUM(x) FOR q IN ())", + "SELECT * FROM t PIVOT (SUM(x) AS FOR q IN ('a'))", + "SELECT * FROM t UNPIVOT ((a, b) FOR k IN ((c1, c2, c3) AS 'x'))", + "SELECT * FROM t UNPIVOT ((a, b) FOR k IN (c1, c2))", + "SELECT * FROM t UNPIVOT ((a, b FOR k IN ((c1, c2)))", + "SELECT * FROM t UNPIVOT INCLUDE (v FOR k IN (c1))", + "SELECT * FROM t PIVOT (SUM(x) FOR q IN ('a' AS))", + "SELECT * FROM t PIVOT (SUM(x) FOR q IN ('a' 'b'))", + "SELECT * FROM t UNPIVOT (v FOR k IN (c1 AS))", + "SELECT * FROM t UNPIVOT (v FOR k IN (c1 c2))", + } + for _, q := range bad { + t.Run(q, func(t *testing.T) { + tree, err := gosqlx.ParseWithDialect(q, keywords.DialectBigQuery) + if err == nil { + ast.ReleaseAST(tree) + t.Fatalf("expected parse error for %q", q) + } + }) + } +} + +// TestPivot_DialectGating_Issue854 pins which dialects recognise PIVOT / +// UNPIVOT now that the gate is Capabilities().SupportsPivotUnpivot: the +// default (no dialect), Generic, PostgreSQL, MySQL, MariaDB, SQLite, +// ClickHouse and Redshift must keep rejecting the clause, and the BigQuery / +// Oracle-only extensions must stay rejected in SQL Server and Snowflake. +func TestPivot_DialectGating_Issue854(t *testing.T) { + const pivotSQL = "SELECT * FROM sales PIVOT (SUM(amount) FOR quarter IN ('Q1', 'Q2'))" + const unpivotSQL = "SELECT * FROM sales UNPIVOT (amount FOR quarter IN (q1, q2))" + + t.Run("default dialect rejects", func(t *testing.T) { + for _, q := range []string{pivotSQL, unpivotSQL} { + if tree, err := gosqlx.Parse(q); err == nil { + ast.ReleaseAST(tree) + t.Errorf("default dialect: expected %q to be rejected", q) + } + } + }) + + rejecting := []keywords.SQLDialect{ + keywords.DialectGeneric, + keywords.DialectPostgreSQL, + keywords.DialectMySQL, + keywords.DialectMariaDB, + keywords.DialectSQLite, + keywords.DialectClickHouse, + keywords.DialectRedshift, + } + for _, d := range rejecting { + t.Run(string(d)+" rejects", func(t *testing.T) { + for _, q := range []string{pivotSQL, unpivotSQL} { + if tree, err := gosqlx.ParseWithDialect(q, d); err == nil { + ast.ReleaseAST(tree) + t.Errorf("%s: expected %q to be rejected", d, q) + } + } + }) + } + + accepting := []keywords.SQLDialect{ + keywords.DialectSQLServer, + keywords.DialectOracle, + keywords.DialectSnowflake, + keywords.DialectBigQuery, + } + for _, d := range accepting { + t.Run(string(d)+" accepts", func(t *testing.T) { + for _, q := range []string{pivotSQL, unpivotSQL} { + tree, err := gosqlx.ParseWithDialect(q, d) + if err != nil { + t.Fatalf("%s: %q: %v", d, q, err) + } + ast.ReleaseAST(tree) + } + }) + } + + // "pivot" stays usable as a plain table alias where PIVOT is not a clause. + t.Run("pivot as alias in PostgreSQL", func(t *testing.T) { + ref := firstTableRef(t, "SELECT * FROM sales pivot", keywords.DialectPostgreSQL) + if ref.Pivot != nil || ref.Alias != "pivot" { + t.Fatalf("expected alias %q and no Pivot, got alias %q pivot %v", "pivot", ref.Alias, ref.Pivot) + } + }) + + extensions := []struct { + name string + sql string + d keywords.SQLDialect + }{ + {"sqlserver multiple aggregates", "SELECT * FROM t PIVOT (SUM(x), COUNT(*) FOR q IN ([a]))", keywords.DialectSQLServer}, + {"sqlserver aggregate alias", "SELECT * FROM t PIVOT (SUM(x) AS s FOR q IN ([a]))", keywords.DialectSQLServer}, + {"sqlserver bare IN alias", "SELECT * FROM t PIVOT (SUM(x) FOR q IN ([a] b))", keywords.DialectSQLServer}, + {"sqlserver INCLUDE NULLS", "SELECT * FROM t UNPIVOT INCLUDE NULLS (v FOR k IN (c1))", keywords.DialectSQLServer}, + {"snowflake multiple aggregates", "SELECT * FROM t PIVOT (SUM(x), COUNT(*) FOR q IN ('a'))", keywords.DialectSnowflake}, + {"snowflake multi-column unpivot", "SELECT * FROM t UNPIVOT ((a, b) FOR k IN ((c1, c2)))", keywords.DialectSnowflake}, + {"oracle bare row value alias", "SELECT * FROM t UNPIVOT (v FOR k IN (c1 'x'))", keywords.DialectOracle}, + } + for _, tc := range extensions { + t.Run(tc.name+" rejected", func(t *testing.T) { + if tree, err := gosqlx.ParseWithDialect(tc.sql, tc.d); err == nil { + ast.ReleaseAST(tree) + t.Fatalf("%s: expected %q to be rejected", tc.d, tc.sql) + } + }) + } + + accepted := []struct { + name string + sql string + d keywords.SQLDialect + }{ + {"oracle multiple aggregates", "SELECT * FROM t PIVOT (SUM(x) AS s, COUNT(*) AS n FOR q IN (1 AS a, 2 AS b))", keywords.DialectOracle}, + {"oracle multi-column unpivot", "SELECT * FROM t UNPIVOT ((a, b) FOR k IN ((c1, c2) AS 'x'))", keywords.DialectOracle}, + {"oracle INCLUDE NULLS", "SELECT * FROM t UNPIVOT INCLUDE NULLS (v FOR k IN (c1))", keywords.DialectOracle}, + {"snowflake INCLUDE NULLS", "SELECT * FROM t UNPIVOT INCLUDE NULLS (v FOR k IN (c1, c2))", keywords.DialectSnowflake}, + {"snowflake bare IN alias", "SELECT * FROM t PIVOT (SUM(x) FOR q IN ('a' qa))", keywords.DialectSnowflake}, + } + for _, tc := range accepted { + t.Run(tc.name+" accepted", func(t *testing.T) { + tree, err := gosqlx.ParseWithDialect(tc.sql, tc.d) + if err != nil { + t.Fatalf("%s: %q: %v", tc.d, tc.sql, err) + } + ast.ReleaseAST(tree) + }) + } +} diff --git a/pkg/sql/parser/pivot.go b/pkg/sql/parser/pivot.go index 4fbfc3a21..c197ba2a3 100644 --- a/pkg/sql/parser/pivot.go +++ b/pkg/sql/parser/pivot.go @@ -13,7 +13,7 @@ // limitations under the License. // Package parser - pivot.go -// SQL Server / Oracle PIVOT and UNPIVOT clause parsing. +// SQL Server / Oracle / Snowflake / BigQuery PIVOT and UNPIVOT clause parsing. package parser @@ -46,17 +46,35 @@ func renderQuotedIdent(tok models.Token) string { return tok.Value } +// renderCurrentIdent renders the current identifier token in source form via +// renderQuotedIdent. The tokenizer records Token.Quote for double-quoted and +// bracketed identifiers but leaves it zero for backtick identifiers (MySQL, +// BigQuery, ...), so an identifier token whose source span is wider than its +// value, or spans lines, is taken to have been backtick-quoted. +func (p *Parser) renderCurrentIdent() string { + tok := p.currentToken.Token + if tok.Quote == 0 && tok.Type == models.TokenTypeIdentifier && + (tok.Word == nil || tok.Word.QuoteStyle == 0) { + start, end := p.currentToken.Start, p.currentToken.End + if start.Line != end.Line || + (end.Column-start.Column > len(tok.Value)) { + tok.Quote = '`' + } + } + return renderQuotedIdent(tok) +} + // renderPivotValue reproduces a PIVOT/UNPIVOT IN-list token in source form: // a string literal is re-quoted and escaped the same way LiteralValue.SQL() // writes strings, a quoted identifier keeps its delimiters -// (renderQuotedIdent), and anything else (a bare name or number) is returned +// (renderCurrentIdent), and anything else (a bare name or number) is returned // as is. func (p *Parser) renderPivotValue() string { if p.isStringLiteral() { lit := ast.LiteralValue{Value: p.currentToken.Token.Value, Type: "STRING"} return lit.SQL() } - return renderQuotedIdent(p.currentToken.Token) + return p.renderCurrentIdent() } // parsePivotAlias consumes an optional alias (with or without AS) following a @@ -78,16 +96,31 @@ func (p *Parser) parsePivotAlias(ref *ast.TableReference) { } // pivotDialectAllowed reports whether PIVOT/UNPIVOT is a recognized clause -// for the parser's current dialect. PIVOT/UNPIVOT are SQL Server, Oracle, -// and Snowflake extensions; in other dialects the words must remain valid -// identifiers. +// for the parser's current dialect, i.e. whether the dialect's capability +// matrix sets SupportsPivotUnpivot (SQL Server, Oracle, Snowflake and +// BigQuery). In other dialects - including the default Unknown dialect and +// Generic - the words must remain valid identifiers. func (p *Parser) pivotDialectAllowed() bool { - return p.dialect == string(keywords.DialectSQLServer) || - p.dialect == string(keywords.DialectOracle) || - p.dialect == string(keywords.DialectSnowflake) + return p.Capabilities().SupportsPivotUnpivot +} + +// pivotMultiAggregateAllowed reports whether the dialect accepts aliased +// and multiple aggregates in PIVOT (Oracle, BigQuery): +// +// PIVOT (SUM(x) AS total, COUNT(*) AS n FOR col IN (...)) +// +// It also gates the multi-column UNPIVOT form, which the same two dialects +// support. +func (p *Parser) pivotMultiAggregateAllowed() bool { + return p.IsOracle() || p.IsBigQuery() +} + +// unpivotNullHandlingAllowed reports whether the dialect accepts +// UNPIVOT { INCLUDE | EXCLUDE } NULLS (Oracle, Snowflake, BigQuery). +func (p *Parser) unpivotNullHandlingAllowed() bool { + return p.IsOracle() || p.IsSnowflake() || p.IsBigQuery() } -// isPivotKeyword returns true if the current token is the contextual PIVOT // isQualifyKeyword returns true if the current token is the Snowflake / // BigQuery QUALIFY clause keyword. QUALIFY tokenizes as an identifier, so // detect by value and gate by dialect to avoid consuming a legitimate @@ -103,6 +136,7 @@ func (p *Parser) isQualifyKeyword() bool { return strings.EqualFold(p.currentToken.Token.Value, "QUALIFY") } +// isPivotKeyword returns true if the current token is the contextual PIVOT // keyword in a dialect that supports it. PIVOT is non-reserved, so it may // arrive as either an identifier or a keyword token. func (p *Parser) isPivotKeyword() bool { @@ -139,11 +173,32 @@ func (p *Parser) parsePivotClause() (*ast.PivotClause, error) { } p.advance() // consume ( - // Parse aggregate function expression (e.g. SUM(sales)) + // Parse aggregate function expression (e.g. SUM(sales)). Oracle and + // BigQuery also allow an alias on each aggregate and several + // comma-separated aggregates. aggFunc, err := p.parseExpression() if err != nil { return nil, err } + var aggAlias string + var extraAggs []ast.PivotAggregate + if p.pivotMultiAggregateAllowed() { + if aggAlias, err = p.parsePivotAggregateAlias(); err != nil { + return nil, err + } + for p.isType(models.TokenTypeComma) { + p.advance() // consume , + fn, err := p.parseExpression() + if err != nil { + return nil, err + } + alias, err := p.parsePivotAggregateAlias() + if err != nil { + return nil, err + } + extraAggs = append(extraAggs, ast.PivotAggregate{Function: fn, Alias: alias}) + } + } // Expect FOR keyword if !p.isType(models.TokenTypeFor) { @@ -179,17 +234,25 @@ func (p *Parser) parsePivotClause() (*ast.PivotClause, error) { } val := p.renderPivotValue() p.advance() - // Optional alias: AS + // Optional alias: [AS] . Oracle, Snowflake and BigQuery allow + // the alias without AS; SQL Server has no IN-value aliases, so the + // bare form stays rejected there. if p.isType(models.TokenTypeAs) { p.advance() // consume AS - if p.isIdentifier() || p.isNonReservedKeyword() { - val += " AS " + renderQuotedIdent(p.currentToken.Token) - p.advance() + if !p.isIdentifier() && !p.isNonReservedKeyword() { + return nil, p.expectedError("alias after AS in PIVOT IN list") } + val += " AS " + p.renderCurrentIdent() + p.advance() + } else if p.isIdentifier() && !p.IsSQLServer() { + val += " AS " + p.renderCurrentIdent() + p.advance() } inValues = append(inValues, val) if p.isType(models.TokenTypeComma) { p.advance() + } else if !p.isType(models.TokenTypeRParen) { + return nil, p.expectedError(", or ) in PIVOT IN list") } } @@ -208,29 +271,81 @@ func (p *Parser) parsePivotClause() (*ast.PivotClause, error) { return &ast.PivotClause{ AggregateFunction: aggFunc, + AggregateAlias: aggAlias, + ExtraAggregates: extraAggs, PivotColumn: pivotCol, InValues: inValues, Pos: pos, }, nil } -// parseUnpivotClause parses UNPIVOT (value_col FOR name_col IN (columns)). -// The current token must be the UNPIVOT keyword. +// parsePivotAggregateAlias consumes an optional "[AS] alias" after a PIVOT +// aggregate (Oracle / BigQuery) and returns the alias, or "" when none. +// AS with no alias after it is an error. +func (p *Parser) parsePivotAggregateAlias() (string, error) { + if p.isType(models.TokenTypeAs) { + p.advance() // consume AS + if !p.isIdentifier() && !p.isNonReservedKeyword() { + return "", p.expectedError("alias after AS in PIVOT aggregate") + } + alias := p.currentToken.Token.Value + p.advance() + return alias, nil + } + if p.isIdentifier() { + alias := p.currentToken.Token.Value + p.advance() + return alias, nil + } + return "", nil +} + +// parseUnpivotClause parses +// +// UNPIVOT [{INCLUDE | EXCLUDE} NULLS] (value_col FOR name_col IN (columns)) +// UNPIVOT [{INCLUDE | EXCLUDE} NULLS] ((v1, v2) FOR name_col IN ((c1, c2) [AS] 'k', ...)) +// +// The NULLS modifier is accepted for Oracle, Snowflake and BigQuery and the +// multi-column form for Oracle and BigQuery. The current token must be the +// UNPIVOT keyword. func (p *Parser) parseUnpivotClause() (*ast.UnpivotClause, error) { pos := p.currentLocation() p.advance() // consume UNPIVOT + var nullHandling string + if p.unpivotNullHandlingAllowed() { + v := p.currentToken.Token.Value + if (strings.EqualFold(v, "INCLUDE") || strings.EqualFold(v, "EXCLUDE")) && + strings.EqualFold(p.peekToken().Token.Value, "NULLS") { + nullHandling = strings.ToUpper(v) + " NULLS" + p.advance() // consume INCLUDE / EXCLUDE + p.advance() // consume NULLS + } + } + if !p.isType(models.TokenTypeLParen) { return nil, p.expectedError("( after UNPIVOT") } p.advance() // consume ( - // Parse value column name - if !p.isIdentifier() { - return nil, p.expectedError("value column name in UNPIVOT") + multiColumn := p.isType(models.TokenTypeLParen) && p.pivotMultiAggregateAllowed() + + // Parse value column name, or the (v1, v2, ...) value column set. + var valueCol string + var valueCols []string + if multiColumn { + cols, err := p.parseUnpivotNameList("value column name in UNPIVOT", false) + if err != nil { + return nil, err + } + valueCols = cols + } else { + if !p.isIdentifier() { + return nil, p.expectedError("value column name in UNPIVOT") + } + valueCol = p.currentToken.Token.Value + p.advance() } - valueCol := p.currentToken.Token.Value - p.advance() // Expect FOR keyword if !p.isType(models.TokenTypeFor) { @@ -257,26 +372,49 @@ func (p *Parser) parseUnpivotClause() (*ast.UnpivotClause, error) { } p.advance() // consume ( - // Parse IN columns — each may have an optional AS alias (Oracle: - // north_sales AS 'North'). + // Parse IN columns (or column sets in the multi-column form) — each may + // have an optional alias: Oracle north_sales AS 'North'; BigQuery also + // allows the alias without AS and an INT64 alias. var cols []string for !p.isType(models.TokenTypeRParen) && !p.isType(models.TokenTypeEOF) { - if !p.isIdentifier() { - return nil, p.expectedError("column name in UNPIVOT IN list") + var col string + if multiColumn { + if !p.isType(models.TokenTypeLParen) { + return nil, p.expectedError("( to open a column set in UNPIVOT IN list") + } + set, err := p.parseUnpivotNameList("column name in UNPIVOT IN list", true) + if err != nil { + return nil, err + } + if len(set) != len(valueCols) { + return nil, p.expectedError( + "a column set with as many columns as the UNPIVOT value columns") + } + col = "(" + strings.Join(set, ", ") + ")" + } else { + if !p.isIdentifier() { + return nil, p.expectedError("column name in UNPIVOT IN list") + } + col = p.renderCurrentIdent() + p.advance() } - col := renderQuotedIdent(p.currentToken.Token) - p.advance() - // Optional alias: AS if p.isType(models.TokenTypeAs) { p.advance() // consume AS - if p.isStringLiteral() || p.isIdentifier() || p.isNonReservedKeyword() { - col += " AS " + p.renderPivotValue() - p.advance() + if !p.isStringLiteral() && !p.isIdentifier() && !p.isNonReservedKeyword() && + !p.isType(models.TokenTypeNumber) { + return nil, p.expectedError("alias after AS in UNPIVOT IN list") } + col += " AS " + p.renderPivotValue() + p.advance() + } else if p.IsBigQuery() && (p.isStringLiteral() || p.isType(models.TokenTypeNumber)) { + col += " AS " + p.renderPivotValue() + p.advance() } cols = append(cols, col) if p.isType(models.TokenTypeComma) { p.advance() + } else if !p.isType(models.TokenTypeRParen) { + return nil, p.expectedError(", or ) in UNPIVOT IN list") } } @@ -294,13 +432,46 @@ func (p *Parser) parseUnpivotClause() (*ast.UnpivotClause, error) { p.advance() // close UNPIVOT ) return &ast.UnpivotClause{ - ValueColumn: valueCol, - NameColumn: nameCol, - InColumns: cols, - Pos: pos, + NullHandling: nullHandling, + ValueColumn: valueCol, + ValueColumns: valueCols, + NameColumn: nameCol, + InColumns: cols, + Pos: pos, }, nil } +// parseUnpivotNameList parses a parenthesized, comma-separated list of +// column names, "(a, b, ...)", for the multi-column UNPIVOT form. With +// sourceForm set each name is returned as written (quoted identifiers keep +// their delimiters, as InColumns entries do); otherwise the bare name value +// is returned, as ValueColumn / ValueColumns store it. The current token +// must be the opening parenthesis. +func (p *Parser) parseUnpivotNameList(what string, sourceForm bool) ([]string, error) { + p.advance() // consume ( + var names []string + for { + if !p.isIdentifier() { + return nil, p.expectedError(what) + } + if sourceForm { + names = append(names, p.renderCurrentIdent()) + } else { + names = append(names, p.currentToken.Token.Value) + } + p.advance() + if !p.isType(models.TokenTypeComma) { + break + } + p.advance() // consume , + } + if !p.isType(models.TokenTypeRParen) { + return nil, p.expectedError(") to close UNPIVOT column set") + } + p.advance() // consume ) + return names, nil +} + // supportsTableFunction reports whether the current dialect allows // function-call style table references in the FROM list — Snowflake // (FLATTEN, TABLE, IDENTIFIER, GENERATOR), BigQuery (UNNEST), From accbfe5cc4ddf4700c581a0646000bb4285831ed Mon Sep 17 00:00:00 2001 From: Ajit Pratap Singh Date: Fri, 23 Oct 2026 00:14:19 +0530 Subject: [PATCH 2/3] fix(parser): preserve quoted PIVOT and UNPIVOT names Retain source delimiters for first and additional aggregate aliases and single/multi-column UNPIVOT value names in AST and formatter output. Keep the existing quoting fallback for hand-built names. Pin backtick token byte spans for Unicode and doubled delimiters, add BigQuery and Oracle round trips, and clarify IN-list alias behavior. Co-Authored-By: Codex Codex-Session: 01a164e5-e59a-7281-b5b1-efc545a2f622 --- CHANGELOG.md | 10 ++-- docs/SQL_COMPATIBILITY.md | 2 +- pkg/formatter/bigquery_pivot_render_test.go | 60 ++++++++++++++++++++- pkg/formatter/render.go | 9 ++-- pkg/sql/ast/ast_clauses.go | 8 +-- pkg/sql/ast/sql_table_hints_pivot.go | 46 +++++++++++++--- pkg/sql/parser/bigquery_pivot_test.go | 13 +++++ pkg/sql/parser/pivot.go | 25 ++++----- pkg/sql/tokenizer/quoted_zero_copy_test.go | 4 ++ 9 files changed, 140 insertions(+), 37 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 84cdb68da..d35beba21 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,8 +21,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 dialect's grammar: Oracle also gets aliased/multiple aggregates, the NULLS modifier and multi-column UNPIVOT; Snowflake gets the NULLS modifier and bare `IN` value aliases; SQL Server is unchanged. -- PIVOT/UNPIVOT backtick-quoted `IN` names and aliases keep their backticks - when rendered (they were written bare, which did not re-parse). +- PIVOT/UNPIVOT quoted aggregate aliases, UNPIVOT value columns, and + backtick-quoted `IN` names and aliases keep their source delimiters when + rendered. This preserves BigQuery backticks and Oracle alias case. ### Added - `ast.PivotClause.AggregateAlias`, `ast.PivotClause.ExtraAggregates` @@ -32,8 +33,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Behavior changes - PIVOT / UNPIVOT `IN` lists now require commas between entries and an alias - after `AS` in every PIVOT dialect (`IN (a b)` used to parse as two entries - and `IN ('a' AS)` silently dropped the `AS`). + after `AS` in every PIVOT dialect. Where bare aliases are supported, + `IN (a b)` is one entry (`a AS b`); otherwise a missing comma is rejected. + `IN ('a' AS)` no longer silently drops the `AS`. ## [1.16.0] - 2026-10-22 — Performance Audit, SQL Highlighting, Dialect-Aware LSP, Claude Code Plugin & ChatGPT App, Correctness Sweep diff --git a/docs/SQL_COMPATIBILITY.md b/docs/SQL_COMPATIBILITY.md index c2ae72095..f211305ec 100644 --- a/docs/SQL_COMPATIBILITY.md +++ b/docs/SQL_COMPATIBILITY.md @@ -719,7 +719,7 @@ gosqlx format --dialect mysql query.sql - PIVOT / UNPIVOT (unreleased, #854): aggregate aliases and multiple aggregates (`PIVOT (SUM(s) AS total, COUNT(*) AS n FOR q IN ('Q1' AS q1, 'Q2' q2))`), `UNPIVOT INCLUDE NULLS` / `EXCLUDE NULLS`, row-value aliases with or without `AS` (string or INT64), and multi-column UNPIVOT (`UNPIVOT ((a, b) FOR k IN ((c1, c2) AS 'x', (c3, c4) AS 'y'))`) - PIVOT `IN` values must be literals or names; constant expressions such as `IN (1 + 1)` or `IN (DATE '2024-01-01')` are not parsed - QUALIFY, `UNNEST` in FROM and `(struct).field` access are capability-gated; BigQuery has no dedicated keyword set -- Aggregate aliases with characters that need quoting are re-rendered in ANSI double quotes, as all GoSQLX aliases are +- Quoted PIVOT aggregate aliases and UNPIVOT value-column names retain their source delimiters, including BigQuery backticks and Oracle double quotes Oracle also accepts aggregate aliases, multiple aggregates, `INCLUDE`/`EXCLUDE NULLS` and multi-column UNPIVOT; Snowflake accepts `INCLUDE`/`EXCLUDE NULLS`. SQL Server keeps the single-aggregate form without aliases or a NULLS modifier. Oracle's tuple row-value aliases (`(c1, c2) AS ('a', 'b')`) are not parsed. diff --git a/pkg/formatter/bigquery_pivot_render_test.go b/pkg/formatter/bigquery_pivot_render_test.go index e0fc89c06..bb1d19fe6 100644 --- a/pkg/formatter/bigquery_pivot_render_test.go +++ b/pkg/formatter/bigquery_pivot_render_test.go @@ -47,8 +47,11 @@ func TestBigQueryPivot_FormatRoundTrip_Issue854(t *testing.T) { { "backtick_alias", "SELECT * FROM sales PIVOT (SUM(amount) AS `total amount` FOR quarter IN ('Q1' AS `q 1`))", - "SELECT * FROM sales PIVOT (SUM(amount) AS \"total amount\" FOR quarter IN ('Q1' AS `q 1`))", + "SELECT * FROM sales PIVOT (SUM(amount) AS `total amount` FOR quarter IN ('Q1' AS `q 1`))", }, + {"backtick_first_and_extra_alias", "SELECT * FROM sales PIVOT (SUM(amount) AS `日本`, COUNT(*) `a``b` FOR quarter IN ('Q1'))", "SELECT * FROM sales PIVOT (SUM(amount) AS `日本`, COUNT(*) AS `a``b` FOR quarter IN ('Q1'))"}, + {"unpivot_backtick_value", "SELECT * FROM sales UNPIVOT (`日本` FOR quarter IN (Q1, Q2))", "SELECT * FROM sales UNPIVOT (`日本` FOR quarter IN (Q1, Q2))"}, + {"unpivot_backtick_value_set", "SELECT * FROM sales UNPIVOT ((`日本`, `a``b`) FOR quarter IN ((Q1, Q2)))", "SELECT * FROM sales UNPIVOT ((`日本`, `a``b`) FOR quarter IN ((Q1, Q2)))"}, { "unpivot_include_nulls", "SELECT * FROM produce UNPIVOT INCLUDE NULLS (sales FOR quarter IN (Q1 'first', Q2 AS 2)) AS u", @@ -122,3 +125,58 @@ func TestPivotItem_ColumnSetQuoting_Issue854(t *testing.T) { } } } + +func TestOraclePivotQuotedAggregateAliasRoundTrip(t *testing.T) { + const want = `SELECT * FROM sales PIVOT (SUM(amount) AS "Total", COUNT(*) AS "Count" FOR quarter IN ('Q1'))` + tree, err := gosqlx.ParseWithDialect(want, keywords.DialectOracle) + if err != nil { + t.Fatal(err) + } + defer ast.ReleaseAST(tree) + out := formatter.FormatAST(tree, ast.CompactStyle()) + if out != want { + t.Errorf("formatted = %s, want %s", out, want) + } + reparsed, err := gosqlx.ParseWithDialect(out, keywords.DialectOracle) + if err != nil { + t.Fatalf("reparse: %v", err) + } + defer ast.ReleaseAST(reparsed) + if got := formatter.FormatAST(reparsed, ast.CompactStyle()); got != want { + t.Errorf("reparsed = %s, want %s", got, want) + } + if got := tree.SQL(); got != want { + t.Errorf("SQL() = %s, want %s", got, want) + } +} + +func TestPivotQuotedNameFallback(t *testing.T) { + makeSelect := func(ref ast.TableReference) *ast.SelectStatement { + return &ast.SelectStatement{Columns: []ast.Expression{&ast.Identifier{Name: "*"}}, From: []ast.TableReference{ref}} + } + cases := []struct { + name string + ref ast.TableReference + want string + }{ + {"first_alias", ast.TableReference{Name: "t", Pivot: &ast.PivotClause{AggregateFunction: &ast.Identifier{Name: "x"}, AggregateAlias: "a); DROP", PivotColumn: "q", InValues: []string{"a"}}}, `SELECT * FROM t PIVOT (x AS "a); DROP" FOR q IN (a))`}, + {"string_alias", ast.TableReference{Name: "t", Pivot: &ast.PivotClause{AggregateFunction: &ast.Identifier{Name: "x"}, AggregateAlias: "'total'", PivotColumn: "q", InValues: []string{"a"}}}, `SELECT * FROM t PIVOT (x AS "'total'" FOR q IN (a))`}, + {"extra_alias", ast.TableReference{Name: "t", Pivot: &ast.PivotClause{AggregateFunction: &ast.Identifier{Name: "x"}, ExtraAggregates: []ast.PivotAggregate{{Function: &ast.Identifier{Name: "y"}, Alias: "select"}}, PivotColumn: "q", InValues: []string{"a"}}}, `SELECT * FROM t PIVOT (x, y AS "select" FOR q IN (a))`}, + {"numeric_extra_alias", ast.TableReference{Name: "t", Pivot: &ast.PivotClause{AggregateFunction: &ast.Identifier{Name: "x"}, ExtraAggregates: []ast.PivotAggregate{{Function: &ast.Identifier{Name: "y"}, Alias: "123"}}, PivotColumn: "q", InValues: []string{"a"}}}, `SELECT * FROM t PIVOT (x, y AS "123" FOR q IN (a))`}, + {"single_value", ast.TableReference{Name: "t", Unpivot: &ast.UnpivotClause{ValueColumn: "a); DROP", NameColumn: "q", InColumns: []string{"a"}}}, `SELECT * FROM t UNPIVOT ("a); DROP" FOR q IN (a))`}, + {"string_value", ast.TableReference{Name: "t", Unpivot: &ast.UnpivotClause{ValueColumn: "'total'", NameColumn: "q", InColumns: []string{"a"}}}, `SELECT * FROM t UNPIVOT ("'total'" FOR q IN (a))`}, + {"multiple_values", ast.TableReference{Name: "t", Unpivot: &ast.UnpivotClause{ValueColumns: []string{"select", "a); DROP"}, NameColumn: "q", InColumns: []string{"(a, b)"}}}, `SELECT * FROM t UNPIVOT (("select", "a); DROP") FOR q IN ((a, b)))`}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + sel := makeSelect(tc.ref) + if got := sel.SQL(); got != tc.want { + t.Errorf("SQL() = %s, want %s", got, tc.want) + } + tree := &ast.AST{Statements: []ast.Statement{sel}} + if got := formatter.FormatAST(tree, ast.CompactStyle()); got != tc.want { + t.Errorf("formatter = %s, want %s", got, tc.want) + } + }) + } +} diff --git a/pkg/formatter/render.go b/pkg/formatter/render.go index 69abc0057..e87bff7fb 100644 --- a/pkg/formatter/render.go +++ b/pkg/formatter/render.go @@ -1462,11 +1462,11 @@ func tableRefSQL(t *ast.TableReference, f *nodeFormatter) string { if i > 0 { sb.WriteString(", ") } - sb.WriteString(ast.QuotePivotColumn(c)) + sb.WriteString(ast.QuoteUnpivotValueColumn(c)) } sb.WriteString(")") } else { - sb.WriteString(ast.QuotePivotColumn(t.Unpivot.ValueColumn)) + sb.WriteString(ast.QuoteUnpivotValueColumn(t.Unpivot.ValueColumn)) } sb.WriteString(" ") sb.WriteString(f.kw("FOR")) @@ -1490,7 +1490,8 @@ func tableRefSQL(t *ast.TableReference, f *nodeFormatter) string { } // writePivotAggregateAlias writes " AS alias" for a PIVOT aggregate alias, -// quoting the alias like a table alias; an empty alias writes nothing. +// preserving source delimiters or quoting it like a table alias; an empty +// alias writes nothing. func writePivotAggregateAlias(sb *strings.Builder, f *nodeFormatter, alias string) { if alias == "" { return @@ -1498,7 +1499,7 @@ func writePivotAggregateAlias(sb *strings.Builder, f *nodeFormatter, alias strin sb.WriteString(" ") sb.WriteString(f.kw("AS")) sb.WriteString(" ") - sb.WriteString(ast.QuoteTableAlias(alias)) + sb.WriteString(ast.QuotePivotAggregateAlias(alias)) } // writePivotItems writes PIVOT IN values or UNPIVOT IN columns separated by diff --git a/pkg/sql/ast/ast_clauses.go b/pkg/sql/ast/ast_clauses.go index a3efcbc40..242a478d3 100644 --- a/pkg/sql/ast/ast_clauses.go +++ b/pkg/sql/ast/ast_clauses.go @@ -747,7 +747,7 @@ func (c TimeTravelClause) Children() []Node { // ExtraAggregates in source order. type PivotClause struct { AggregateFunction Expression // The aggregate function, e.g. SUM(sales) - AggregateAlias string // Optional alias of AggregateFunction (BigQuery prefix / Oracle alias) + AggregateAlias string // Optional alias of AggregateFunction; quoted aliases retain source delimiters ExtraAggregates []PivotAggregate // Second and later aggregates (Oracle / BigQuery), in source order PivotColumn string // The column used for pivoting, e.g. region InValues []string // The values to pivot on, e.g. [North], 'Q1' AS q1 @@ -758,7 +758,7 @@ type PivotClause struct { // the "COUNT(*) AS n" in PIVOT (SUM(x) AS total, COUNT(*) AS n FOR ...). type PivotAggregate struct { Function Expression // The aggregate function call - Alias string // Optional alias; empty when none was given + Alias string // Optional alias; quoted aliases retain source delimiters } func (p *PivotClause) expressionNode() {} @@ -787,8 +787,8 @@ func (p PivotClause) Children() []Node { // parenthesized column set such as "(q1, q2) AS 'H1'". type UnpivotClause struct { NullHandling string // "", "INCLUDE NULLS" or "EXCLUDE NULLS" (Oracle / Snowflake / BigQuery) - ValueColumn string // The target value column, e.g. sales (single-column form) - ValueColumns []string // The target value columns (multi-column form), e.g. h1, h2 + ValueColumn string // The target value column; quoted names retain source delimiters + ValueColumns []string // The target value columns; quoted names retain source delimiters NameColumn string // The target name column, e.g. region InColumns []string // The source columns to unpivot, e.g. north_sales, south_sales Pos models.Location // Source position of the UNPIVOT keyword diff --git a/pkg/sql/ast/sql_table_hints_pivot.go b/pkg/sql/ast/sql_table_hints_pivot.go index 650a24af0..210443bca 100644 --- a/pkg/sql/ast/sql_table_hints_pivot.go +++ b/pkg/sql/ast/sql_table_hints_pivot.go @@ -91,6 +91,38 @@ func QuotePivotColumn(name string) string { return buf.String() } +// isQuotedPivotIdentifier accepts only one complete delimited identifier. +// Bare atoms, numbers and string literals must use the normal name quoting +// fallback, even though pivotAtomLen also recognizes them in IN values. +func isQuotedPivotIdentifier(name string) bool { + if len(name) < 2 { + return false + } + switch name[0] { + case '"', '`', '[': + return pivotAtomLen(name) == len(name) + } + return false +} + +// QuotePivotAggregateAlias retains source quoting for parsed aliases and +// applies normal table-alias quoting to bare or hand-built values. +func QuotePivotAggregateAlias(alias string) string { + if isQuotedPivotIdentifier(alias) { + return alias + } + return QuoteTableAlias(alias) +} + +// QuoteUnpivotValueColumn retains source quoting for parsed value columns and +// applies normal column quoting to bare or hand-built values. +func QuoteUnpivotValueColumn(name string) string { + if isQuotedPivotIdentifier(name) { + return name + } + return QuotePivotColumn(name) +} + // pivotColumnNeedsQuoting reports whether writePivotColumn would quote some // part of name. func pivotColumnNeedsQuoting(name string) bool { @@ -237,10 +269,10 @@ func pivotAtomLen(s string) int { } // writePivotClause writes " PIVOT (agg [AS alias][, ...] FOR col IN (v1, v2))". -// The pivot column is quoted when it is not a plain name, aggregate aliases -// are quoted like table aliases, and each IN value is written in its stored -// source form (e.g. [Q1], 'JAN', 1 AS one) when well formed, otherwise -// double-quoted (see QuotePivotItem). +// The pivot column is quoted when it is not a plain name. Aggregate aliases +// retain source quoting or are quoted like table aliases. Each IN value is +// written in its stored source form (e.g. [Q1], 'JAN', 1 AS one) when well +// formed, otherwise double-quoted (see QuotePivotItem). func writePivotClause(buf *bytes.Buffer, p *PivotClause) { if p == nil { return @@ -270,7 +302,7 @@ func writePivotAggregateAlias(buf *bytes.Buffer, alias string) { return } buf.WriteString(" AS ") - writeTableAlias(buf, alias) + buf.WriteString(QuotePivotAggregateAlias(alias)) } // writeUnpivotClause writes @@ -293,11 +325,11 @@ func writeUnpivotClause(buf *bytes.Buffer, u *UnpivotClause) { if i > 0 { buf.WriteString(", ") } - writePivotColumn(buf, c) + buf.WriteString(QuoteUnpivotValueColumn(c)) } buf.WriteByte(')') } else { - writePivotColumn(buf, u.ValueColumn) + buf.WriteString(QuoteUnpivotValueColumn(u.ValueColumn)) } buf.WriteString(" FOR ") writePivotColumn(buf, u.NameColumn) diff --git a/pkg/sql/parser/bigquery_pivot_test.go b/pkg/sql/parser/bigquery_pivot_test.go index 8324720ec..e00bffc19 100644 --- a/pkg/sql/parser/bigquery_pivot_test.go +++ b/pkg/sql/parser/bigquery_pivot_test.go @@ -94,6 +94,11 @@ func TestBigQueryPivot_Issue854(t *testing.T) { pivotCol: "quarter", inValues: []string{"'Q1' AS `q 1`"}, }, + { + name: "quoted aggregate aliases preserve source form", + sql: "SELECT * FROM sales PIVOT (SUM(amount) AS `日本`, COUNT(*) `a``b` FOR quarter IN ('Q1'))", + aggAlias: "`日本`", extraAggs: []string{"`a``b`"}, pivotCol: "quarter", inValues: []string{"'Q1'"}, + }, { name: "subquery source", sql: "SELECT * FROM (SELECT product, sales, quarter FROM produce) PIVOT (SUM(sales) FOR quarter IN ('Q1', 'Q2'))", @@ -187,6 +192,14 @@ func TestBigQueryUnpivot_Issue854(t *testing.T) { nameCol: "semesters", inColumns: []string{"(Q1, Q2) AS 'semester_1'", "(Q3, Q4) AS 'semester_2'"}, }, + { + name: "quoted single value column", sql: "SELECT * FROM produce UNPIVOT (`日本` FOR quarter IN (Q1, Q2))", + valueCol: "`日本`", nameCol: "quarter", inColumns: []string{"Q1", "Q2"}, + }, + { + name: "quoted multiple value columns", sql: "SELECT * FROM produce UNPIVOT ((`日本`, `a``b`) FOR quarter IN ((Q1, Q2)))", + valueCols: []string{"`日本`", "`a``b`"}, nameCol: "quarter", inColumns: []string{"(Q1, Q2)"}, + }, { name: "multi-column with NULLS modifier, unaliased sets and alias", sql: "SELECT * FROM produce UNPIVOT INCLUDE NULLS ((a, b) FOR k IN ((Q1, Q2), (Q3, Q4))) unp", diff --git a/pkg/sql/parser/pivot.go b/pkg/sql/parser/pivot.go index c197ba2a3..ed2ea28b6 100644 --- a/pkg/sql/parser/pivot.go +++ b/pkg/sql/parser/pivot.go @@ -288,12 +288,12 @@ func (p *Parser) parsePivotAggregateAlias() (string, error) { if !p.isIdentifier() && !p.isNonReservedKeyword() { return "", p.expectedError("alias after AS in PIVOT aggregate") } - alias := p.currentToken.Token.Value + alias := p.renderCurrentIdent() p.advance() return alias, nil } if p.isIdentifier() { - alias := p.currentToken.Token.Value + alias := p.renderCurrentIdent() p.advance() return alias, nil } @@ -334,7 +334,7 @@ func (p *Parser) parseUnpivotClause() (*ast.UnpivotClause, error) { var valueCol string var valueCols []string if multiColumn { - cols, err := p.parseUnpivotNameList("value column name in UNPIVOT", false) + cols, err := p.parseUnpivotNameList("value column name in UNPIVOT") if err != nil { return nil, err } @@ -343,7 +343,7 @@ func (p *Parser) parseUnpivotClause() (*ast.UnpivotClause, error) { if !p.isIdentifier() { return nil, p.expectedError("value column name in UNPIVOT") } - valueCol = p.currentToken.Token.Value + valueCol = p.renderCurrentIdent() p.advance() } @@ -382,7 +382,7 @@ func (p *Parser) parseUnpivotClause() (*ast.UnpivotClause, error) { if !p.isType(models.TokenTypeLParen) { return nil, p.expectedError("( to open a column set in UNPIVOT IN list") } - set, err := p.parseUnpivotNameList("column name in UNPIVOT IN list", true) + set, err := p.parseUnpivotNameList("column name in UNPIVOT IN list") if err != nil { return nil, err } @@ -442,23 +442,16 @@ func (p *Parser) parseUnpivotClause() (*ast.UnpivotClause, error) { } // parseUnpivotNameList parses a parenthesized, comma-separated list of -// column names, "(a, b, ...)", for the multi-column UNPIVOT form. With -// sourceForm set each name is returned as written (quoted identifiers keep -// their delimiters, as InColumns entries do); otherwise the bare name value -// is returned, as ValueColumn / ValueColumns store it. The current token -// must be the opening parenthesis. -func (p *Parser) parseUnpivotNameList(what string, sourceForm bool) ([]string, error) { +// column names, "(a, b, ...)", for the multi-column UNPIVOT form. Names +// retain their source quoting. The current token must be the opening parenthesis. +func (p *Parser) parseUnpivotNameList(what string) ([]string, error) { p.advance() // consume ( var names []string for { if !p.isIdentifier() { return nil, p.expectedError(what) } - if sourceForm { - names = append(names, p.renderCurrentIdent()) - } else { - names = append(names, p.currentToken.Token.Value) - } + names = append(names, p.renderCurrentIdent()) p.advance() if !p.isType(models.TokenTypeComma) { break diff --git a/pkg/sql/tokenizer/quoted_zero_copy_test.go b/pkg/sql/tokenizer/quoted_zero_copy_test.go index 8b46e0d2a..4fd51aefb 100644 --- a/pkg/sql/tokenizer/quoted_zero_copy_test.go +++ b/pkg/sql/tokenizer/quoted_zero_copy_test.go @@ -126,6 +126,10 @@ func quotedCases() []quotedCase { {name: "backtick clean", dialect: keywords.DialectMySQL, sql: "`col`", typ: bt, value: "col", quoteStyle: noWord, end: loc(1, 6), alias: aliasYes}, {name: "backtick empty", dialect: keywords.DialectMySQL, sql: "``", typ: bt, value: "", quoteStyle: noWord}, {name: "backtick doubled", dialect: keywords.DialectMySQL, sql: "`a``b`", typ: bt, value: "a`b", quoteStyle: noWord, alias: aliasNo}, + // parser.renderCurrentIdent uses this byte-wide span to recover backtick quoting. + {name: "backtick unicode byte span", dialect: keywords.DialectBigQuery, sql: "`日本`", typ: bt, value: "日本", quoteStyle: noWord, end: loc(1, 9), alias: aliasYes}, + {name: "backtick doubled byte span", dialect: keywords.DialectBigQuery, sql: "`a``b`", typ: bt, value: "a`b", quoteStyle: noWord, end: loc(1, 7), alias: aliasNo}, + {name: "bare unicode byte span", dialect: keywords.DialectBigQuery, sql: "日本", typ: bt, value: "日本", quoteStyle: 0, end: loc(1, 7), alias: aliasYes}, {name: "backtick multi-line", dialect: keywords.DialectMySQL, sql: "`a\nb` x", typ: bt, value: "a\nb", quoteStyle: noWord, end: loc(2, 3), alias: aliasYes}, {name: "backtick invalid UTF-8 kept raw", dialect: keywords.DialectMySQL, sql: "`a\xffb`", typ: bt, value: "a\xffb", quoteStyle: noWord, alias: aliasYes}, {name: "backtick unterminated", dialect: keywords.DialectMySQL, sql: "`abc", errCode: gerrors.ErrCodeUnterminatedString, errLoc: loc(1, 1)}, From e4aab414f1b855abe9ee01444a0860b1d8d8c656 Mon Sep 17 00:00:00 2001 From: Ajit Pratap Singh Date: Fri, 23 Oct 2026 00:24:12 +0530 Subject: [PATCH 3/3] fix(parser): gate numeric UNPIVOT aliases by dialect Keep numeric row aliases after AS in Oracle and BigQuery while rejecting them in Snowflake and SQL Server. Preserve the older nonnumeric alias handling and BigQuery bare numeric aliases. Add a four-dialect acceptance matrix and spaced backtick value/input column round trips in the AST and formatter paths. Co-Authored-By: Codex Codex-Session: 01a164e5-e59a-7281-b5b1-efc545a2f622 --- CHANGELOG.md | 2 ++ pkg/formatter/bigquery_pivot_render_test.go | 2 ++ pkg/sql/parser/bigquery_pivot_test.go | 4 ++++ pkg/sql/parser/pivot.go | 4 +++- 4 files changed, 11 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d35beba21..9ace05f73 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - PIVOT/UNPIVOT quoted aggregate aliases, UNPIVOT value columns, and backtick-quoted `IN` names and aliases keep their source delimiters when rendered. This preserves BigQuery backticks and Oracle alias case. +- Numeric UNPIVOT row aliases after `AS` are limited to BigQuery and Oracle; + Snowflake and SQL Server reject them. ### Added - `ast.PivotClause.AggregateAlias`, `ast.PivotClause.ExtraAggregates` diff --git a/pkg/formatter/bigquery_pivot_render_test.go b/pkg/formatter/bigquery_pivot_render_test.go index bb1d19fe6..e90d92230 100644 --- a/pkg/formatter/bigquery_pivot_render_test.go +++ b/pkg/formatter/bigquery_pivot_render_test.go @@ -52,6 +52,8 @@ func TestBigQueryPivot_FormatRoundTrip_Issue854(t *testing.T) { {"backtick_first_and_extra_alias", "SELECT * FROM sales PIVOT (SUM(amount) AS `日本`, COUNT(*) `a``b` FOR quarter IN ('Q1'))", "SELECT * FROM sales PIVOT (SUM(amount) AS `日本`, COUNT(*) AS `a``b` FOR quarter IN ('Q1'))"}, {"unpivot_backtick_value", "SELECT * FROM sales UNPIVOT (`日本` FOR quarter IN (Q1, Q2))", "SELECT * FROM sales UNPIVOT (`日本` FOR quarter IN (Q1, Q2))"}, {"unpivot_backtick_value_set", "SELECT * FROM sales UNPIVOT ((`日本`, `a``b`) FOR quarter IN ((Q1, Q2)))", "SELECT * FROM sales UNPIVOT ((`日本`, `a``b`) FOR quarter IN ((Q1, Q2)))"}, + {"unpivot_backtick_spaced_value", "SELECT * FROM sales UNPIVOT (`total amount` FOR quarter IN (`Q 1`, `Q 2`))", "SELECT * FROM sales UNPIVOT (`total amount` FOR quarter IN (`Q 1`, `Q 2`))"}, + {"unpivot_backtick_spaced_value_set", "SELECT * FROM sales UNPIVOT ((`a b`, c) FOR quarter IN ((`Q 1`, `Q 2`)))", "SELECT * FROM sales UNPIVOT ((`a b`, c) FOR quarter IN ((`Q 1`, `Q 2`)))"}, { "unpivot_include_nulls", "SELECT * FROM produce UNPIVOT INCLUDE NULLS (sales FOR quarter IN (Q1 'first', Q2 AS 2)) AS u", diff --git a/pkg/sql/parser/bigquery_pivot_test.go b/pkg/sql/parser/bigquery_pivot_test.go index e00bffc19..3393e9584 100644 --- a/pkg/sql/parser/bigquery_pivot_test.go +++ b/pkg/sql/parser/bigquery_pivot_test.go @@ -338,8 +338,10 @@ func TestPivot_DialectGating_Issue854(t *testing.T) { {"sqlserver aggregate alias", "SELECT * FROM t PIVOT (SUM(x) AS s FOR q IN ([a]))", keywords.DialectSQLServer}, {"sqlserver bare IN alias", "SELECT * FROM t PIVOT (SUM(x) FOR q IN ([a] b))", keywords.DialectSQLServer}, {"sqlserver INCLUDE NULLS", "SELECT * FROM t UNPIVOT INCLUDE NULLS (v FOR k IN (c1))", keywords.DialectSQLServer}, + {"sqlserver numeric row value alias", "SELECT * FROM t UNPIVOT (v FOR k IN (c1 AS 1))", keywords.DialectSQLServer}, {"snowflake multiple aggregates", "SELECT * FROM t PIVOT (SUM(x), COUNT(*) FOR q IN ('a'))", keywords.DialectSnowflake}, {"snowflake multi-column unpivot", "SELECT * FROM t UNPIVOT ((a, b) FOR k IN ((c1, c2)))", keywords.DialectSnowflake}, + {"snowflake numeric row value alias", "SELECT * FROM t UNPIVOT (v FOR k IN (c1 AS 1))", keywords.DialectSnowflake}, {"oracle bare row value alias", "SELECT * FROM t UNPIVOT (v FOR k IN (c1 'x'))", keywords.DialectOracle}, } for _, tc := range extensions { @@ -358,6 +360,8 @@ func TestPivot_DialectGating_Issue854(t *testing.T) { }{ {"oracle multiple aggregates", "SELECT * FROM t PIVOT (SUM(x) AS s, COUNT(*) AS n FOR q IN (1 AS a, 2 AS b))", keywords.DialectOracle}, {"oracle multi-column unpivot", "SELECT * FROM t UNPIVOT ((a, b) FOR k IN ((c1, c2) AS 'x'))", keywords.DialectOracle}, + {"oracle numeric row value alias", "SELECT * FROM t UNPIVOT (v FOR k IN (c1 AS 1))", keywords.DialectOracle}, + {"bigquery numeric row value alias", "SELECT * FROM t UNPIVOT (v FOR k IN (c1 AS 1))", keywords.DialectBigQuery}, {"oracle INCLUDE NULLS", "SELECT * FROM t UNPIVOT INCLUDE NULLS (v FOR k IN (c1))", keywords.DialectOracle}, {"snowflake INCLUDE NULLS", "SELECT * FROM t UNPIVOT INCLUDE NULLS (v FOR k IN (c1, c2))", keywords.DialectSnowflake}, {"snowflake bare IN alias", "SELECT * FROM t PIVOT (SUM(x) FOR q IN ('a' qa))", keywords.DialectSnowflake}, diff --git a/pkg/sql/parser/pivot.go b/pkg/sql/parser/pivot.go index ed2ea28b6..02df029a9 100644 --- a/pkg/sql/parser/pivot.go +++ b/pkg/sql/parser/pivot.go @@ -400,8 +400,10 @@ func (p *Parser) parseUnpivotClause() (*ast.UnpivotClause, error) { } if p.isType(models.TokenTypeAs) { p.advance() // consume AS + // Oracle accepts a literal alias; BigQuery also permits INT64 aliases. + numericAlias := p.isType(models.TokenTypeNumber) && (p.IsOracle() || p.IsBigQuery()) if !p.isStringLiteral() && !p.isIdentifier() && !p.isNonReservedKeyword() && - !p.isType(models.TokenTypeNumber) { + !numericAlias { return nil, p.expectedError("alias after AS in UNPIVOT IN list") } col += " AS " + p.renderPivotValue()