Skip to content

Commit

Permalink
Merge #43405 #43424
Browse files Browse the repository at this point in the history 
43405: opt: fix optsteps crash caused by constraint expressions r=RaduBerinde a=RaduBerinde

#### opt: make FormatExpr easier to use

During debugging, it's sometimes useful to add a statement to print an
expression. This is fairly hard and can crash easily because the memo
or the catalog isn't set in the formatting context. In particular, it
is impossible to print a scalar expression that contains a relational
expression because `FormatExpr` only sets the memo when we print
relational expressions.

This change improves the situation by allowing the caller of
`FormatExpr` to pass the `*Memo` and by adding a convenience wrapper
method on the `Optimizer`. We also automatically unset the "fully
qualify" flag if there is no catalog (which would otherwise crash).

Release note: None

#### opt: fix optsteps crash caused by TableMeta expressions

The optbuilder creates scalar constraint and computed column
expressions and hangs them off the `TableMeta`. If a normalization
rule fires for one of these expressions, `optsteps` panics because it
can't find a path to the expression in the memo.

This change fixes the problem by adding special code in opt_steps
which effectively treats these expressions as children of Scan
expressions.

Release note: None

#### opt: show TableMeta expressions when formatting expressions

We build check constraint and computed column expressions and attach
them to `TableMeta`, to be used later by exploration rules. These are
currently invisible. This change adds them under "canonical" scans
(the normalized scan expression before any exploration rules).

Release note: None


43424: reducesql: also attempt to remove columns from PKs r=mjibson a=mjibson

Release note: None

Co-authored-by: Radu Berinde <[email protected]>
Co-authored-by: Matt Jibson <[email protected]>
  • Loading branch information
@RaduBerinde @maddyblue
3 people committed Jan 2, 2020
3 parents d037845 + 6cb068d + 151fea1 commit e643312
Show file tree
Hide file tree
Showing 22 changed files with 1,277 additions and 92 deletions.
2 changes: 1 addition & 1 deletion pkg/sql/apply_join.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ func (a *applyJoinNode) Next(params runParams) (bool, error) {
// Enhance the error with the EXPLAIN (OPT, VERBOSE) of the inner
// expression.
fmtFlags := memo.ExprFmtHideQualifications | memo.ExprFmtHideScalars | memo.ExprFmtHideTypes
explainOpt := memo.FormatExpr(newRightSide, fmtFlags, nil /* catalog */)
explainOpt := a.optimizer.FormatExpr(newRightSide, fmtFlags)
err = errors.WithDetailf(err, "newRightSide:\n%s", explainOpt)
}
return false, err
Expand Down
7 changes: 7 additions & 0 deletions pkg/sql/opt/exec/execbuilder/testdata/catalog
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,13 @@ TABLE abcdef
└── INDEX primary
└── rowid int not null default (unique_rowid()) [hidden]
scan abcdef
├── check constraint expressions
│ └── f > 2
└── computed column expressions
├── d:4
│ └── (b + c) + 1
└── e:5
└── variable: a

statement ok
CREATE TABLE uvwxy (
Expand Down
9 changes: 9 additions & 0 deletions pkg/sql/opt/memo/expr.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"strings"

"github.com/cockroachdb/cockroach/pkg/sql/opt"
"github.com/cockroachdb/cockroach/pkg/sql/opt/cat"
"github.com/cockroachdb/cockroach/pkg/sql/opt/props"
"github.com/cockroachdb/cockroach/pkg/sql/opt/props/physical"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
Expand Down Expand Up @@ -442,6 +443,14 @@ type WindowFrame struct {
FrameExclusion tree.WindowFrameExclusion
}

// IsCanonical returns true if the ScanPrivate indicates an original unaltered
// primary index Scan operator (i.e. unconstrained and not limited).
func (s *ScanPrivate) IsCanonical() bool {
return s.Index == cat.PrimaryIndex &&
s.Constraint == nil &&
s.HardLimit == 0
}

// NeedResults returns true if the mutation operator can return the rows that
// were mutated.
func (m *MutationPrivate) NeedResults() bool {
Expand Down
36 changes: 32 additions & 4 deletions pkg/sql/opt/memo/expr_format.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import (
"bytes"
"context"
"fmt"
"sort"
"strings"
"unicode"

"github.com/cockroachdb/cockroach/pkg/sql/opt"
Expand Down Expand Up @@ -90,10 +92,10 @@ func (f ExprFmtFlags) HasFlags(subset ExprFmtFlags) bool {

// FormatExpr returns a string representation of the given expression, formatted
// according to the specified flags.
func FormatExpr(e opt.Expr, flags ExprFmtFlags, catalog cat.Catalog) string {
var mem *Memo
if nd, ok := e.(RelExpr); ok {
mem = nd.Memo()
func FormatExpr(e opt.Expr, flags ExprFmtFlags, mem *Memo, catalog cat.Catalog) string {
if catalog == nil {
// Automatically hide qualifications if we have no catalog.
flags |= ExprFmtHideQualifications
}
f := MakeExprFmtCtx(flags, mem, catalog)
f.FormatExpr(e)
Expand Down Expand Up @@ -310,6 +312,32 @@ func (f *ExprFmtCtx) formatRelational(e RelExpr, tp treeprinter.Node) {
}

case *ScanExpr:
if t.IsCanonical() {
// For the canonical scan, show the expressions attached to the TableMeta.
tab := md.TableMeta(t.Table)
if len(tab.Constraints) > 0 {
c := tp.Childf("check constraint expressions")
for i := 0; i < len(tab.Constraints); i++ {
f.formatExpr(tab.Constraints[i], c)
}
}
if len(tab.ComputedCols) > 0 {
c := tp.Childf("computed column expressions")
cols := make(opt.ColList, 0, len(tab.ComputedCols))
for col := range tab.ComputedCols {
cols = append(cols, col)
}
sort.Slice(cols, func(i, j int) bool {
return cols[i] < cols[j]
})
for _, col := range cols {
f.Buffer.Reset()
formatCol(f, "" /* label */, col, opt.ColSet{} /* notNullCols */, false /* omitType */)
colInfo := strings.TrimPrefix(f.Buffer.String(), " ")
f.formatExpr(tab.ComputedCols[col], c.Child(colInfo))
}
}
}
if t.Constraint != nil {
tp.Childf("constraint: %s", t.Constraint)
}
Expand Down
28 changes: 28 additions & 0 deletions pkg/sql/opt/memo/testdata/logprops/delete
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,13 @@ delete abcde
├── interesting orderings: (+11)
├── scan abcde
│ ├── columns: a:7(int!null) b:8(int) c:9(int!null) d:10(int) rowid:11(int!null) e:12(int)
│ ├── computed column expressions
│ │ └── d:10(int)
│ │ └── plus [type=int]
│ │ ├── plus [type=int]
│ │ │ ├── variable: b [type=int]
│ │ │ └── variable: c [type=int]
│ │ └── const: 1 [type=int]
│ ├── key: (11)
│ ├── fd: (11)-->(7-10,12)
│ ├── prune: (7-12)
Expand Down Expand Up @@ -66,6 +73,13 @@ project
├── interesting orderings: (+11)
├── scan abcde
│ ├── columns: a:7(int!null) b:8(int) c:9(int!null) d:10(int) rowid:11(int!null) e:12(int)
│ ├── computed column expressions
│ │ └── d:10(int)
│ │ └── plus [type=int]
│ │ ├── plus [type=int]
│ │ │ ├── variable: b [type=int]
│ │ │ └── variable: c [type=int]
│ │ └── const: 1 [type=int]
│ ├── key: (11)
│ ├── fd: (11)-->(7-10,12)
│ ├── prune: (7-12)
Expand Down Expand Up @@ -103,6 +117,13 @@ project
├── interesting orderings: (+11)
├── scan abcde
│ ├── columns: a:7(int!null) b:8(int) c:9(int!null) d:10(int) rowid:11(int!null) e:12(int)
│ ├── computed column expressions
│ │ └── d:10(int)
│ │ └── plus [type=int]
│ │ ├── plus [type=int]
│ │ │ ├── variable: b [type=int]
│ │ │ └── variable: c [type=int]
│ │ └── const: 1 [type=int]
│ ├── key: (11)
│ ├── fd: (11)-->(7-10,12)
│ ├── prune: (7-12)
Expand Down Expand Up @@ -136,6 +157,13 @@ project
├── interesting orderings: (+11)
├── scan abcde
│ ├── columns: a:7(int!null) b:8(int) c:9(int!null) d:10(int) rowid:11(int!null) e:12(int)
│ ├── computed column expressions
│ │ └── d:10(int)
│ │ └── plus [type=int]
│ │ ├── plus [type=int]
│ │ │ ├── variable: b [type=int]
│ │ │ └── variable: c [type=int]
│ │ └── const: 1 [type=int]
│ ├── key: (11)
│ ├── fd: (11)-->(7-10,12)
│ ├── prune: (7-12)
Expand Down
28 changes: 28 additions & 0 deletions pkg/sql/opt/memo/testdata/logprops/update
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,13 @@ update abcde
│ │ ├── interesting orderings: (+11)
│ │ ├── scan abcde
│ │ │ ├── columns: a:7(int!null) b:8(int) c:9(int!null) d:10(int) rowid:11(int!null) e:12(int)
│ │ │ ├── computed column expressions
│ │ │ │ └── d:10(int)
│ │ │ │ └── plus [type=int]
│ │ │ │ ├── plus [type=int]
│ │ │ │ │ ├── variable: b [type=int]
│ │ │ │ │ └── variable: c [type=int]
│ │ │ │ └── const: 1 [type=int]
│ │ │ ├── key: (11)
│ │ │ ├── fd: (11)-->(7-10,12)
│ │ │ ├── prune: (7-12)
Expand Down Expand Up @@ -103,6 +110,13 @@ project
│ │ ├── interesting orderings: (+11)
│ │ ├── scan abcde
│ │ │ ├── columns: a:7(int!null) b:8(int) c:9(int!null) d:10(int) rowid:11(int!null) e:12(int)
│ │ │ ├── computed column expressions
│ │ │ │ └── d:10(int)
│ │ │ │ └── plus [type=int]
│ │ │ │ ├── plus [type=int]
│ │ │ │ │ ├── variable: b [type=int]
│ │ │ │ │ └── variable: c [type=int]
│ │ │ │ └── const: 1 [type=int]
│ │ │ ├── key: (11)
│ │ │ ├── fd: (11)-->(7-10,12)
│ │ │ ├── prune: (7-12)
Expand Down Expand Up @@ -164,6 +178,13 @@ project
│ │ ├── interesting orderings: (+11)
│ │ ├── scan abcde
│ │ │ ├── columns: a:7(int!null) b:8(int) c:9(int!null) d:10(int) rowid:11(int!null) e:12(int)
│ │ │ ├── computed column expressions
│ │ │ │ └── d:10(int)
│ │ │ │ └── plus [type=int]
│ │ │ │ ├── plus [type=int]
│ │ │ │ │ ├── variable: b [type=int]
│ │ │ │ │ └── variable: c [type=int]
│ │ │ │ └── const: 1 [type=int]
│ │ │ ├── key: (11)
│ │ │ ├── fd: (11)-->(7-10,12)
│ │ │ ├── prune: (7-12)
Expand Down Expand Up @@ -219,6 +240,13 @@ project
│ │ ├── interesting orderings: (+11)
│ │ ├── scan abcde
│ │ │ ├── columns: a:7(int!null) b:8(int) c:9(int!null) d:10(int) rowid:11(int!null) e:12(int)
│ │ │ ├── computed column expressions
│ │ │ │ └── d:10(int)
│ │ │ │ └── plus [type=int]
│ │ │ │ ├── plus [type=int]
│ │ │ │ │ ├── variable: b [type=int]
│ │ │ │ │ └── variable: c [type=int]
│ │ │ │ └── const: 1 [type=int]
│ │ │ ├── key: (11)
│ │ │ ├── fd: (11)-->(7-10,12)
│ │ │ ├── prune: (7-12)
Expand Down
25 changes: 25 additions & 0 deletions pkg/sql/opt/memo/testdata/logprops/upsert
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,11 @@ project
│ │ │ │ └── const: 1 [type=int]
│ │ │ ├── scan abc
│ │ │ │ ├── columns: a:10(int!null) b:11(int) c:12(int) rowid:13(int!null)
│ │ │ │ ├── computed column expressions
│ │ │ │ │ └── c:12(int)
│ │ │ │ │ └── plus [type=int]
│ │ │ │ │ ├── variable: b [type=int]
│ │ │ │ │ └── const: 1 [type=int]
│ │ │ │ ├── key: (13)
│ │ │ │ ├── fd: (13)-->(10-12), (10)-->(11-13), (11,12)~~>(10,13)
│ │ │ │ ├── prune: (10-13)
Expand Down Expand Up @@ -299,6 +304,11 @@ project
│ │ │ │ │ │ └── const: 1 [type=int]
│ │ │ │ │ ├── scan abc
│ │ │ │ │ │ ├── columns: a:10(int!null) b:11(int) c:12(int) rowid:13(int!null)
│ │ │ │ │ │ ├── computed column expressions
│ │ │ │ │ │ │ └── c:12(int)
│ │ │ │ │ │ │ └── plus [type=int]
│ │ │ │ │ │ │ ├── variable: b [type=int]
│ │ │ │ │ │ │ └── const: 1 [type=int]
│ │ │ │ │ │ ├── key: (13)
│ │ │ │ │ │ ├── fd: (13)-->(10-12), (10)-->(11-13), (11,12)~~>(10,13)
│ │ │ │ │ │ ├── prune: (10-13)
Expand All @@ -313,6 +323,11 @@ project
│ │ │ │ └── null [type=unknown]
│ │ │ ├── scan abc
│ │ │ │ ├── columns: a:14(int!null) b:15(int) c:16(int) rowid:17(int!null)
│ │ │ │ ├── computed column expressions
│ │ │ │ │ └── c:16(int)
│ │ │ │ │ └── plus [type=int]
│ │ │ │ │ ├── variable: b [type=int]
│ │ │ │ │ └── const: 1 [type=int]
│ │ │ │ ├── key: (17)
│ │ │ │ ├── fd: (17)-->(14-16), (14)-->(15-17), (15,16)~~>(14,17)
│ │ │ │ ├── prune: (14-17)
Expand All @@ -327,6 +342,11 @@ project
│ │ └── null [type=unknown]
│ ├── scan abc
│ │ ├── columns: a:18(int!null) b:19(int) c:20(int) rowid:21(int!null)
│ │ ├── computed column expressions
│ │ │ └── c:20(int)
│ │ │ └── plus [type=int]
│ │ │ ├── variable: b [type=int]
│ │ │ └── const: 1 [type=int]
│ │ ├── key: (21)
│ │ ├── fd: (21)-->(18-20), (18)-->(19-21), (19,20)~~>(18,21)
│ │ ├── prune: (18-21)
Expand Down Expand Up @@ -422,6 +442,11 @@ project
│ │ │ │ └── const: 1 [type=int]
│ │ │ ├── scan abc
│ │ │ │ ├── columns: a:9(int!null) b:10(int) c:11(int) rowid:12(int!null)
│ │ │ │ ├── computed column expressions
│ │ │ │ │ └── c:11(int)
│ │ │ │ │ └── plus [type=int]
│ │ │ │ │ ├── variable: b [type=int]
│ │ │ │ │ └── const: 1 [type=int]
│ │ │ │ ├── key: (12)
│ │ │ │ ├── fd: (12)-->(9-11), (9)-->(10-12), (10,11)~~>(9,12)
│ │ │ │ ├── prune: (9-12)
Expand Down
6 changes: 6 additions & 0 deletions pkg/sql/opt/memo/testdata/stats/insert
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,9 @@ with &1
│ ├── fd: ()-->(5), (7)-->(4,6)
│ ├── scan abc
│ │ ├── columns: a:4(int!null) b:5(string) c:6(float) rowid:7(int!null)
│ │ ├── computed column expressions
│ │ │ └── c:6(float)
│ │ │ └── a::FLOAT8 [type=float]
│ │ ├── stats: [rows=2000, distinct(4)=2000, null(4)=0, distinct(5)=10, null(5)=0, distinct(7)=2000, null(7)=0]
│ │ ├── key: (7)
│ │ └── fd: (7)-->(4-6)
Expand Down Expand Up @@ -108,6 +111,9 @@ insert xyz
├── fd: (7)-->(4-6)
├── scan abc
│ ├── columns: a:4(int!null) b:5(string) c:6(float) rowid:7(int!null)
│ ├── computed column expressions
│ │ └── c:6(float)
│ │ └── a::FLOAT8 [type=float]
│ ├── stats: [rows=2000]
│ ├── key: (7)
│ └── fd: (7)-->(4-6)
Expand Down
6 changes: 6 additions & 0 deletions pkg/sql/opt/memo/testdata/stats/upsert
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,9 @@ with &1
│ │ │ │ │ ├── fd: ()-->(5), (7)-->(4,6)
│ │ │ │ │ ├── scan abc
│ │ │ │ │ │ ├── columns: a:4(int!null) b:5(string) c:6(float) rowid:7(int!null)
│ │ │ │ │ │ ├── computed column expressions
│ │ │ │ │ │ │ └── c:6(float)
│ │ │ │ │ │ │ └── a::FLOAT8 [type=float]
│ │ │ │ │ │ ├── stats: [rows=2000, distinct(4)=2000, null(4)=0, distinct(5)=10, null(5)=0, distinct(7)=2000, null(7)=0]
│ │ │ │ │ │ ├── key: (7)
│ │ │ │ │ │ └── fd: (7)-->(4-6)
Expand Down Expand Up @@ -157,6 +160,9 @@ upsert xyz
│ ├── fd: (7)-->(4-6)
│ ├── scan abc
│ │ ├── columns: a:4(int!null) b:5(string) c:6(float) rowid:7(int!null)
│ │ ├── computed column expressions
│ │ │ └── c:6(float)
│ │ │ └── a::FLOAT8 [type=float]
│ │ ├── stats: [rows=2000]
│ │ ├── key: (7)
│ │ └── fd: (7)-->(4-6)
Expand Down
3 changes: 3 additions & 0 deletions pkg/sql/opt/norm/testdata/rules/inline
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ update computed
├── fd: ()-->(7-9), (4)-->(5,6)
├── scan computed
│ ├── columns: a:4(int!null) b:5(int) c:6(int)
│ ├── computed column expressions
│ │ └── c:6(int)
│ │ └── (a + b) + 1 [type=int]
│ ├── key: (4)
│ └── fd: (4)-->(5,6)
└── projections
Expand Down
Loading

0 comments on commit e643312

Please sign in to comment.