Skip to content
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

Jump to bottom

opt,sql: disallow column references in ROWS/RANGE #40832

Merged
merged 1 commit into from
Sep 19, 2019
Merged

opt,sql: disallow column references in ROWS/RANGE #40832

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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 pkg/sql/logictest/testdata/logic_test/window
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2849,7 +2849,7 @@ 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
statement error GROUPS mode requires an ORDER BY clause
SELECT avg(price) OVER (GROUPS group_id PRECEDING) FROM products

statement error GROUPS mode requires an ORDER BY clause
Expand Down
84 changes: 0 additions & 84 deletions pkg/sql/opt/norm/testdata/rules/decorrelate
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -610,90 +610,6 @@ project
└── windows
└── row-number [type=int]

norm expect=TryDecorrelateWindow
SELECT
*
FROM
uv,
LATERAL (
SELECT
row_number() OVER (
PARTITION BY
s
ORDER BY
f
RANGE
BETWEEN u::FLOAT PRECEDING AND UNBOUNDED FOLLOWING
),
i
FROM
a
)
----
project
├── columns: u:1(int!null) v:2(int) row_number:8(int) i:4(int)
├── fd: (1)-->(2)
└── window partition=(1,6) ordering=+5 opt(1,2,6)
├── columns: u:1(int!null) v:2(int) i:4(int) f:5(float) s:6(string) row_number:8(int)
├── fd: (1)-->(2)
├── inner-join (hash)
│ ├── columns: u:1(int!null) v:2(int) i:4(int) f:5(float) s:6(string)
│ ├── fd: (1)-->(2)
│ ├── scan uv
│ │ ├── columns: u:1(int!null) v:2(int)
│ │ ├── key: (1)
│ │ └── fd: (1)-->(2)
│ ├── scan a
│ │ └── columns: i:4(int) f:5(float) s:6(string)
│ └── filters (true)
└── windows
└── windows-item: range from offset to unbounded [type=int, outer=(1)]
└── window-from-offset [type=int]
├── row-number [type=int]
└── u::FLOAT8 [type=float]

norm expect=TryDecorrelateWindow
SELECT
*
FROM
uv,
LATERAL (
SELECT
row_number() OVER (
PARTITION BY
s
ORDER BY
f
RANGE
BETWEEN UNBOUNDED PRECEDING AND u::FLOAT FOLLOWING
),
i
FROM
a
)
----
project
├── columns: u:1(int!null) v:2(int) row_number:8(int) i:4(int)
├── fd: (1)-->(2)
└── window partition=(1,6) ordering=+5 opt(1,2,6)
├── columns: u:1(int!null) v:2(int) i:4(int) f:5(float) s:6(string) row_number:8(int)
├── fd: (1)-->(2)
├── inner-join (hash)
│ ├── columns: u:1(int!null) v:2(int) i:4(int) f:5(float) s:6(string)
│ ├── fd: (1)-->(2)
│ ├── scan uv
│ │ ├── columns: u:1(int!null) v:2(int)
│ │ ├── key: (1)
│ │ └── fd: (1)-->(2)
│ ├── scan a
│ │ └── columns: i:4(int) f:5(float) s:6(string)
│ └── filters (true)
└── windows
└── windows-item: range from unbounded to offset [type=int, outer=(1)]
└── window-to-offset [type=int]
├── row-number [type=int]
└── u::FLOAT8 [type=float]

norm expect=TryDecorrelateWindow
SELECT * FROM uv, LATERAL (SELECT avg(f) FILTER (WHERE u = 3) OVER (), i FROM a)
----
Expand Down
17 changes: 0 additions & 17 deletions pkg/sql/opt/norm/testdata/rules/prune_cols
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1657,23 +1657,6 @@ project
└── ntile [type=int, outer=(2)]
└── variable: t.public.a.i [type=int]

# Ensure OFFSET expressions don't get pruned.
norm
SELECT ntile(i) OVER (ORDER BY i RANGE BETWEEN f::INT PRECEDING AND CURRENT ROW) FROM a
----
project
├── columns: ntile:5(int)
└── window partition=() ordering=+2
├── columns: i:2(int) f:3(float) ntile:5(int)
├── scan a
│ └── columns: i:2(int) f:3(float)
└── windows
└── windows-item: range from offset to current-row [type=int, outer=(2,3)]
└── window-from-offset [type=int]
├── ntile [type=int]
│ └── variable: i [type=int]
└── f::INT8 [type=int]

# Ensure filter cols don't get pruned.
norm
SELECT
Expand Down
11 changes: 0 additions & 11 deletions pkg/sql/opt/optbuilder/scope.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1140,7 +1140,6 @@ func (s *scope) replaceWindowFn(f *tree.FuncExpr, def *tree.FunctionDefinition)

var (
errOrderByIndexInWindow = pgerror.New(pgcode.FeatureNotSupported, "ORDER BY INDEX in window definition is not supported")
errVarOffsetGroups = pgerror.New(pgcode.Syntax, "GROUPS offset cannot contain variables")
)

// analyzeWindowFrame performs semantic analysis of offset expressions of
Expand Down Expand Up @@ -1175,16 +1174,6 @@ func analyzeWindowFrame(s *scope, windowDef *tree.WindowDef) error {
}
}
case tree.GROUPS:
if startBound != nil && startBound.HasOffset() {
if tree.ContainsVars(startBound.OffsetExpr) {
return errVarOffsetGroups
}
}
if endBound != nil && endBound.HasOffset() {
if tree.ContainsVars(endBound.OffsetExpr) {
return errVarOffsetGroups
}
}
if len(windowDef.OrderBy) == 0 {
return pgerror.Newf(pgcode.Windowing, "GROUPS mode requires an ORDER BY clause")
}
Expand Down
63 changes: 19 additions & 44 deletions pkg/sql/opt/optbuilder/testdata/window
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -942,20 +942,7 @@ project
build
SELECT avg(k) OVER (ORDER BY v RANGE BETWEEN k - 10 PRECEDING AND CURRENT ROW) FROM kv
----
project
├── columns: avg:8(decimal)
└── window partition=() ordering=+2
├── columns: k:1(int!null) v:2(int) w:3(int) f:4(float) d:5(decimal) s:6(string) b:7(bool) avg:8(decimal)
├── scan kv
│ └── columns: k:1(int!null) v:2(int) w:3(int) f:4(float) d:5(decimal) s:6(string) b:7(bool)
└── windows
└── windows-item: range from offset to current-row [type=decimal]
└── window-from-offset [type=decimal]
├── avg [type=decimal]
│ └── variable: k [type=int]
└── minus [type=int]
├── variable: k [type=int]
└── const: 10 [type=int]
error (42P10): argument of RANGE must not contain variables

build
SELECT avg(k) OVER (ORDER BY v RANGE BETWEEN UNBOUNDED PRECEDING AND 10 FOLLOWING) FROM kv
Expand Down Expand Up @@ -1061,35 +1048,7 @@ SELECT
FROM
kv outer_table
----
project
├── columns: avg:16(decimal)
└── window partition=(3)
├── columns: outer_table.k:1(int!null) outer_table.v:2(int) outer_table.w:3(int) outer_table.f:4(float) outer_table.d:5(decimal) outer_table.s:6(string) outer_table.b:7(bool) avg:16(decimal)
├── scan outer_table
│ └── columns: outer_table.k:1(int!null) outer_table.v:2(int) outer_table.w:3(int) outer_table.f:4(float) outer_table.d:5(decimal) outer_table.s:6(string) outer_table.b:7(bool)
└── windows
└── windows-item: rows from offset to offset [type=decimal]
└── window-to-offset [type=decimal]
├── window-from-offset [type=decimal]
│ ├── avg [type=decimal]
│ │ └── variable: outer_table.v [type=int]
│ └── subquery [type=int]
│ └── max1-row
│ ├── columns: count_rows:15(int)
│ └── scalar-group-by
│ ├── columns: count_rows:15(int)
│ ├── project
│ │ └── select
│ │ ├── columns: inner_table.k:8(int!null) inner_table.v:9(int) inner_table.w:10(int) inner_table.f:11(float) inner_table.d:12(decimal) inner_table.s:13(string) inner_table.b:14(bool)
│ │ ├── scan inner_table
│ │ │ └── columns: inner_table.k:8(int!null) inner_table.v:9(int) inner_table.w:10(int) inner_table.f:11(float) inner_table.d:12(decimal) inner_table.s:13(string) inner_table.b:14(bool)
│ │ └── filters
│ │ └── eq [type=bool]
│ │ ├── variable: inner_table.k [type=int]
│ │ └── variable: outer_table.v [type=int]
│ └── aggregations
│ └── count-rows [type=int]
└── const: 1 [type=int]
error (42P10): argument of ROWS must not contain variables

build
SELECT
Expand Down Expand Up @@ -1555,7 +1514,7 @@ 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
error (42P10): argument of GROUPS must not contain variables

build
SELECT
Expand All @@ -1579,3 +1538,19 @@ project
├── windows-item: range from unbounded to current-row exclude ties [type=int]
│ └── rank [type=int]
└── rank [type=int]

exec-ddl
CREATE TABLE table1 (col5 CHAR, col8 INT2);
----

build
SELECT
min(tab_536191.col5) OVER (
ROWS BETWEEN tab_536191.col8 FOLLOWING AND 1 FOLLOWING
)
FROM
table1 AS tab_536191
GROUP BY
tab_536191.col8, tab_536191.col5
----
error (42P10): argument of ROWS must not contain variables
21 changes: 19 additions & 2 deletions pkg/sql/opt/optbuilder/window.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/opt"
"github.com/cockroachdb/cockroach/pkg/sql/opt/memo"
"github.com/cockroachdb/cockroach/pkg/sql/opt/props/physical"
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/sql/types"
"github.com/cockroachdb/errors"
Expand Down Expand Up @@ -126,6 +128,7 @@ func (b *Builder) buildWindow(outScope *scope, inScope *scope) {
b.constructProjectForScope(outScope, argScope)
outScope.expr = argScope.expr

var referencedCols opt.ColSet
// frames accumulates the set of distinct window frames we're computing over
// so that we can group functions over the same partition and ordering.
frames := make([]memo.WindowExpr, 0, len(inScope.windows))
Expand All @@ -142,7 +145,9 @@ func (b *Builder) buildWindow(outScope *scope, inScope *scope) {
b.buildScalar(
w.WindowDef.Frame.Bounds.StartBound.OffsetExpr.(tree.TypedExpr),
inScope,
nil, nil, nil,
nil,
nil,
&referencedCols,
),
)
}
Expand All @@ -153,7 +158,19 @@ func (b *Builder) buildWindow(outScope *scope, inScope *scope) {
b.buildScalar(
w.WindowDef.Frame.Bounds.EndBound.OffsetExpr.(tree.TypedExpr),
inScope,
nil, nil, nil,
nil,
nil,
&referencedCols,
),
)
}

if !referencedCols.Empty() {
panic(
pgerror.Newf(
pgcode.InvalidColumnReference,
"argument of %s must not contain variables",
tree.WindowModeName(windowFrames[i].Mode),
),
)
}
Expand Down
20 changes: 13 additions & 7 deletions pkg/sql/sem/tree/select.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -923,18 +923,24 @@ func (node WindowFrameExclusion) Format(ctx *FmtCtx) {
}
}

// Format implements the NodeFormatter interface.
func (node *WindowFrame) Format(ctx *FmtCtx) {
switch node.Mode {
// WindowModeName returns the name of the window frame mode.
func WindowModeName(mode WindowFrameMode) string {
switch mode {
case RANGE:
ctx.WriteString("RANGE ")
return "RANGE"
case ROWS:
ctx.WriteString("ROWS ")
return "ROWS"
case GROUPS:
ctx.WriteString("GROUPS ")
return "GROUPS"
default:
panic(errors.AssertionFailedf("unhandled case: %d", log.Safe(node.Mode)))
panic(errors.AssertionFailedf("unhandled case: %d", log.Safe(mode)))
}
}

// Format implements the NodeFormatter interface.
func (node *WindowFrame) Format(ctx *FmtCtx) {
ctx.WriteString(WindowModeName(node.Mode))
ctx.WriteByte(' ')
if node.Bounds.EndBound != nil {
ctx.WriteString("BETWEEN ")
ctx.FormatNode(node.Bounds.StartBound)
Expand Down