Skip to content

Commit

Permalink
Merge #50581
Browse files Browse the repository at this point in the history 
50581: opt: remove CanHaveSideEffects r=RaduBerinde a=RaduBerinde

#### opt: use volatility for SimplifyZeroCardinalityGroup

Switch to using volatility instead of `CanHaveSideEffects` for this rule.

Release note: None

#### opt: use volatility for CanInlineWith

Switch to using volatility instead of `CanHaveSideEffects` for this rule.

Release note: None

#### opt: use volatility for Project FDs

Switch to using volatility instead of `CanHaveSideEffects` for Project FDs.

Release note: None

#### opt: use volatility for hoisting from CASE

Switch to using volatility instead of `CanHaveSideEffects` for hoisting
expressions out of CASE and similar. We can only allow "leak proof" operators to
be hoisted.

Release note: None

#### opt: use volatility for deduplicating columns in optbuilder

Switch to using volatility instead of `CanHaveSideEffects` when deciding whether
we can reuse a projected column.

Release note: None

#### opt: remove CanHaveSideEffects

All uses of `CanHaveSideEffects` have been replaced with uses of VolatilitySet.
Remove this property.

Release note: None

Co-authored-by: Radu Berinde <radu@cockroachlabs.com>
  • Loading branch information
@RaduBerinde
craig[bot] and RaduBerinde committed Jun 25, 2020
2 parents c0502c6 + 1b07b3f commit 55bf878
Show file tree
Hide file tree
Showing 62 changed files with 800 additions and 855 deletions.
12 changes: 6 additions & 6 deletions pkg/sql/opt/exec/execbuilder/testdata/explain
View file
Original file line number Diff line number Diff line change
Expand Up @@ -689,12 +689,12 @@ values · · (column1 int, column2 int, column3 i
query TTTTT
EXPLAIN (TYPES) SELECT 2*count(k) as z, v FROM t WHERE v>123 GROUP BY v HAVING v<2 AND count(k)>1
----
· distribution local · ·
· vectorized true · ·
render · · (z int, v int) ·
│ render 0 (z)[int] · ·
│ render 1 (v)[int] · ·
└── norows · · (v int, z int) ·
· distribution local · ·
· vectorized true · ·
render · · (z int, v int) ·
│ render 0 ((count)[int] * (2)[int])[int] · ·
│ render 1 (v)[int] · ·
└── norows · · (v int, count int) ·

query TTTTT
EXPLAIN (TYPES) DELETE FROM t WHERE v > 1
Expand Down
4 changes: 2 additions & 2 deletions pkg/sql/opt/memo/expr.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -637,7 +637,7 @@ func (prj *ProjectExpr) initUnexportedFields(mem *Memo) {
continue
}

if !item.scalar.CanHaveSideEffects {
if !item.scalar.VolatilitySet.HasVolatile() {
from := item.scalar.OuterCols.Intersection(inputProps.OutputCols)

// We want to set up the FD: from --> colID.
Expand All @@ -647,7 +647,7 @@ func (prj *ProjectExpr) initUnexportedFields(mem *Memo) {
//
// We only add the FD if composite types are not involved.
//
// TODO(radu): add a allowlist of expressions/operators that are ok, like
// TODO(radu): add an allowlist of expressions/operators that are ok, like
// arithmetic.
composite := false
for i, ok := from.Next(0); ok; i, ok = from.Next(i + 1) {
Expand Down
6 changes: 0 additions & 6 deletions pkg/sql/opt/memo/expr_format.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -653,9 +653,6 @@ func (f *ExprFmtCtx) formatRelational(e RelExpr, tp treeprinter.Node) {
if !relational.VolatilitySet.IsLeakProof() {
writeFlag(relational.VolatilitySet.String())
}
if relational.CanHaveSideEffects {
writeFlag("side-effects")
}
if relational.CanMutate {
writeFlag("mutations")
}
Expand Down Expand Up @@ -923,9 +920,6 @@ func (f *ExprFmtCtx) scalarPropsStrings(scalar opt.ScalarExpr) []string {
if !scalarProps.VolatilitySet.IsLeakProof() {
emitProp(scalarProps.VolatilitySet.String())
}
if scalarProps.CanHaveSideEffects {
emitProp("side-effects")
}
if scalarProps.HasCorrelatedSubquery {
emitProp("correlated-subquery")
} else if scalarProps.HasSubquery {
Expand Down
15 changes: 2 additions & 13 deletions pkg/sql/opt/memo/logical_props_builder.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ func (b *logicalPropsBuilder) buildScanProps(scan *ScanExpr, rel *props.Relation
// ------------
// A Locking option is a side-effect (we don't want to elide this scan).
if scan.Locking != nil {
rel.CanHaveSideEffects = true
rel.VolatilitySet.AddVolatile()
}

Expand Down Expand Up @@ -911,7 +910,6 @@ func (b *logicalPropsBuilder) buildLimitProps(limit *LimitExpr, rel *props.Relat
// ------------
// Negative limits can trigger a runtime error.
if constLimit < 0 || !haveConstLimit {
rel.CanHaveSideEffects = true
rel.VolatilitySet.AddImmutable()
}

Expand Down Expand Up @@ -1356,8 +1354,8 @@ func (b *logicalPropsBuilder) buildZipItemProps(item *ZipItem, scalar *props.Sca
//
// Note that shared is an "input-output" argument, and should be assumed
// to be partially filled in already. Boolean fields such as HasPlaceholder,
// CanHaveSideEffects and HasCorrelatedSubquery should never be reset to false
// once set to true.
// HasCorrelatedSubquery should never be reset to false once set to true;
// VolatilitySet should never be re-initialized.
func BuildSharedProps(e opt.Expr, shared *props.Shared) {
switch t := e.(type) {
case *VariableExpr:
Expand Down Expand Up @@ -1387,7 +1385,6 @@ func BuildSharedProps(e opt.Expr, shared *props.Shared) {
}
}
if !nonZero {
shared.CanHaveSideEffects = true
shared.VolatilitySet.AddImmutable()
}

Expand All @@ -1401,10 +1398,6 @@ func BuildSharedProps(e opt.Expr, shared *props.Shared) {
}

case *FunctionExpr:
if t.Properties.Impure {
// Impure functions can return different value on each call.
shared.CanHaveSideEffects = true
}
shared.VolatilitySet.Add(t.Overload.Volatility)

case *CastExpr:
Expand Down Expand Up @@ -1444,7 +1437,6 @@ func BuildSharedProps(e opt.Expr, shared *props.Shared) {
}
shared.VolatilitySet.Add(o.Volatility)
} else if opt.IsMutationOp(e) {
shared.CanHaveSideEffects = true
shared.CanMutate = true
shared.VolatilitySet.AddVolatile()
}
Expand All @@ -1470,9 +1462,6 @@ func BuildSharedProps(e opt.Expr, shared *props.Shared) {
shared.HasPlaceholder = true
}
shared.VolatilitySet.UnionWith(cached.VolatilitySet)
if cached.CanHaveSideEffects {
shared.CanHaveSideEffects = true
}
if cached.CanMutate {
shared.CanMutate = true
}
Expand Down
14 changes: 7 additions & 7 deletions pkg/sql/opt/memo/testdata/logprops/delete
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ delete abcde
├── columns: <none>
├── fetch columns: a:7(int) b:8(int) c:9(int) d:10(int) rowid:11(int) e:12(int)
├── cardinality: [0 - 0]
├── volatile, side-effects, mutations
├── volatile, mutations
└── select
├── columns: a:7(int!null) b:8(int) c:9(int!null) d:10(int) rowid:11(int!null) e:12(int)
├── key: (11)
Expand Down Expand Up @@ -55,13 +55,13 @@ DELETE FROM abcde WHERE a=1 RETURNING *
----
project
├── columns: a:1(int!null) b:2(int) c:3(int!null) d:4(int)
├── volatile, side-effects, mutations
├── volatile, mutations
├── fd: ()-->(1)
├── prune: (1-4)
└── delete abcde
├── columns: a:1(int!null) b:2(int) c:3(int!null) d:4(int) rowid:5(int!null)
├── fetch columns: a:7(int) b:8(int) c:9(int) d:10(int) rowid:11(int) e:12(int)
├── volatile, side-effects, mutations
├── volatile, mutations
├── key: (5)
├── fd: ()-->(1), (5)-->(2-4)
├── prune: (1-4)
Expand Down Expand Up @@ -96,15 +96,15 @@ DELETE FROM abcde WHERE rowid=1 RETURNING *
project
├── columns: a:1(int!null) b:2(int) c:3(int!null) d:4(int)
├── cardinality: [0 - 1]
├── volatile, side-effects, mutations
├── volatile, mutations
├── key: ()
├── fd: ()-->(1-4)
├── prune: (1-4)
└── delete abcde
├── columns: a:1(int!null) b:2(int) c:3(int!null) d:4(int) rowid:5(int!null)
├── fetch columns: a:7(int) b:8(int) c:9(int) d:10(int) rowid:11(int) e:12(int)
├── cardinality: [0 - 1]
├── volatile, side-effects, mutations
├── volatile, mutations
├── key: ()
├── fd: ()-->(1-5)
├── prune: (1-4)
Expand Down Expand Up @@ -139,13 +139,13 @@ DELETE FROM abcde WHERE b=c RETURNING *;
----
project
├── columns: a:1(int!null) b:2(int!null) c:3(int!null) d:4(int)
├── volatile, side-effects, mutations
├── volatile, mutations
├── fd: (2)==(3), (3)==(2)
├── prune: (1-4)
└── delete abcde
├── columns: a:1(int!null) b:2(int!null) c:3(int!null) d:4(int) rowid:5(int!null)
├── fetch columns: a:7(int) b:8(int) c:9(int) d:10(int) rowid:11(int) e:12(int)
├── volatile, side-effects, mutations
├── volatile, mutations
├── key: (5)
├── fd: (2)==(3), (3)==(2), (5)-->(1-4)
├── prune: (1-4)
Expand Down
46 changes: 23 additions & 23 deletions pkg/sql/opt/memo/testdata/logprops/insert
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,18 +30,18 @@ insert abcde
│ ├── column11:11 => rowid:5
│ └── column12:12 => e:6
├── cardinality: [0 - 0]
├── volatile, side-effects, mutations
├── volatile, mutations
└── project
├── columns: column13:13(int!null) y:8(int!null) column10:10(int!null) column11:11(int) column12:12(int)
├── cardinality: [0 - 10]
├── volatile, side-effects
├── volatile
├── fd: ()-->(10,12), (8)-->(13)
├── prune: (8,10-13)
├── interesting orderings: (+8)
├── project
│ ├── columns: column10:10(int!null) column11:11(int) column12:12(int) y:8(int!null)
│ ├── cardinality: [0 - 10]
│ ├── volatile, side-effects
│ ├── volatile
│ ├── fd: ()-->(10,12)
│ ├── prune: (8,10-12)
│ ├── interesting orderings: (+8)
Expand All @@ -67,7 +67,7 @@ insert abcde
│ │ └── const: 10 [type=int]
│ └── projections
│ ├── const: 10 [as=column10:10, type=int]
│ ├── function: unique_rowid [as=column11:11, type=int, volatile, side-effects]
│ ├── function: unique_rowid [as=column11:11, type=int, volatile]
│ └── cast: INT8 [as=column12:12, type=int, immutable]
│ └── null [type=unknown]
└── projections
Expand All @@ -84,7 +84,7 @@ INSERT INTO abcde (a, b) SELECT y, y FROM xyz ORDER BY y, z LIMIT 10 RETURNING *
project
├── columns: a:1(int!null) b:2(int!null) c:3(int!null) d:4(int!null)
├── cardinality: [0 - 10]
├── volatile, side-effects, mutations
├── volatile, mutations
├── fd: ()-->(3), (1)==(2), (2)==(1), (1)-->(4)
├── prune: (1-4)
└── insert abcde
Expand All @@ -97,19 +97,19 @@ project
│ ├── column11:11 => rowid:5
│ └── column12:12 => e:6
├── cardinality: [0 - 10]
├── volatile, side-effects, mutations
├── volatile, mutations
├── fd: ()-->(3), (1)==(2), (2)==(1), (1)-->(4)
└── project
├── columns: column13:13(int!null) y:8(int!null) column10:10(int!null) column11:11(int) column12:12(int)
├── cardinality: [0 - 10]
├── volatile, side-effects
├── volatile
├── fd: ()-->(10,12), (8)-->(13)
├── prune: (8,10-13)
├── interesting orderings: (+8)
├── project
│ ├── columns: column10:10(int!null) column11:11(int) column12:12(int) y:8(int!null)
│ ├── cardinality: [0 - 10]
│ ├── volatile, side-effects
│ ├── volatile
│ ├── fd: ()-->(10,12)
│ ├── prune: (8,10-12)
│ ├── interesting orderings: (+8)
Expand All @@ -135,7 +135,7 @@ project
│ │ └── const: 10 [type=int]
│ └── projections
│ ├── const: 10 [as=column10:10, type=int]
│ ├── function: unique_rowid [as=column11:11, type=int, volatile, side-effects]
│ ├── function: unique_rowid [as=column11:11, type=int, volatile]
│ └── cast: INT8 [as=column12:12, type=int, immutable]
│ └── null [type=unknown]
└── projections
Expand All @@ -151,7 +151,7 @@ INSERT INTO abcde (a, b) SELECT y, y FROM xyz ORDER BY y, z RETURNING *
----
project
├── columns: a:1(int!null) b:2(int!null) c:3(int!null) d:4(int!null)
├── volatile, side-effects, mutations
├── volatile, mutations
├── fd: ()-->(3), (1)==(2), (2)==(1), (1)-->(4)
├── prune: (1-4)
└── insert abcde
Expand All @@ -163,16 +163,16 @@ project
│ ├── column13:13 => d:4
│ ├── column11:11 => rowid:5
│ └── column12:12 => e:6
├── volatile, side-effects, mutations
├── volatile, mutations
├── fd: ()-->(3), (1)==(2), (2)==(1), (1)-->(4)
└── project
├── columns: column13:13(int!null) y:8(int!null) column10:10(int!null) column11:11(int) column12:12(int)
├── volatile, side-effects
├── volatile
├── fd: ()-->(10,12), (8)-->(13)
├── prune: (8,10-13)
├── project
│ ├── columns: column10:10(int!null) column11:11(int) column12:12(int) y:8(int!null)
│ ├── volatile, side-effects
│ ├── volatile
│ ├── fd: ()-->(10,12)
│ ├── prune: (8,10-12)
│ ├── project
Expand All @@ -186,7 +186,7 @@ project
│ │ └── interesting orderings: (+7)
│ └── projections
│ ├── const: 10 [as=column10:10, type=int]
│ ├── function: unique_rowid [as=column11:11, type=int, volatile, side-effects]
│ ├── function: unique_rowid [as=column11:11, type=int, volatile]
│ └── cast: INT8 [as=column12:12, type=int, immutable]
│ └── null [type=unknown]
└── projections
Expand All @@ -210,20 +210,20 @@ insert abcde
│ ├── column10:10 => rowid:5
│ └── column11:11 => e:6
├── cardinality: [1 - 1]
├── volatile, side-effects, mutations
├── volatile, mutations
├── key: ()
├── fd: ()-->(1-5)
└── project
├── columns: column12:12(int!null) column1:7(int!null) column2:8(int!null) column9:9(int!null) column10:10(int) column11:11(int)
├── cardinality: [1 - 1]
├── volatile, side-effects
├── volatile
├── key: ()
├── fd: ()-->(7-12)
├── prune: (7-12)
├── project
│ ├── columns: column9:9(int!null) column10:10(int) column11:11(int) column1:7(int!null) column2:8(int!null)
│ ├── cardinality: [1 - 1]
│ ├── volatile, side-effects
│ ├── volatile
│ ├── key: ()
│ ├── fd: ()-->(7-11)
│ ├── prune: (7-11)
Expand All @@ -238,7 +238,7 @@ insert abcde
│ │ └── const: 2 [type=int]
│ └── projections
│ ├── const: 10 [as=column9:9, type=int]
│ ├── function: unique_rowid [as=column10:10, type=int, volatile, side-effects]
│ ├── function: unique_rowid [as=column10:10, type=int, volatile]
│ └── cast: INT8 [as=column11:11, type=int, immutable]
│ └── null [type=unknown]
└── projections
Expand All @@ -254,7 +254,7 @@ INSERT INTO abcde (a, b) SELECT y, (z+1)::int FROM xyz WHERE y=1 RETURNING a, c;
----
project
├── columns: a:1(int!null) c:3(int!null)
├── volatile, side-effects, mutations
├── volatile, mutations
├── fd: ()-->(1,3)
├── prune: (1,3)
└── insert abcde
Expand All @@ -266,16 +266,16 @@ project
│ ├── column14:14 => d:4
│ ├── column12:12 => rowid:5
│ └── column13:13 => e:6
├── volatile, side-effects, mutations
├── volatile, mutations
├── fd: ()-->(1,3), (2)-->(4)
└── project
├── columns: column14:14(int) y:8(int!null) int8:10(int) column11:11(int!null) column12:12(int) column13:13(int)
├── volatile, side-effects
├── volatile
├── fd: ()-->(8,11,13), (10)-->(14)
├── prune: (8,10-14)
├── project
│ ├── columns: column11:11(int!null) column12:12(int) column13:13(int) y:8(int!null) int8:10(int)
│ ├── volatile, side-effects
│ ├── volatile
│ ├── fd: ()-->(8,11,13)
│ ├── prune: (8,10-13)
│ ├── project
Expand Down Expand Up @@ -306,7 +306,7 @@ project
│ │ └── const: 1.0 [type=float]
│ └── projections
│ ├── const: 10 [as=column11:11, type=int]
│ ├── function: unique_rowid [as=column12:12, type=int, volatile, side-effects]
│ ├── function: unique_rowid [as=column12:12, type=int, volatile]
│ └── cast: INT8 [as=column13:13, type=int, immutable]
│ └── null [type=unknown]
└── projections
Expand Down
8 changes: 4 additions & 4 deletions pkg/sql/opt/memo/testdata/logprops/join
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1476,13 +1476,13 @@ SELECT (SELECT m FROM
----
with &1
├── columns: m:19(int)
├── volatile, side-effects, mutations
├── volatile, mutations
├── fd: ()-->(19)
├── prune: (19)
├── project
│ ├── columns: uv.u:4(int!null) uv.v:5(int!null)
│ ├── cardinality: [1 - 1]
│ ├── volatile, side-effects, mutations
│ ├── volatile, mutations
│ ├── key: ()
│ ├── fd: ()-->(4,5)
│ ├── prune: (4,5)
Expand All @@ -1493,13 +1493,13 @@ with &1
│ │ ├── column2:8 => uv.v:5
│ │ └── column9:9 => rowid:6
│ ├── cardinality: [1 - 1]
│ ├── volatile, side-effects, mutations
│ ├── volatile, mutations
│ ├── key: ()
│ ├── fd: ()-->(4-6)
│ └── values
│ ├── columns: column1:7(int!null) column2:8(int!null) column9:9(int)
│ ├── cardinality: [1 - 1]
│ ├── volatile, side-effects
│ ├── volatile
│ ├── key: ()
│ ├── fd: ()-->(7-9)
│ ├── prune: (7-9)
Expand Down
Loading

0 comments on commit 55bf878

Please sign in to comment.