Skip to content

Improved window function support #2913

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
fulghum wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Improved window function support #2913

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7b24fb9
First pass on sum/avg window function implementations
fulghum Jul 9, 2026
070f57e
upgrade go-mysql-server to dev version at 1e74a51e8ddaaf8583699e8275f…
fulghum Jul 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ require (
github.com/dolthub/dolt/go v0.40.5-0.20260710173237-7bdcf6ee8148
github.com/dolthub/eventsapi_schema v0.0.0-20260310172945-37a9265ade69
github.com/dolthub/flatbuffers/v23 v23.3.3-dh.2

@itoqa itoqa Bot Jul 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

View All Evidence

High severity Named window inheritance drops ORDER BY semantics

What failed: Named-window reference resolution loses ORDER BY/frame behavior for aggregate windows, so inherited and override forms behave like full-partition windows instead of ordered running windows.

Impact · Steps · Stub / mock · Analysis · Why this is likely a bug
  • Severity: High High severity
  • Impact: Queries that use named window references can produce silently incorrect rankings and aggregates because ORDER BY is dropped during resolution, causing users to make decisions from wrong analytical results.
  • Steps to Reproduce:
    1. Create t_inherit(id, grp, amt) and insert rows (1,1,10),(2,1,20),(3,1,30),(4,2,5),(5,2,15).
    2. Run SELECT id, SUM(amt) OVER w2 AS s FROM t_inherit WINDOW w1 AS (PARTITION BY grp), w2 AS (w1 ORDER BY id) ORDER BY id; and observe full-partition sums (60,60,60,20,20).
    3. Run SELECT id, SUM(amt) OVER (PARTITION BY grp ORDER BY id) AS s FROM t_inherit ORDER BY id; and observe correct running sums (10,30,60,5,20).
  • Stub / mock content: No stubs, mocks, or bypasses were applied for this test in the recorded run.
  • Code Analysis: The PR updates the engine dependency in go.mod (line 12) to github.com/dolthub/go-mysql-server v0.20.1-0.20260713191803-1e74a51e8dda. Runtime evidence isolates the defect to named-window resolution: inline OVER (PARTITION BY grp ORDER BY id) is correct, but OVER w2 and OVER (w1 ORDER BY id) both collapse to partition totals. The parser/AST bridge in this repo maps named references through RefName (postgres/parser/parser/sql.y and server/ast/window.go), so resolution reaches the planner; the incorrect aggregate behavior is consistent with the changed dependency's named-window merge path keeping an unbounded frame for these reference forms. Smallest practical fix: patch or pin the dependency so named-window merge preserves ORDER BY/frame semantics for inherited/override references, or add a guard in the planning path to avoid promoting these references to an unbounded full-partition frame.
  • Why this is likely a bug: Equivalent inline and named window forms should be semantically consistent for SUM OVER with the same PARTITION/ORDER specification, but only named-reference forms return incorrect full-partition totals. This is deterministic on the same dataset and points to a planner merge defect rather than test setup noise.
Relevant code

go.mod:11-13

github.com/dolthub/flatbuffers/v23 v23.3.3-dh.2
github.com/dolthub/go-mysql-server v0.20.1-0.20260713191803-1e74a51e8dda
github.com/dolthub/pg_query_go/v6 v6.0.0-20251215122834-fb20be4254d1

postgres/parser/parser/sql.y:13631-13634

| OVER window_name
  {
    $$.val = &tree.WindowDef{RefName: tree.Name($2)}
  }

server/ast/window.go:104-107

return &vitess.WindowDef{
	Name:        vitess.NewColIdent(string(node.Name)),
	NameRef:     vitess.NewColIdent(string(node.RefName)),
	PartitionBy: partitionBy,
Evidence Package
Copy prompt for an agent 
Ito QA identified the following failure during automated PR testing. Please investigate and propose a fix.

**High severity — Named window inheritance drops ORDER BY semantics**

**What failed:** Named-window reference resolution loses ORDER BY/frame behavior for aggregate windows, so inherited and override forms behave like full-partition windows instead of ordered running windows.

- **Impact:** Queries that use named window references can produce silently incorrect rankings and aggregates because ORDER BY is dropped during resolution, causing users to make decisions from wrong analytical results.
- **Steps to reproduce:**
  1. Create `t_inherit(id, grp, amt)` and insert rows `(1,1,10),(2,1,20),(3,1,30),(4,2,5),(5,2,15)`.
  2. Run `SELECT id, SUM(amt) OVER w2 AS s FROM t_inherit WINDOW w1 AS (PARTITION BY grp), w2 AS (w1 ORDER BY id) ORDER BY id;` and observe full-partition sums (`60,60,60,20,20`).
  3. Run `SELECT id, SUM(amt) OVER (PARTITION BY grp ORDER BY id) AS s FROM t_inherit ORDER BY id;` and observe correct running sums (`10,30,60,5,20`).
- **Stub / mock content:** No stubs, mocks, or bypasses were applied for this test in the recorded run.
- **Code analysis:** The PR updates the engine dependency in `go.mod` (line 12) to `github.com/dolthub/go-mysql-server v0.20.1-0.20260713191803-1e74a51e8dda`. Runtime evidence isolates the defect to named-window resolution: inline `OVER (PARTITION BY grp ORDER BY id)` is correct, but `OVER w2` and `OVER (w1 ORDER BY id)` both collapse to partition totals. The parser/AST bridge in this repo maps named references through `RefName` (`postgres/parser/parser/sql.y` and `server/ast/window.go`), so resolution reaches the planner; the incorrect aggregate behavior is consistent with the changed dependency's named-window merge path keeping an unbounded frame for these reference forms. Smallest practical fix: patch or pin the dependency so named-window merge preserves ORDER BY/frame semantics for inherited/override references, or add a guard in the planning path to avoid promoting these references to an unbounded full-partition frame.
- **Why this is likely a bug:** Equivalent inline and named window forms should be semantically consistent for SUM OVER with the same PARTITION/ORDER specification, but only named-reference forms return incorrect full-partition totals. This is deterministic on the same dataset and points to a planner merge defect rather than test setup noise.

**Relevant code:**

`go.mod:11-13`

~~~gomod
github.com/dolthub/flatbuffers/v23 v23.3.3-dh.2
github.com/dolthub/go-mysql-server v0.20.1-0.20260713191803-1e74a51e8dda
github.com/dolthub/pg_query_go/v6 v6.0.0-20251215122834-fb20be4254d1
~~~

`postgres/parser/parser/sql.y:13631-13634`

~~~yacc
| OVER window_name
  {
    $$.val = &tree.WindowDef{RefName: tree.Name($2)}
  }
~~~

`server/ast/window.go:104-107`

~~~go
return &vitess.WindowDef{
	Name:        vitess.NewColIdent(string(node.Name)),
	NameRef:     vitess.NewColIdent(string(node.RefName)),
	PartitionBy: partitionBy,
~~~

github.com/dolthub/go-mysql-server v0.20.1-0.20260709233935-b8ef331dca43
github.com/dolthub/go-mysql-server v0.20.1-0.20260713191803-1e74a51e8dda
github.com/dolthub/pg_query_go/v6 v6.0.0-20251215122834-fb20be4254d1
github.com/dolthub/sqllogictest/go v0.0.0-20260624223518-788480b24166
github.com/dolthub/vitess v0.0.0-20260624214226-81d034e0fde8
Expand Down
2 changes: 2 additions & 0 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,8 @@ github.com/dolthub/go-icu-regex v0.0.0-20260610153742-72563bc7ca83 h1:FEMjCGEroD
github.com/dolthub/go-icu-regex v0.0.0-20260610153742-72563bc7ca83/go.mod h1:F3cnm+vMRK1HaU6+rNqQrOCyR03HHhR1GWG2gnPOqaE=
github.com/dolthub/go-mysql-server v0.20.1-0.20260709233935-b8ef331dca43 h1:BrDle6fyYfw4wxfzD4mD0R+PXT80e7Vl9UHWtKA58qw=
github.com/dolthub/go-mysql-server v0.20.1-0.20260709233935-b8ef331dca43/go.mod h1:mj5/QX3V8i92REbA1w6CzyknJAFdKtdE7l931405C/E=
github.com/dolthub/go-mysql-server v0.20.1-0.20260713191803-1e74a51e8dda h1:+s4sow/n3tOECc3VgTi2QGH7GWMumQgJbFtdXzDJxi4=
github.com/dolthub/go-mysql-server v0.20.1-0.20260713191803-1e74a51e8dda/go.mod h1:mj5/QX3V8i92REbA1w6CzyknJAFdKtdE7l931405C/E=
github.com/dolthub/gozstd v0.0.0-20240423170813-23a2903bca63 h1:OAsXLAPL4du6tfbBgK0xXHZkOlos63RdKYS3Sgw/dfI=
github.com/dolthub/gozstd v0.0.0-20240423170813-23a2903bca63/go.mod h1:lV7lUeuDhH5thVGDCKXbatwKy2KW80L4rMT46n+Y2/Q=
github.com/dolthub/ishell v0.0.0-20260414231531-5f031e3e9037 h1:oIW9HwuWrhxv+4HZxA+QQSKHLqWFyXZ2FmNjUYwkdiM=
Expand Down
2 changes: 1 addition & 1 deletion postgres/parser/parser/sql.y
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13630,7 +13630,7 @@ over_clause:
}
| OVER window_name

@itoqa itoqa Bot Jul 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

View All Evidence

High severity Named window reference via OVER w resolves correctly

What failed: Named window reference OVER w produces a full-partition SUM per row instead of a running SUM within each partition, even though inline window syntax behaves correctly.

Impact · Steps · Stub / mock · Analysis · Why this is likely a bug
  • Severity: High High severity
  • Impact: Queries that rely on named window ORDER BY semantics can return silently incorrect running totals, which can mislead users making decisions from query results.
  • Steps to Reproduce:
    1. Create table t_named(id int, grp int, amt int) and insert (1,1,10),(2,1,20),(3,2,5).
    2. Run SELECT id, SUM(amt) OVER w AS s FROM t_named WINDOW w AS (PARTITION BY grp ORDER BY id) ORDER BY id;.
    3. Observe id=1 returns 30 instead of 10; compare with inline OVER (PARTITION BY grp ORDER BY id) which returns 10,30,5.
  • Stub / mock content: No stubs, mocks, or bypasses were applied for this test in the recorded run.
  • Code Analysis: postgres/parser/parser/sql.y now emits RefName for OVER window_name, so named windows flow through reference merge behavior. The PR also updates go-mysql-server in go.mod, and that dependency path applies a default unbounded frame too early in buildWindowDef, so after merge the inherited ORDER BY still uses a full-partition frame for SUM. The smallest practical fix is to make default frame selection happen only after resolving inherited ORDER BY and frame from the referenced window, or temporarily pin/revert the dependency bump until that fix is available.
  • Why this is likely a bug: The same PARTITION BY and ORDER BY returns correct results with inline OVER syntax, while named-window SUM returns a silently wrong value for the first row; this isolates the defect to named-window reference resolution rather than test setup. Returning incorrect aggregate results for valid SQL is a production-code correctness bug.
Relevant code

postgres/parser/parser/sql.y:13631-13634

| OVER window_name\n  {\n    $$.val = &tree.WindowDef{RefName: tree.Name($2)}\n  }

go.mod:12

github.com/dolthub/go-mysql-server v0.20.1-0.20260713191803-1e74a51e8dda
Evidence Package
Copy prompt for an agent 
Ito QA identified the following failure during automated PR testing. Please investigate and propose a fix.

**High severity — Named window reference via OVER w resolves correctly**

**What failed:** Named window reference OVER w produces a full-partition SUM per row instead of a running SUM within each partition, even though inline window syntax behaves correctly.

- **Impact:** Queries that rely on named window ORDER BY semantics can return silently incorrect running totals, which can mislead users making decisions from query results.
- **Steps to reproduce:**
  1. Create table t_named(id int, grp int, amt int) and insert (1,1,10),(2,1,20),(3,2,5).
  2. Run SELECT id, SUM(amt) OVER w AS s FROM t_named WINDOW w AS (PARTITION BY grp ORDER BY id) ORDER BY id;.
  3. Observe id=1 returns 30 instead of 10; compare with inline OVER (PARTITION BY grp ORDER BY id) which returns 10,30,5.
- **Stub / mock content:** No stubs, mocks, or bypasses were applied for this test in the recorded run.
- **Code analysis:** postgres/parser/parser/sql.y now emits RefName for OVER window_name, so named windows flow through reference merge behavior. The PR also updates go-mysql-server in go.mod, and that dependency path applies a default unbounded frame too early in buildWindowDef, so after merge the inherited ORDER BY still uses a full-partition frame for SUM. The smallest practical fix is to make default frame selection happen only after resolving inherited ORDER BY and frame from the referenced window, or temporarily pin/revert the dependency bump until that fix is available.
- **Why this is likely a bug:** The same PARTITION BY and ORDER BY returns correct results with inline OVER syntax, while named-window SUM returns a silently wrong value for the first row; this isolates the defect to named-window reference resolution rather than test setup. Returning incorrect aggregate results for valid SQL is a production-code correctness bug.

**Relevant code:**

`postgres/parser/parser/sql.y:13631-13634`

~~~yacc
| OVER window_name\n  {\n    $$.val = &tree.WindowDef{RefName: tree.Name($2)}\n  }
~~~

`go.mod:12`

~~~go
github.com/dolthub/go-mysql-server v0.20.1-0.20260713191803-1e74a51e8dda
~~~

@fulghum fulghum Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This turned out to be a real bug in go-mysql-server that had slipped through our engine tests. Fixed in: dolthub/go-mysql-server#3621

{
$$.val = &tree.WindowDef{Name: tree.Name($2)}
$$.val = &tree.WindowDef{RefName: tree.Name($2)}
}
| /* EMPTY */
{
Expand Down
26 changes: 17 additions & 9 deletions server/analyzer/init.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,25 +127,33 @@ func initEngine() {
plan.ValidateForeignKeyDefinition = validateForeignKeyDefinition

planbuilder.IsAggregateFunc = IsAggregateFunc
planbuilder.IsWindowFunc = IsWindowFunc

expression.DefaultExpressionFactory = pgexpression.PostgresExpressionFactory{}

expression.SplitConjunction = splitConjunction
}

// postgresOnlyAggregateFuncNames holds Postgres aggregate functions with no MySQL equivalent. Every name
// here must be recognized by both IsAggregateFunc and IsWindowFunc: buildScalar in GMS's planbuilder only
// routes a call with an OVER(...) clause into the window-building path if IsWindowFunc recognizes its name,
// and Postgres allows any aggregate to be used as a window function.
var postgresOnlyAggregateFuncNames = map[string]bool{
"array_agg": true,
"bool_and": true,
"bool_or": true,
}

// IsAggregateFunc checks if the given function name is an aggregate function. This is the entire set supported by
// MySQL plus some postgres specific ones.
func IsAggregateFunc(name string) bool {
if planbuilder.IsMySQLAggregateFuncName(name) {
return true
}

switch name {
case "array_agg", "bool_and", "bool_or":
return true
}
return planbuilder.IsMySQLAggregateFuncName(name) || postgresOnlyAggregateFuncNames[name]
}

return false
// IsWindowFunc checks if the given function name is a window function. This is the entire set supported by
// MySQL plus some postgres specific ones.
func IsWindowFunc(name string) bool {
return planbuilder.IsMySQLWindowFuncName(name) || postgresOnlyAggregateFuncNames[name]
}

// insertAnalyzerRules inserts the given rule(s) before or after the given analyzer.RuleId, returning an updated slice.
Expand Down
12 changes: 10 additions & 2 deletions server/analyzer/type_sanitizer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,17 @@ func TypeSanitizer(ctx *sql.Context, a *analyzer.Analyzer, node sql.Node, scope
case sql.FunctionExpression:
// Compiled functions are Doltgres functions. We're only concerned with GMS functions.
if _, ok := expr.(framework.Function); !ok {
// Some aggregation functions cannot be wrapped due to expectations in the analyzer, so we exclude them here.
// Aggregation/window-only expressions (Sum, Avg, Count, BitAnd, Rank, ...) can't be
// Eval()'d directly - only via NewBuffer/NewWindowFunction - so wrapping one in
// GMSCast (which evaluates its child directly) breaks it.
// sql.WindowAdaptableExpression is the common parent interface for both sql.Aggregation
// and sql.WindowAggregation, so checking it covers every current and future case in one
// shot. Only the *outer* reference to an aggregate's result (a GetField, handled
// elsewhere in this function) still needs its declared type corrected.
if _, ok := expr.(sql.WindowAdaptableExpression); ok {
return expr, transform.SameTree, nil
}
switch expr.FunctionName() {
case "Count", "CountDistinct", "group_concat", "JSONObjectAgg", "Sum":
case "coalesce":
// Replace GMS Coalesce with a Doltgres-native implementation that uses
// Postgres type-resolution rules (FindCommonType) to infer the result type.
Expand Down
Loading
Loading