diff --git a/pkg/sql/opt/bench/bench_test.go b/pkg/sql/opt/bench/bench_test.go index 4cf1b7d3b6c3..ed8be58205bc 100644 --- a/pkg/sql/opt/bench/bench_test.go +++ b/pkg/sql/opt/bench/bench_test.go @@ -316,8 +316,7 @@ func (h *harness) runUsingAPI(tb testing.TB, bmType BenchmarkType, query string) h.optimizer.DisableOptimizations() } bld := optbuilder.New(h.ctx, &h.semaCtx, &h.evalCtx, h.cat, h.optimizer.Factory(), h.stmt) - root, props, err := bld.Build() - if err != nil { + if err := bld.Build(); err != nil { tb.Fatalf("%v", err) } @@ -325,5 +324,5 @@ func (h *harness) runUsingAPI(tb testing.TB, bmType BenchmarkType, query string) return } - h.optimizer.Optimize(root, props) + h.optimizer.Optimize() } diff --git a/pkg/sql/opt/idxconstraint/index_constraints_test.go b/pkg/sql/opt/idxconstraint/index_constraints_test.go index e2b303845123..b47110d57b83 100644 --- a/pkg/sql/opt/idxconstraint/index_constraints_test.go +++ b/pkg/sql/opt/idxconstraint/index_constraints_test.go @@ -145,11 +145,11 @@ func TestIndexConstraints(t *testing.T) { } b := optbuilder.NewScalar(ctx, &semaCtx, &evalCtx, &f) b.AllowUnsupportedExpr = true - group, err := b.Build(typedExpr) + err = b.Build(typedExpr) if err != nil { return fmt.Sprintf("error: %v\n", err) } - ev := memo.MakeNormExprView(f.Memo(), group) + ev := f.Memo().Root() var ic idxconstraint.Instance ic.Init(ev, indexCols, notNullCols, invertedIndex, &evalCtx, &f) @@ -256,11 +256,11 @@ func BenchmarkIndexConstraints(b *testing.B) { evalCtx := tree.MakeTestingEvalContext(cluster.MakeTestingClusterSettings()) bld := optbuilder.NewScalar(context.Background(), &semaCtx, &evalCtx, &f) - group, err := bld.Build(typedExpr) + err = bld.Build(typedExpr) if err != nil { b.Fatal(err) } - ev := memo.MakeNormExprView(f.Memo(), group) + ev := f.Memo().Root() b.ResetTimer() for i := 0; i < b.N; i++ { var ic idxconstraint.Instance diff --git a/pkg/sql/opt/memo/expr_view.go b/pkg/sql/opt/memo/expr_view.go index 0284dd3058a1..1c57979879a9 100644 --- a/pkg/sql/opt/memo/expr_view.go +++ b/pkg/sql/opt/memo/expr_view.go @@ -211,10 +211,6 @@ func (ev ExprView) bestExpr() *BestExpr { return ev.mem.group(ev.group).bestExpr(ev.best) } -func (ev ExprView) bestExprID() BestExprID { - return BestExprID{group: ev.group, ordinal: ev.best} -} - // -------------------------------------------------------------------- // String representation. // -------------------------------------------------------------------- diff --git a/pkg/sql/opt/memo/expr_view_test.go b/pkg/sql/opt/memo/expr_view_test.go index 7b7c6c79b661..2283eff6f353 100644 --- a/pkg/sql/opt/memo/expr_view_test.go +++ b/pkg/sql/opt/memo/expr_view_test.go @@ -51,11 +51,11 @@ func BenchmarkExprView(b *testing.B) { bld := optbuilder.New( context.Background(), &semaCtx, &evalCtx, catalog, o.Factory(), stmt, ) - root, props, err := bld.Build() + err = bld.Build() if err != nil { b.Fatal(err) } - exprView := o.Optimize(root, props) + exprView := o.Optimize() stack := make([]memo.ExprView, 16) for i := 0; i < b.N; i++ { diff --git a/pkg/sql/opt/memo/memo.go b/pkg/sql/opt/memo/memo.go index 5281921a3d90..5255d574bd35 100644 --- a/pkg/sql/opt/memo/memo.go +++ b/pkg/sql/opt/memo/memo.go @@ -136,9 +136,13 @@ type Memo struct { // there are so many duplicates. privateStorage privateStorage - // root is the root of the lowest cost expression tree in the memo. It is - // set once after optimization is complete. - root BestExprID + // rootGroup is the root group of the memo expression forest. It is set via + // a call to SetRoot. + rootGroup GroupID + + // rootProps are the physical properties required of the root memo group. It + // is set via a call to SetRoot. + rootProps PhysicalPropsID // memEstimate is the approximate memory usage of the memo, in bytes. memEstimate int64 @@ -166,7 +170,8 @@ func (m *Memo) Init() { m.listStorage.init() m.privateStorage.init() - m.root = BestExprID{} + m.rootGroup = 0 + m.rootProps = 0 m.memEstimate = 0 } @@ -182,30 +187,45 @@ func (m *Memo) Metadata() *opt.Metadata { return &m.metadata } -// Root returns the root of the memo's lowest cost expression tree. It can only -// be accessed once the memo has been fully optimized. +// RootGroup returns the root memo group previously set via a call to SetRoot. +func (m *Memo) RootGroup() GroupID { + return m.rootGroup +} + +// RootProps returns the physical properties required of the root memo group, +// previously set via a call to SetRoot. +func (m *Memo) RootProps() PhysicalPropsID { + return m.rootProps +} + +// Root returns an ExprView wrapper around the root of the memo. If the memo has +// not yet been optimized, this will be a view over the normalized expression +// tree. Otherwise, it's a view over the lowest cost expression tree. func (m *Memo) Root() ExprView { - if !m.isOptimized() { - panic("memo has not yet been optimized and had its root expression set") + if m.isOptimized() { + root := m.group(m.rootGroup) + for i, n := 0, root.bestExprCount(); i < n; i++ { + be := root.bestExpr(bestOrdinal(i)) + if be.required == m.rootProps { + return MakeExprView(m, BestExprID{group: m.rootGroup, ordinal: bestOrdinal(i)}) + } + } + panic("could not find best expression that matches the root properties") } - return MakeExprView(m, m.root) + return MakeNormExprView(m, m.rootGroup) } -// SetRoot stores the root of the memo's lowest cost expression tree. -func (m *Memo) SetRoot(root ExprView) { - if root.mem != m { - panic("the given root is in a different memo") - } - if root.best == normBestOrdinal { - panic("cannot set the memo root to be a normalized expression tree") - } - m.root = root.bestExprID() +// SetRoot stores the root memo group, as well as the physical properties +// required of the root group. +func (m *Memo) SetRoot(group GroupID, physical PhysicalPropsID) { + m.rootGroup = group + m.rootProps = physical } // isOptimized returns true if the memo has been fully optimized. func (m *Memo) isOptimized() bool { // The memo is optimized once a best expression has been set at the root. - return m.root.ordinal != normBestOrdinal + return m.rootGroup != 0 && m.group(m.rootGroup).firstBestExpr.initialized() } // -------------------------------------------------------------------- diff --git a/pkg/sql/opt/memo/memo_format.go b/pkg/sql/opt/memo/memo_format.go index 6dbd1a161270..6b5282a79804 100644 --- a/pkg/sql/opt/memo/memo_format.go +++ b/pkg/sql/opt/memo/memo_format.go @@ -36,7 +36,7 @@ func (m *Memo) format(f *memoFmtCtx) string { // If requested, we topological sort the memo with respect to its root group. // Otherwise, we simply print out all the groups in the order and with the // names they're referred to physically. - if f.flags.HasFlags(FmtRaw) || !m.isOptimized() { + if f.flags.HasFlags(FmtRaw) { // In this case we renumber with the identity mapping, so the groups have // the same numbers as they're represented with internally. f.ordering = make([]GroupID, len(m.groups)-1) @@ -44,7 +44,7 @@ func (m *Memo) format(f *memoFmtCtx) string { f.ordering[i] = GroupID(i + 1) } } else { - f.ordering = m.sortGroups(m.root.group) + f.ordering = m.sortGroups(m.RootGroup()) } // We renumber the groups so that they're still printed in order from 1..N. @@ -78,9 +78,9 @@ func (m *Memo) format(f *memoFmtCtx) string { // If showing raw memo, then add header text to point to root expression if // it's available. - if f.flags.HasFlags(FmtRaw) && m.isOptimized() { - ev := m.Root() - return fmt.Sprintf("root: G%d, %s\n%s", f.numbering[ev.Group()], ev.Physical(), tp.String()) + if f.flags.HasFlags(FmtRaw) { + rootProps := m.LookupPhysicalProps(m.RootProps()) + return fmt.Sprintf("root: G%d, %s\n%s", f.numbering[m.RootGroup()], rootProps, tp.String()) } return tp.String() } @@ -168,7 +168,7 @@ func (m *Memo) formatBestExprSet(f *memoFmtCtx, tp treeprinter.Node, mgrp *group sort.Slice(beSort, func(i, j int) bool { // Always order the root required properties first. - if mgrp.id == m.root.group { + if mgrp.id == m.RootGroup() { if beSort[i].required == beSort[i].best.required { return true } diff --git a/pkg/sql/opt/optbuilder/builder.go b/pkg/sql/opt/optbuilder/builder.go index 44d96e6097b2..9a42de7cdff3 100644 --- a/pkg/sql/opt/optbuilder/builder.go +++ b/pkg/sql/opt/optbuilder/builder.go @@ -18,9 +18,7 @@ import ( "context" "github.com/cockroachdb/cockroach/pkg/sql/opt" - "github.com/cockroachdb/cockroach/pkg/sql/opt/memo" "github.com/cockroachdb/cockroach/pkg/sql/opt/norm" - "github.com/cockroachdb/cockroach/pkg/sql/opt/props" "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror" "github.com/cockroachdb/cockroach/pkg/sql/sem/transform" "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" @@ -31,10 +29,13 @@ import ( // process. As part of the build process, it performs name resolution and // type checking on the expressions within Builder.stmt. // -// The memo structure is the primary data structure used for query -// optimization, so building the memo is the first step required to -// optimize a query. The memo is maintained inside Builder.factory, -// which exposes methods to construct expression groups inside the memo. +// The memo structure is the primary data structure used for query optimization, +// so building the memo is the first step required to optimize a query. The memo +// is maintained inside Builder.factory, which exposes methods to construct +// expression groups inside the memo. Once the expression tree has been built, +// the builder calls SetRoot on the memo to indicate the root memo group, as +// well as the set of physical properties (e.g., row and column ordering) that +// at least one expression in the root group must satisfy. // // A memo is essentially a compact representation of a forest of logically- // equivalent query trees. Each tree is either a logical or a physical plan @@ -116,12 +117,9 @@ func New( // Builder.factory from the parsed SQL statement in Builder.stmt. See the // comment above the Builder type declaration for details. // -// The first return value `root` is the group ID of the root memo group. -// The second return value `required` is the set of physical properties -// (e.g., row and column ordering) that are required of the root memo group. // If any subroutines panic with a builderError as part of the build process, // the panic is caught here and returned as an error. -func (b *Builder) Build() (root memo.GroupID, required *props.Physical, err error) { +func (b *Builder) Build() (err error) { defer func() { if r := recover(); r != nil { // This code allows us to propagate builder errors without adding @@ -136,9 +134,12 @@ func (b *Builder) Build() (root memo.GroupID, required *props.Physical, err erro } }() + // Build the memo, and call SetRoot on the memo to indicate the root group + // and physical properties. outScope := b.buildStmt(b.stmt, b.allocScope()) - physicalProps := outScope.makePhysicalProps() - return outScope.group, &physicalProps, nil + physical := b.factory.Memo().InternPhysicalProps(outScope.makePhysicalProps()) + b.factory.Memo().SetRoot(outScope.group, physical) + return nil } // builderError is used for semantic errors that occur during the build process diff --git a/pkg/sql/opt/optbuilder/builder_test.go b/pkg/sql/opt/optbuilder/builder_test.go index fc8de7c7b562..93d44c5de7d7 100644 --- a/pkg/sql/opt/optbuilder/builder_test.go +++ b/pkg/sql/opt/optbuilder/builder_test.go @@ -23,7 +23,6 @@ import ( "github.com/cockroachdb/cockroach/pkg/settings/cluster" "github.com/cockroachdb/cockroach/pkg/sql/opt/memo" "github.com/cockroachdb/cockroach/pkg/sql/opt/optbuilder" - "github.com/cockroachdb/cockroach/pkg/sql/opt/props" "github.com/cockroachdb/cockroach/pkg/sql/opt/testutils" "github.com/cockroachdb/cockroach/pkg/sql/opt/testutils/testcat" "github.com/cockroachdb/cockroach/pkg/sql/opt/xform" @@ -108,11 +107,11 @@ func TestBuilder(t *testing.T) { o.DisableOptimizations() b := optbuilder.NewScalar(ctx, &semaCtx, &evalCtx, o.Factory()) b.AllowUnsupportedExpr = tester.Flags.AllowUnsupportedExpr - group, err := b.Build(typedExpr) + err = b.Build(typedExpr) if err != nil { return fmt.Sprintf("error: %s\n", strings.TrimSpace(err.Error())) } - exprView := o.Optimize(group, &props.Physical{}) + exprView := o.Optimize() return exprView.FormatString(tester.Flags.ExprFormat) default: diff --git a/pkg/sql/opt/optbuilder/explain.go b/pkg/sql/opt/optbuilder/explain.go index fd5daf636fe2..86b65fd82dc2 100644 --- a/pkg/sql/opt/optbuilder/explain.go +++ b/pkg/sql/opt/optbuilder/explain.go @@ -56,7 +56,7 @@ func (b *Builder) buildExplain(explain *tree.Explain, inScope *scope) (outScope def := memo.ExplainOpDef{ Options: opts, ColList: colsToColList(outScope.cols), - Props: stmtScope.makePhysicalProps(), + Props: *stmtScope.makePhysicalProps(), } outScope.group = b.factory.ConstructExplain(stmtScope.group, b.factory.InternExplainOpDef(&def)) return outScope diff --git a/pkg/sql/opt/optbuilder/scalar.go b/pkg/sql/opt/optbuilder/scalar.go index 7e78a46d757e..8b18d6c5bf92 100644 --- a/pkg/sql/opt/optbuilder/scalar.go +++ b/pkg/sql/opt/optbuilder/scalar.go @@ -668,7 +668,7 @@ func NewScalar( // Build a memo structure from a TypedExpr: the root group represents a scalar // expression equivalent to expr. -func (sb *ScalarBuilder) Build(expr tree.TypedExpr) (root memo.GroupID, err error) { +func (sb *ScalarBuilder) Build(expr tree.TypedExpr) (err error) { defer func() { if r := recover(); r != nil { // This code allows us to propagate builder errors without adding @@ -683,5 +683,7 @@ func (sb *ScalarBuilder) Build(expr tree.TypedExpr) (root memo.GroupID, err erro } }() - return sb.buildScalar(expr, &sb.scope, nil, nil, nil), nil + group := sb.buildScalar(expr, &sb.scope, nil, nil, nil) + sb.factory.Memo().SetRoot(group, memo.MinPhysPropsID) + return nil } diff --git a/pkg/sql/opt/optbuilder/scope.go b/pkg/sql/opt/optbuilder/scope.go index 91c7a69121cd..530d5bd28c6c 100644 --- a/pkg/sql/opt/optbuilder/scope.go +++ b/pkg/sql/opt/optbuilder/scope.go @@ -203,8 +203,8 @@ func (s *scope) makeOrderingChoice() props.OrderingChoice { // makePhysicalProps constructs physical properties using the columns in the // scope for presentation and s.ordering for required ordering. -func (s *scope) makePhysicalProps() props.Physical { - p := props.Physical{} +func (s *scope) makePhysicalProps() *props.Physical { + p := &props.Physical{} if len(s.cols) > 0 { p.Presentation = make(props.Presentation, 0, len(s.cols)) diff --git a/pkg/sql/opt/testutils/forcing_opt.go b/pkg/sql/opt/testutils/forcing_opt.go index 3524425c2403..a8cbaa44703e 100644 --- a/pkg/sql/opt/testutils/forcing_opt.go +++ b/pkg/sql/opt/testutils/forcing_opt.go @@ -28,9 +28,6 @@ import ( type forcingOptimizer struct { o xform.Optimizer - root memo.GroupID - required *props.Physical - // remaining is the number of "unused" steps remaining. remaining int @@ -111,16 +108,14 @@ func newForcingOptimizer( }, ) - var err error - fo.root, fo.required, err = tester.buildExpr(fo.o.Factory()) - if err != nil { + if err := tester.buildExpr(fo.o.Factory()); err != nil { return nil, err } return fo, nil } func (fo *forcingOptimizer) optimize() memo.ExprView { - return fo.o.Optimize(fo.root, fo.required) + return fo.o.Optimize() } // restrictToExprs sets up the optimizer to restrict the result to only those @@ -135,7 +130,7 @@ func (fo *forcingOptimizer) restrictToExprs( ) { coster := newForcingCoster(fo.o.Coster()) - restrictToGroup(coster, mem, fo.root, group) + restrictToGroup(coster, mem, mem.RootGroup(), group) for e := 0; e < mem.ExprCount(group); e++ { if !exprs.Contains(e) { @@ -151,7 +146,7 @@ func (fo *forcingOptimizer) restrictToExprs( func (fo *forcingOptimizer) restrictToGroup(mem *memo.Memo, group memo.GroupID) { coster := newForcingCoster(fo.o.Coster()) - restrictToGroup(coster, mem, fo.root, group) + restrictToGroup(coster, mem, mem.RootGroup(), group) fo.o.SetCoster(coster) } diff --git a/pkg/sql/opt/testutils/opt_tester.go b/pkg/sql/opt/testutils/opt_tester.go index c6d867285f1a..a2ebd842ba8e 100644 --- a/pkg/sql/opt/testutils/opt_tester.go +++ b/pkg/sql/opt/testutils/opt_tester.go @@ -31,7 +31,6 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/opt/memo" "github.com/cockroachdb/cockroach/pkg/sql/opt/norm" "github.com/cockroachdb/cockroach/pkg/sql/opt/optbuilder" - "github.com/cockroachdb/cockroach/pkg/sql/opt/props" "github.com/cockroachdb/cockroach/pkg/sql/opt/testutils/testcat" "github.com/cockroachdb/cockroach/pkg/sql/opt/xform" "github.com/cockroachdb/cockroach/pkg/sql/parser" @@ -495,11 +494,11 @@ func (ot *OptTester) Optimize() (memo.ExprView, error) { func (ot *OptTester) Memo() (string, error) { var o xform.Optimizer o.Init(&ot.evalCtx) - root, required, err := ot.buildExpr(o.Factory()) + err := ot.buildExpr(o.Factory()) if err != nil { return "", err } - o.Optimize(root, required) + o.Optimize() return o.Memo().FormatString(ot.Flags.MemoFormat), nil } @@ -767,12 +766,10 @@ func (ot *OptTester) ExploreTrace() (string, error) { return ot.builder.String(), nil } -func (ot *OptTester) buildExpr( - factory *norm.Factory, -) (root memo.GroupID, required *props.Physical, _ error) { +func (ot *OptTester) buildExpr(factory *norm.Factory) error { stmt, err := parser.ParseOne(ot.sql) if err != nil { - return 0, nil, err + return err } b := optbuilder.New(ot.ctx, &ot.semaCtx, &ot.evalCtx, ot.catalog, factory, stmt) @@ -790,11 +787,11 @@ func (ot *OptTester) makeOptimizer() *xform.Optimizer { } func (ot *OptTester) optimizeExpr(o *xform.Optimizer) (memo.ExprView, error) { - root, required, err := ot.buildExpr(o.Factory()) + err := ot.buildExpr(o.Factory()) if err != nil { return memo.ExprView{}, err } - return o.Optimize(root, required), nil + return o.Optimize(), nil } func (ot *OptTester) output(format string, args ...interface{}) { diff --git a/pkg/sql/opt/xform/optimizer.go b/pkg/sql/opt/xform/optimizer.go index fcd81b8d1de4..4c99448c8ecd 100644 --- a/pkg/sql/opt/xform/optimizer.go +++ b/pkg/sql/opt/xform/optimizer.go @@ -163,12 +163,12 @@ func (o *Optimizer) Memo() *memo.Memo { // properties at the lowest possible execution cost, but is still logically // equivalent to the given expression. If there is a cost "tie", then any one // of the qualifying lowest cost expressions may be selected by the optimizer. -func (o *Optimizer) Optimize(root memo.GroupID, requiredProps *props.Physical) memo.ExprView { +func (o *Optimizer) Optimize() memo.ExprView { // Optimize the root node according to the properties required of it. - root, requiredProps = o.optimizeRootWithProps(root, requiredProps) + o.optimizeRootWithProps() // Now optimize the entire expression tree. - state := o.optimizeGroup(root, o.mem.InternPhysicalProps(requiredProps)) + state := o.optimizeGroup(o.mem.RootGroup(), o.mem.RootProps()) // Validate the resulting operator. ev := memo.MakeExprView(o.mem, state.best) @@ -182,7 +182,6 @@ func (o *Optimizer) Optimize(root memo.GroupID, requiredProps *props.Physical) m panic(fmt.Sprintf(format, ev.Logical().Relational.OuterCols)) } - o.mem.SetRoot(ev) return ev } @@ -544,13 +543,14 @@ func (o *Optimizer) ensureOptState(group memo.GroupID, required memo.PhysicalPro // optimizeRootWithProps tries to simplify the root operator based on the // properties required of it. This may trigger the creation of a new root and // new properties. -func (o *Optimizer) optimizeRootWithProps( - root memo.GroupID, rootProps *props.Physical, -) (memo.GroupID, *props.Physical) { +func (o *Optimizer) optimizeRootWithProps() { + root := o.mem.RootGroup() + rootProps := o.mem.LookupPhysicalProps(o.mem.RootProps()) + relational := o.mem.GroupProperties(root).Relational if relational == nil { // Only need to optimize relational root operators. - return root, rootProps + return } // [SimplifyRootOrdering] @@ -563,6 +563,7 @@ func (o *Optimizer) optimizeRootWithProps( simplified.Ordering = rootProps.Ordering.Copy() simplified.Ordering.Simplify(&relational.FuncDeps) rootProps = &simplified + o.mem.SetRoot(root, o.mem.InternPhysicalProps(rootProps)) if o.appliedRule != nil { o.appliedRule(opt.SimplifyRootOrdering, root, 0, 0) @@ -580,13 +581,13 @@ func (o *Optimizer) optimizeRootWithProps( if o.f.CustomFuncs().CanPruneCols(root, neededCols) { if o.matchedRule == nil || o.matchedRule(opt.PruneRootCols) { root = o.f.CustomFuncs().PruneCols(root, neededCols) + o.mem.SetRoot(root, o.mem.InternPhysicalProps(rootProps)) + if o.appliedRule != nil { o.appliedRule(opt.PruneRootCols, root, 0, 0) } } } - - return root, rootProps } // optStateKey associates optState with a group that is being optimized with diff --git a/pkg/sql/opt/xform/testdata/rules/scan b/pkg/sql/opt/xform/testdata/rules/scan index b5a28122066b..4f38ac3a31d2 100644 --- a/pkg/sql/opt/xform/testdata/rules/scan +++ b/pkg/sql/opt/xform/testdata/rules/scan @@ -305,7 +305,7 @@ memo (optimized, ~2KB) memo SELECT s, i, f FROM a WHERE s='foo' ORDER BY s DESC, i ---- -memo (optimized, ~7KB) +memo (optimized, ~8KB) ├── G1: (select G2 G3) (scan a@s_idx,cols=(2-4),constrained) (index-join G4 a,cols=(2-4)) │ ├── "[presentation: s:4,i:2,f:3] [ordering: +2 opt(4)]" │ │ ├── best: (sort G1) diff --git a/pkg/sql/opt_index_selection.go b/pkg/sql/opt_index_selection.go index fb058aa9b775..68fa352640ae 100644 --- a/pkg/sql/opt_index_selection.go +++ b/pkg/sql/opt_index_selection.go @@ -141,11 +141,11 @@ func (p *planner) selectIndex( } bld := optbuilder.NewScalar(ctx, &p.semaCtx, p.EvalContext(), optimizer.Factory()) bld.AllowUnsupportedExpr = true - filterGroup, err := bld.Build(s.filter) + err := bld.Build(s.filter) if err != nil { return nil, err } - filterExpr := memo.MakeNormExprView(optimizer.Memo(), filterGroup) + filterExpr := optimizer.Memo().Root() for _, c := range candidates { if err := c.makeIndexConstraints( &optimizer, filterExpr, p.EvalContext(), diff --git a/pkg/sql/opt_index_selection_test.go b/pkg/sql/opt_index_selection_test.go index 8e47ca87603f..54bcb485d88a 100644 --- a/pkg/sql/opt_index_selection_test.go +++ b/pkg/sql/opt_index_selection_test.go @@ -24,7 +24,6 @@ import ( "github.com/cockroachdb/cockroach/pkg/roachpb" "github.com/cockroachdb/cockroach/pkg/settings/cluster" "github.com/cockroachdb/cockroach/pkg/sql/opt/constraint" - "github.com/cockroachdb/cockroach/pkg/sql/opt/memo" "github.com/cockroachdb/cockroach/pkg/sql/opt/optbuilder" "github.com/cockroachdb/cockroach/pkg/sql/opt/xform" "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" @@ -101,11 +100,11 @@ func makeSpans( evalCtx := tree.MakeTestingEvalContext(cluster.MakeTestingClusterSettings()) bld := optbuilder.NewScalar(context.Background(), &semaCtx, &evalCtx, o.Factory()) bld.AllowUnsupportedExpr = true - filterGroup, err := bld.Build(expr) + err := bld.Build(expr) if err != nil { t.Fatal(err) } - filterExpr := memo.MakeNormExprView(o.Memo(), filterGroup) + filterExpr := o.Memo().Root() err = c.makeIndexConstraints(&o, filterExpr, p.EvalContext()) if err != nil { t.Fatal(err) diff --git a/pkg/sql/plan.go b/pkg/sql/plan.go index 7b675e90710e..46241a05ecfd 100644 --- a/pkg/sql/plan.go +++ b/pkg/sql/plan.go @@ -375,23 +375,21 @@ func (p *planner) makeOptimizerPlan(ctx context.Context, stmt Statement) error { p.optimizer.Init(p.EvalContext()) f := p.optimizer.Factory() bld := optbuilder.New(ctx, &p.semaCtx, p.EvalContext(), &catalog, f, stmt.AST) - root, props, err := bld.Build() - - // Remember whether the plan was correlated before processing the - // error. This way, the executor will abort early with a useful error message instead of trying - // the heuristic planner. - p.curPlan.isCorrelated = bld.IsCorrelated - + err := bld.Build() if err != nil { + // isCorrelated is used in the fallback case to create a better error. + p.curPlan.isCorrelated = bld.IsCorrelated return err } // If in the PREPARE phase, construct a dummy plan that has correct output // columns. Only output columns and placeholder types are needed. if p.extendedEvalCtx.PrepareOnly { - md := p.optimizer.Memo().Metadata() - resultCols := make(sqlbase.ResultColumns, len(props.Presentation)) - for i, col := range props.Presentation { + mem := f.Memo() + md := mem.Metadata() + physical := mem.LookupPhysicalProps(mem.RootProps()) + resultCols := make(sqlbase.ResultColumns, len(physical.Presentation)) + for i, col := range physical.Presentation { resultCols[i].Name = col.Label resultCols[i].Typ = md.ColumnType(col.ID) } @@ -399,19 +397,17 @@ func (p *planner) makeOptimizerPlan(ctx context.Context, stmt Statement) error { return nil } - ev := p.optimizer.Optimize(root, props) + ev := p.optimizer.Optimize() - factory := makeExecFactory(p) - plan, err := execbuilder.New(&factory, ev).Build() + execFactory := makeExecFactory(p) + plan, err := execbuilder.New(&execFactory, ev).Build() if err != nil { return err } p.curPlan = *plan.(*planTop) - // Since the assignment above just cleared the AST and isCorrelated - // field, we need to set them again. + // Since the assignment above just cleared the AST, we need to set it again. p.curPlan.AST = stmt.AST - p.curPlan.isCorrelated = bld.IsCorrelated cols := planColumns(p.curPlan.plan) if stmt.ExpectedTypes != nil { diff --git a/pkg/sql/sem/tree/eval_test.go b/pkg/sql/sem/tree/eval_test.go index 5cb1e65b5994..849d6bbaa87d 100644 --- a/pkg/sql/sem/tree/eval_test.go +++ b/pkg/sql/sem/tree/eval_test.go @@ -24,7 +24,6 @@ import ( "github.com/cockroachdb/cockroach/pkg/settings/cluster" "github.com/cockroachdb/cockroach/pkg/sql/opt/exec/execbuilder" "github.com/cockroachdb/cockroach/pkg/sql/opt/optbuilder" - "github.com/cockroachdb/cockroach/pkg/sql/opt/props" "github.com/cockroachdb/cockroach/pkg/sql/opt/xform" "github.com/cockroachdb/cockroach/pkg/sql/parser" _ "github.com/cockroachdb/cockroach/pkg/sql/sem/builtins" @@ -1488,11 +1487,10 @@ func optBuildScalar(evalCtx *tree.EvalContext, e tree.TypedExpr) (tree.TypedExpr o.Init(evalCtx) b := optbuilder.NewScalar(context.TODO(), &tree.SemaContext{}, evalCtx, o.Factory()) b.AllowUnsupportedExpr = true - group, err := b.Build(e) - if err != nil { + if err := b.Build(e); err != nil { return nil, err } - ev := o.Optimize(group, &props.Physical{}) + ev := o.Optimize() bld := execbuilder.New(nil /* factory */, ev) ivh := tree.MakeIndexedVarHelper(nil /* container */, 0)