Skip to content

Commit

Permalink
Merge #38270
Browse files Browse the repository at this point in the history
38270: sql: prohibit offset with GROUPS containing vars r=justinj a=justinj

This commit adds a check during semantic analysis to verify that a
window function using GROUPS mode does not include an offset expression
containing variable references. This was handled by the heuristic planner
previously, but this places a check earlier so that it's also checked by the
optimizer.

Fixes #38090

Release note: None

Co-authored-by: Justin Jaffray <justin@cockroachlabs.com>
  • Loading branch information
craig[bot] and Justin Jaffray committed Jun 18, 2019
2 parents 5f65390 + e3a2fe6 commit a009282
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 0 deletions.
3 changes: 3 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/window
Original file line number Diff line number Diff line change
Expand Up @@ -2832,6 +2832,9 @@ iPad {"Microsoft Lumia","HTC One",Nexus,iPhone,"HP Elite","Lenovo Th
Kindle Fire {"Microsoft Lumia","HTC One",Nexus,iPhone,"HP Elite","Lenovo Thinkpad","Sony VAIO",Dell,iPad,"Kindle Fire"}
Samsung {"Microsoft Lumia","HTC One",Nexus,iPhone,"HP Elite","Lenovo Thinkpad","Sony VAIO",Dell,iPad,"Kindle Fire",Samsung}

statement error GROUPS offset cannot contain variables
SELECT avg(price) OVER (GROUPS group_id PRECEDING) FROM products

statement error frame starting offset must not be null
SELECT avg(price) OVER (GROUPS NULL PRECEDING) FROM products

Expand Down
5 changes: 5 additions & 0 deletions pkg/sql/opt/optbuilder/testdata/window
Original file line number Diff line number Diff line change
Expand Up @@ -1548,3 +1548,8 @@ build
SELECT rank() OVER (w) FROM kv WINDOW w as (ORDER BY v RANGE BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW)
----
error (42P20): cannot copy window "w" because it has a frame clause

build
SELECT rank() OVER (ORDER BY k GROUPS k PRECEDING) FROM kv
----
error (42601): GROUPS offset cannot contain variables
11 changes: 11 additions & 0 deletions pkg/sql/sem/tree/type_check.go
Original file line number Diff line number Diff line change
Expand Up @@ -693,6 +693,7 @@ func (expr *ComparisonExpr) TypeCheck(ctx *SemaContext, desired *types.T) (Typed

var (
errOrderByIndexInWindow = pgerror.New(pgcode.FeatureNotSupported, "ORDER BY INDEX in window definition is not supported")
errVarOffsetGroups = pgerror.New(pgcode.Syntax, fmt.Sprintf("GROUPS offset cannot contain variables"))
errStarNotAllowed = pgerror.New(pgcode.Syntax, "cannot use \"*\" in this context")
errInvalidDefaultUsage = pgerror.New(pgcode.Syntax, "DEFAULT can only appear in a VALUES list within INSERT or on the right side of a SET")
errInvalidMaxUsage = pgerror.New(pgcode.Syntax, "MAXVALUE can only appear within a range partition expression")
Expand Down Expand Up @@ -998,6 +999,16 @@ func (f *WindowFrame) TypeCheck(ctx *SemaContext, windowDef *WindowDef) error {
}
}
case GROUPS:
if startBound != nil && startBound.HasOffset() {
if ContainsVars(startBound.OffsetExpr) {
return errVarOffsetGroups
}
}
if endBound != nil && endBound.HasOffset() {
if ContainsVars(endBound.OffsetExpr) {
return errVarOffsetGroups
}
}
// In GROUPS mode, offsets must be non-null, non-negative integers.
// Non-nullity and non-negativity will be checked later.
requiredType = types.Int
Expand Down

0 comments on commit a009282

Please sign in to comment.