[ajitpratap0]

Summary

#608 ast/formatter: named windows render as OVER (w) ... WINDOW w AS (w ...) and do not round-trip; golden file pins the broken output

Fixes #608. WindowSpec.Name was doing two jobs: the WINDOW definition name and the bare OVER w reference. Both renderers wrote it inside the parentheses, so OVER w ... WINDOW w AS (PARTITION BY y) came out as OVER (w) ... WINDOW w AS (w PARTITION BY y), which fails to re-parse with E2002. The SQL:2003 refinement OVER (w ORDER BY z) did not parse in any dialect.

The change adds one field, WindowSpec.BaseWindow, for the existing window that a parenthesized spec refines. The pool release path resets it. The parser fills BaseWindow, and ast.SQL() and pkg/formatter now render OVER w, OVER (w ...) and WINDOW w AS (<base> ...) correctly. The non-standard WINDOW w AS w2 is normalized to WINDOW w AS (w2).

Verification:

• New regression tests fail on the parent and pass on HEAD.
• An independent sweep over 12 dialects confirmed byte-for-byte round-trips through ast.SQL(), formatter.FormatAST and gosqlx.Format.
• Output for inputs without named windows is identical on the parent and on HEAD.
• The full CGO race gate, go vet and golangci-lint are clean.

Deferred:

• The CLI renderer (gosqlx format, cmd/gosqlx/cmd/sql_formatter.go) still drops WINDOW and base-window names. That is cli: gosqlx format silently corrupts many expressions; switch the CLI to pkg/formatter #599, which replaces that renderer.
• Quoting of window names that need quotes is a possible follow-up.
• GROUPS/EXCLUDE frames are parser: window frames reject GROUPS, EXCLUDE and INTERVAL '1' DAY bounds; ast doc example uses Type:"2 PRECEDING" #609.

#609 parser: window frames reject GROUPS, EXCLUDE and INTERVAL '1' DAY bounds; ast doc example uses Type:"2 PRECEDING"

This change makes window frames accept the GROUPS unit and the SQL:2011 EXCLUDE clause (CURRENT ROW, GROUP, TIES, NO OTHERS); an invalid continuation is a positioned E2002 parse error. INTERVAL literals now take an ANSI unit qualifier (INTERVAL '1' DAY, INTERVAL '1-2' YEAR TO MONTH, MySQL units such as YEAR_MONTH), stored in a new additive field IntervalExpression.Qualifier. This fixes the RANGE BETWEEN INTERVAL '1' DAY PRECEDING failure and the old d + INTERVAL '1' AS DAY mis-render. ast.SQL(), pkg/formatter and the CLI formatter all emit the new WindowFrame.Exclude field. The FunctionCall doc example now uses the parser's bound shape and is pinned by a golden test.

Validation:

• All issue queries round-trip byte for byte through SQL(), re-parse and FormatAST(Compact) in all 8 dialects.
• The new tests fail on the parent commit.
• Behaviour outside the issue is unchanged: explicit or quoted aliases, plural words, MySQL INTERVAL 30 DAY, existing ROWS/RANGE frames and ast/formatter: named windows render as OVER (w) ... WINDOW w AS (w ...) and do not round-trip; golden file pins the broken output #608 named windows give the same output as before.
• The race suite, go vet and golangci-lint are clean.

Deferred, out of scope:

• MySQL INTERVAL 1 DAY renders as INTERVAL '1 DAY'. Pre-existing; recommend a follow-up issue.
• CLI gosqlx format drops INTERVAL values. Tracked in cli: gosqlx format silently corrupts many expressions; switch the CLI to pkg/formatter #599.
• An arithmetic right-hand side of a comparison fails to parse. Tracked in parser: arithmetic on the right-hand side of a comparison fails (WHERE a = 1 + 2) #596.
• Interval field precision and non-literal MySQL interval values.
• Frame-unit casing is preserved as written.
• GROUPS/EXCLUDE are not gated by dialect; MySQL, SQL Server, Snowflake and ClickHouse accept them although those engines do not.

Commits (2), one implementation commit per issue plus test-only follow-ups:

• f71328c fix(parser,ast,formatter): accept GROUPS frames, EXCLUDE clause and INTERVAL '' qualifiers (parser: window frames reject GROUPS, EXCLUDE and INTERVAL '1' DAY bounds; ast doc example uses Type:"2 PRECEDING" #609)
• bbe520e fix(parser,ast,formatter): render named windows as OVER w and accept OVER (w ...) refinement (ast/formatter: named windows render as OVER (w) ... WINDOW w AS (w ...) and do not round-trip; golden file pins the broken output #608)

18 files changed, 1059 insertions(+), 37 deletions(-)

Test Plan

• Each issue's reproduction cases were run against main before the fix (Haiku scout), and a failing regression test was written before the code change.
• Independent review (Opus, or Sonnet on small chains) with a bounded fix loop.
• Independent Opus validator ran the repository race gate (CGO_ENABLED=1 go test -race -timeout 300s $(go list ./... | grep -v /cbinding)), go vet, golangci-lint, re-ran the reproduction cases on HEAD, and confirmed the new tests fail on the parent commit.
• Perf items: benchstat -count=6 before/after on the same machine.

#608 evidence (validator)

Not a perf item, so no benchstat. Repro and round-trip evidence:

(1) Parent 61ca6c8 with the new pkg/gosqlx/issue_608_test.go copied in. Both TestIssue608_NamedWindowRoundTrip and TestIssue608_FormatBareNamedWindow FAIL in the default, postgresql and mysql dialects:
- Wrong render: got `SELECT sum(x) OVER (w) FROM t WINDOW w AS (w PARTITION BY y)`.
- Refinement does not parse: `OVER (w ORDER BY z)` and `OVER (w)` give E2002 at 1:21; the window chain gives E2002 at 1:84.
- `WINDOW w AS w2` is rendered as `WINDOW w AS (w)`, so w2 is lost.
- On the parent, the parser, formatter and golden test files do not compile, because they reference the new BaseWindow field.

(2) On HEAD, an independent sweep program (a separate module with a replace directive, so the worktree was not touched) ran across 12 dialects: default, generic, postgresql, mysql, mariadb, sqlite, sqlserver, oracle, snowflake, clickhouse, bigquery and redshift. For each one it ran ast.SQL(), a re-parse and stability check, formatter.FormatAST(CompactStyle) and its re-parse, and gosqlx.Format. Every scout case now renders the expected text and round-trips:
- `SELECT sum(x) OVER w FROM t WINDOW w AS (PARTITION BY y)`
- `OVER (w ORDER BY z)`, which yields Over.BaseWindow="w" and Name=""
- `OVER (w)`, which stays parenthesized
- the frame definition case
- `WINDOW w1 AS (...), w2 AS (w1 ORDER BY b)`
- `WINDOW w AS w2`, which renders as `WINDOW w AS (w2)` with Name=w and Base=w2. This is the only output that differs from its input, and it is the intended normalization.

(3) Inputs without named windows produce byte-identical output on the parent and on HEAD across all 12 dialects (diff of 85 lines is empty). The inputs were `OVER ()`, `OVER (PARTITION BY a ORDER BY b)`, ROWS BETWEEN, ROWS UNBOUNDED, RANGE BETWEEN, and DESC NULLS LAST.

#608 race gate: PASS. Ran CGO_ENABLED=1 go test -race -timeout 300s $(go list ./... | grep -v /cbinding) in the worktree at HEAD bbe520e: exit 0, 44 packages ok, 0 FAIL, 0 DATA RACE. Log: /private/tmp/claude-501/-Users-ajitpratapsingh-dev-GoSQLX/b99cd3cf-cb92-4294-a0db-d6a7c9a86253/scratchpad/b3/608/val/race.txt. No flakes and no re-runs needed.
#609 evidence (validator)

Not a perf item, so no benchstat. Repro and round-trip evidence below.

**Probe setup.** I wrote a scratch probe module (scratchpad/b3/609/val/probe) that runs SQL() → re-parse plus formatter.FormatAST(CompactStyle). It ran in 9 dialects (default, postgresql, mysql, sqlite, sqlserver, oracle, snowflake, clickhouse, mariadb), against both HEAD and a git-archive of the parent bbe520e. I then diffed the two outputs.

**Fixed (byte-for-byte round-trip in every dialect, SQL() and compact formatter):**
- `... OVER (ORDER BY y GROUPS BETWEEN 1 PRECEDING AND CURRENT ROW) FROM t`. Parent: E2002 at 1:32, "expected ), got GROUPS".
- EXCLUDE CURRENT ROW, TIES, GROUP and NO OTHERS. Parent: E2002 at 1:73.
- `RANGE BETWEEN INTERVAL '1' DAY PRECEDING ...`. Parent: E2002 at 1:59.
- `d + INTERVAL '1' DAY`. Parent rendered `INTERVAL '1' AS DAY`; it now renders unchanged. The same holds for HOUR, MINUTE, SECOND, MONTH, YEAR, WEEK and QUARTER, plus `'1-2' YEAR TO MONTH` and `'1 02:00' DAY TO MINUTE`, which previously failed with "got TO".
- Also fixed: the single-bound form `ROWS 2 PRECEDING EXCLUDE TIES`, `OVER (w ROWS ... EXCLUDE TIES)`, `WHERE d > INTERVAL '1' DAY AND x = 1`, and `BETWEEN INTERVAL '1' DAY AND INTERVAL '2' DAY`.
- gosqlx.Format now gives `SELECT d + INTERVAL '1' DAY\nFROM t` and emits EXCLUDE TIES.
- `INTERVAL '1-2' YEAR TO MONTH` parses to `&ast.IntervalExpression{Value:"1-2", Qualifier:"YEAR TO MONTH"}`.

**Unchanged from the parent (output identical):**
- `INTERVAL '1 day' AS day`
- `INTERVAL '1 day' "day"`
- `INTERVAL '1' days` and `INTERVAL '1' foo`, which stay aliases
- MySQL `INTERVAL 30 DAY` → `{Value:"30 DAY", Qualifier:""}`, SQL `INTERVAL '30 DAY'`
- `INTERVAL 1 DAY` frame offset → `'1 DAY'`
- the existing ROWS frame
- the #608 `OVER w` / `WINDOW` output
- `?` in the default dialect is still E2001; `$1` in postgresql is still valid

**Invalid EXCLUDE is a clean error, with no panic or dropped tokens:**
- EXCLUDE FOO, EXCLUDE NO, EXCLUDE CURRENT, a bare EXCLU

#609 race gate: PASS. Ran CGO_ENABLED=1 go test -race -timeout 300s $(go list ./... | grep -v /cbinding) in the worktree at HEAD f71328c. Exit code 0: 44 packages ok (16 from the test cache, which is valid for identical code), no FAIL, no DATA RACE. No flakes, so no re-run was needed.

Breaking Changes

• ast/formatter: named windows render as OVER (w) ... WINDOW w AS (w ...) and do not round-trip; golden file pins the broken output #608: Additive only: a new exported field BaseWindow string on ast.WindowSpec, documented as the existing window name refined inside the parentheses. No signature changes; writeWindowSpec is unexported. Observable behaviour changes:
• OVER w now renders as OVER w; it used to render as OVER (w).
• WINDOW definitions no longer repeat their name inside AS (...).
• OVER (w ...) and OVER (w) now parse, with BaseWindow set.
• WINDOW w AS w2 now keeps w2, rendering as AS (w2).

All of these turn previously invalid or non-round-tripping output into valid output. Hand-built OVER specs with Name plus parts render as before.

• parser: window frames reject GROUPS, EXCLUDE and INTERVAL '1' DAY bounds; ast doc example uses Type:"2 PRECEDING" #609: Additive only. Two new exported string fields: ast.WindowFrame.Exclude ("CURRENT ROW" | "GROUP" | "TIES" | "NO OTHERS" | "") and ast.IntervalExpression.Qualifier (for example "DAY" or "YEAR TO MONTH"; "" when absent). WindowFrame.Type may now also be "GROUPS". Observable behaviour changes: GROUPS frames, the EXCLUDE clause and ANSI-qualified INTERVAL literals now parse. SQL(), pkg/formatter and the CLI render them. INTERVAL '<v>' <unit> is no longer parsed as INTERVAL '' plus a column alias named after the unit. No signatures change. The Value of existing interval forms is unchanged (the MySQL numeric form still stores "30 DAY").

Notes

• CHANGELOG.md is intentionally not touched; the consolidated CHANGELOG PR docs: consolidate CHANGELOG for the perf audit and batch-2/batch-3 fixes; refresh performance baselines (#566) #598 gets the entries for this batch before it merges.
• Part of the batch-3 merge train (follow-ups cli: gosqlx format silently corrupts many expressions; switch the CLI to pkg/formatter #599-schema/postgres: loader misreports index columns and key flags (column order, INCLUDE, expression and partitioned indexes, read-only roles) #620); later PRs in the train are rebased onto this one after it merges.
• Base: main.

Fixes #608
Fixes #609
Follow-up to #566

🤖 Generated with Claude Code

https://claude.ai/code/session_01UZSk1292jPc8ipDboNxeve

[vercel]

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

Project	Deployment	Actions	Updated
gosqlx-website	Ready	Preview	Oct 14, 2026 6:28am UTC

[github-actions]

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

---

Review of PR #629 (#608 named windows, #609 GROUPS/EXCLUDE/INTERVAL)

I read the diff for the parser, AST, formatter and CLI changes. I did not run the tests or the race gate here. I'm relying on the validator evidence in the PR description for that.

Overall: The change is small, additive and well scoped. The BaseWindow, Exclude and Qualifier fields are the right model. Keeping Name as the definition or reference name and BaseWindow as the refined window removes the ambiguity behind #608.

What looks good

• writeWindowSpec(buf, base, w) takes the base name as an explicit argument. That makes it hard to write Name inside the parentheses again by accident.
• The fallback for hand-built specs that carry Name plus other parts (pkg/sql/ast/sql.go, the FunctionCall OVER branch) keeps the old output for existing callers.
• parseFrameExclusion returns a positioned E2002 on an invalid continuation. It never silently drops tokens.
• parseIntervalQualifier is conservative:
  • It only accepts unquoted identifiers.
  • It requires a known leading field.
  • It only consumes TO when the end field is valid.
  • That keeps INTERVAL '1' days and "day" as aliases.
• The pool release resets BaseWindow, which avoids stale state from pooled objects.
• The regression tests cover multiple dialects and round-trips, and the CLI formatter test covers EXCLUDE.

Observations and minor suggestions

1. Unquoted alias named like a unit (behavior change). SELECT INTERVAL '1' day FROM t used to be a value with the alias day, and it is now INTERVAL '1' DAY. This is the intended fix. It is still a silent change for anyone who aliased an interval with an unquoted unit word such as month, year or second. A short note in the CHANGELOG entry (docs: consolidate CHANGELOG for the perf audit and batch-2/batch-3 fixes; refresh performance baselines (#566) #598) would help.
2. Base-window detection (window.go, start of parseWindowSpec). p.isIdentifier() accepts any identifier-class token as the base window. This relies on PARTITION, ORDER, ROWS, RANGE and GROUPS all having dedicated token types. That holds for GROUPS here. I'd add a test showing that OVER (groups ...) or a window actually named groups doesn't mis-parse. I'd also test the OVER (w ROWS ...) case in dialects where ROWS might lex differently, such as ClickHouse and Snowflake.
3. WINDOW w AS w2 handling (select.go). The code copies winSpec.Name into BaseWindow and then overwrites Name. It works, but it depends on parseWindowSpec having put the bare name in Name. A one-line comment on that coupling is already there, which is good. A unit assertion on the AST shape (Name=w, Base=w2) is also in the PR, so this is fine.
4. Interval Value and Qualifier for MySQL. INTERVAL 30 DAY keeps "30 DAY" in Value with an empty Qualifier, while the ANSI form splits them. The PR already calls this out as deferred. A TODO in code that points to the follow-up issue would stop it from being forgotten.
5. Exclude and frame type casing. Type keeps the original casing (groups vs GROUPS), while Exclude is normalized to upper case. A consumer comparing Type == "GROUPS" will miss lowercase input. Consider normalizing Type in parseWindowSpec for consistency. The PR lists this as a known deferral.
6. Dialect gating. GROUPS and EXCLUDE are accepted in MySQL, SQL Server, Snowflake and ClickHouse, which reject them. That is acceptable for a permissive parser. docs/SQL_COMPATIBILITY.md should state it explicitly, if it doesn't already.
7. Unquoted window names. OVER w and WINDOW w AS (...) write names verbatim. A name that needed quoting would round-trip wrongly. This is an acknowledged follow-up, and I agree it can wait.

Performance and security

• No new allocations on hot paths beyond one string per qualifier or exclude. The qualifier uses strings.ToUpper on short tokens, which is fine.
• The EXCLUDE and qualifier parsers consume a bounded number of tokens, so they add no recursion or input-driven blowup. That fits the bounds work from fix(tokenizer,parser,formatter): bound input-driven recursion, comment counts and set-operation rendering #597.
• I found no injection or security concerns. All rendering goes through existing writers, and the values come from the parser.

Test coverage

Coverage is good: tests fail on the parent, round-trips run across dialects, and the golden doc example is pinned. Two small gaps are the casing and window-name edge cases from item 2 and item 5, and negative tests for EXCLUDE at end of input (for example ... ROWS 1 PRECEDING EXCLUDE). The PR mentions the latter was validated manually, so a permanent test would be cheap.

I found no blocking issues, so this looks good to merge once CI is green.
· Branch: fix/window-named-and-frames

[ajitpratap0]

Merge train (batch 3): rebased onto main (f5433f6).

• Adversarial pass (Opus): found one must-fix, fixed in ed1dada and re-verified. The legacy CLI renderer dropped the new BaseWindow, so OVER (w ORDER BY z) printed OVER (ORDER BY z). gosqlx format now prints the base window, and OVER w prints as bare OVER w.
• Bot review item 2: checked on HEAD. OVER (w ROWS 1 PRECEDING) ... WINDOW w AS (ORDER BY y) round-trips in clickhouse, snowflake, postgresql and mysql. A dangling EXCLUDE gives E2002 'expected CURRENT ROW, GROUP, TIES or NO OTHERS after EXCLUDE'.
• Deferred: frame-type casing, the MySQL INTERVAL split, quoting of window names, and dialect gating. They are listed for follow-up issues and the docs: consolidate CHANGELOG for the perf audit and batch-2/batch-3 fixes; refresh performance baselines (#566) #598 CHANGELOG.

[github-actions]

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

---

Review of PR #629 (#608 named windows, #609 GROUPS/EXCLUDE/INTERVAL)

I read the parser, AST and SQL-rendering diffs. I did not run tests or the race gate, so I'm relying on the validator evidence in the PR description for that.

Overall: No blocking issues. The change is additive and well scoped. The BaseWindow, Exclude and Qualifier fields model the grammar correctly.

What looks good

• writeWindowSpec(buf, base, w) takes the base name as an argument, so Name can't be written inside the parentheses by accident.
• The fallback for hand-built specs that carry Name plus other parts keeps their old output.
• parseFrameExclusion returns a positioned E2002 on a bad continuation. It never drops tokens silently.
• parseIntervalQualifier is conservative. It accepts only unquoted identifiers with a known leading field, and it consumes TO only when the end field is valid. Quoted "day" and plural days stay aliases.
• The pool release path resets BaseWindow.

Suggestions (non-blocking)

1. Unquoted alias named like a unit. SELECT INTERVAL '1' day FROM t used to alias the interval as day. It now parses as INTERVAL '1' DAY. This is the intended fix, but it silently changes behavior for aliases such as month, year or second. Please mention it in the CHANGELOG entry on docs: consolidate CHANGELOG for the perf audit and batch-2/batch-3 fixes; refresh performance baselines (#566) #598.
2. Base-window detection (window.go, start of parseWindowSpec). p.isIdentifier() takes any identifier-class token as the base window. The comment says PARTITION, ORDER, ROWS, RANGE and GROUPS all have dedicated token types, so they are safe. Please add a test for OVER (w ROWS ...) and OVER (w GROUPS ...) in ClickHouse, Snowflake and MySQL. Also test a window literally named groups or rows, if those lex as non-identifiers. That would pin the assumption.
3. Frame Type casing. Type keeps the casing as written, but Exclude is upper-cased. A consumer comparing Type == "GROUPS" misses lowercase input. Normalizing in parseWindowSpec would be cheap. If you defer it, a doc comment on WindowFrame.Type should say it is not normalized.
4. Negative test for EXCLUDE at end of input. The cases ... ROWS 1 PRECEDING EXCLUDE and EXCLUDE NO (EOF) were validated manually. Please add them as permanent tests.
5. MySQL INTERVAL 30 DAY keeps "30 DAY" in Value with an empty Qualifier, while the ANSI form splits them. A TODO(#issue) next to the MySQL branch in parseIntervalExpression would keep the follow-up visible.
6. WINDOW w AS w2 (select.go). The code copies Name into BaseWindow and then overwrites Name. It works, but it depends on parseWindowSpec having put the bare name into Name. The existing comment covers this, and the AST-shape assertion in the tests guards it.
7. Unquoted window names. OVER w and WINDOW w AS (...) write names verbatim, so a name that needs quoting would not round-trip. This is already listed as a follow-up and can wait.

Performance and security

• The only added allocation is one string per qualifier or exclusion.
• EXCLUDE and qualifier parsing consume a bounded number of tokens, so there is no recursion or input-driven blowup.
• I found no injection or security concerns. Rendering goes through the existing writers.

Tests

Coverage is good. Tests fail on the parent, round-trips run across dialects, and the doc example is pinned by a golden test. The gaps are items 2–4 above.

Looks good to merge once CI is green.
· Branch: fix/window-named-and-frames