From 77979d987352d47c89d570556643018bb3630273 Mon Sep 17 00:00:00 2001
From: Andrew Kimball <andyk@cockroachlabs.com>
Date: Wed, 5 Sep 2018 19:36:43 -0700
Subject: [PATCH] opt: Store root of normalized tree in Memo in optbuilder

The Memo has always stored the root of the lowest cost expression tree,
once exploration completes. This commit changes optbuilder to store the
root of the normalized tree even before exploration starts. This cleans
up the API and will make it easier to copy the Memo from place to place
during the PREPARE phase and when using a prepared Memo.

Release note: None
---
 pkg/sql/opt/bench/bench_test.go               |  5 +-
 .../idxconstraint/index_constraints_test.go   |  8 +--
 pkg/sql/opt/memo/expr_view.go                 |  4 --
 pkg/sql/opt/memo/expr_view_test.go            |  4 +-
 pkg/sql/opt/memo/memo.go                      | 58 +++++++++++++------
 pkg/sql/opt/memo/memo_format.go               | 12 ++--
 pkg/sql/opt/optbuilder/builder.go             | 25 ++++----
 pkg/sql/opt/optbuilder/builder_test.go        |  5 +-
 pkg/sql/opt/optbuilder/explain.go             |  2 +-
 pkg/sql/opt/optbuilder/scalar.go              |  6 +-
 pkg/sql/opt/optbuilder/scope.go               |  4 +-
 pkg/sql/opt/testutils/forcing_opt.go          | 13 ++---
 pkg/sql/opt/testutils/opt_tester.go           | 15 ++---
 pkg/sql/opt/xform/optimizer.go                | 21 +++----
 pkg/sql/opt/xform/testdata/rules/scan         |  2 +-
 pkg/sql/opt_index_selection.go                |  4 +-
 pkg/sql/opt_index_selection_test.go           |  5 +-
 pkg/sql/plan.go                               | 28 ++++-----
 pkg/sql/sem/tree/eval_test.go                 |  6 +-
 19 files changed, 115 insertions(+), 112 deletions(-)

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 15bfa6ae50a0..946782baa611 100644
--- a/pkg/sql/opt/optbuilder/scalar.go
+++ b/pkg/sql/opt/optbuilder/scalar.go
@@ -667,7 +667,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
@@ -682,5 +682,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)