Skip to content

[BUG] SQL() and formatter drop numeric window-frame bounds (ROWS BETWEEN 1 PRECEDING ... renders as ROWS BETWEEN PRECEDING ...) #589

Closed

Description

@ajitpratap0

Summary

ast.AST.SQL() and pkg/formatter drop the numeric offset of a window-frame bound. ROWS BETWEEN 1 PRECEDING AND CURRENT ROW is parsed correctly but rendered as ROWS BETWEEN PRECEDING AND CURRENT ROW, which is invalid SQL and changes the query's meaning if a consumer re-parses it after a round trip. Reproduced on main at 142bff2.

Reproduction

tree, _ := gosqlx.Parse("SELECT SUM(x) OVER (ORDER BY y ROWS BETWEEN 1 PRECEDING AND CURRENT ROW) FROM t")
fmt.Println(tree.SQL())
// SELECT SUM(x) OVER (ORDER BY y ROWS BETWEEN PRECEDING AND CURRENT ROW) FROM t
out, _ := formatter.FormatString("SELECT SUM(x) OVER (ORDER BY y ROWS BETWEEN UNBOUNDED PRECEDING AND 2 FOLLOWING) FROM t")
// SELECT SUM(x) OVER (ORDER BY y ROWS BETWEEN UNBOUNDED PRECEDING AND FOLLOWING)
//   FROM t;

Also affected: the single-bound form ROWS 3 PRECEDING renders as ROWS PRECEDING.

Unrelated but noticed in the same probe: RANGE BETWEEN INTERVAL '1' DAY PRECEDING AND CURRENT ROW does not parse (E2002 "expected PRECEDING or FOLLOWING after numeric value, got IDENTIFIER"); the frame-bound grammar only accepts a bare numeric literal. That may deserve its own issue.

Where

AST node: pkg/sql/ast/ast_clauses.go WindowFrameBound{Type string; Value Expression}. The parser fills Value for <n> PRECEDING/FOLLOWING:

pkg/sql/parser/window.go:514:	bound := &ast.WindowFrameBound{}

Rendering sites that write Type but ignore Value:

pkg/formatter/render.go:1382:		return fmt.Sprintf("%s BETWEEN %s AND %s", f.Type, f.Start.Type, f.End.Type)
pkg/formatter/render.go:1384:	return fmt.Sprintf("%s %s", f.Type, f.Start.Type)
pkg/sql/ast/sql.go:2009:		writeWindowFrame(buf, w.FrameClause)
pkg/sql/ast/sql.go:2013:func writeWindowFrame(buf *bytes.Buffer, f *WindowFrame) {
pkg/sql/ast/sql.go:2017:		buf.WriteString(f.Start.Type)
pkg/sql/ast/sql.go:2023:	buf.WriteString(f.Start.Type)

(WindowFrameBound.TokenLiteral() also returns only Type.)

Proposed fix

  • In ast/sql.go's window-frame writer and in formatter/render.go, when bound.Value != nil write the value first: <value> PRECEDING / <value> FOLLOWING; keep CURRENT ROW and UNBOUNDED ... as today.
  • Make WindowFrameBound.TokenLiteral() include the value as well, since the CLI formatter falls back to it.
  • Tests: table-driven round-trip for ROWS/RANGE/GROUPS with BETWEEN n PRECEDING AND m FOLLOWING, n PRECEDING alone, UNBOUNDED, CURRENT ROW, and EXCLUDE variants if supported; assert SQL() output re-parses to a deep-equal AST (idempotent). Add these cases to pkg/sql/ast/roundtrip_test.go and the formatter golden tests.

Related

Found by the #557 implementer while writing Walk/Children tests (PR #588). Same class of silent data-loss as #531 and the dropped INSERT CTE fixed in #584.

Found by an automated implementation pass on 2026-10-12.

Metadata

Metadata

Assignees

No one assigned

    Labels

    astAST node changesbugSomething isn't workingformatter

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions