Merge #153423

153423: opt: decouple LOS costing from optimizer r=mgartner a=mgartner

In #93377 the coster and the optimizer were coupled together when cost
scaling for the remote branch of a locality optimized search was
hard-coded into the optimizer. This coupling blurs the responsibilities
of the optimizer and coster and defies their design. `Coster` mentions
that it "is an interface so that different costing algorithms can be
used by the optimizer".

To fix this, a `RemoteBranch` field has been added to physical
properties and the coster uses this field to perform cost scaling. There
should be no difference in end-user behavior due to this change.

Release note: None


Co-authored-by: Marcus Gartner <[email protected]>

Author: craig[bot]

pkg/sql/opt/exec/execbuilder/testdata/explain_redact

@@ -3546,12 +3546,12 @@ memo (optimized, ~8KB, required=[presentation: info:9] [distribution: test])
  │    ├── [ordering: +8 opt(2)]
  │    │    ├── best: (sort G3)
  │    │    └── cost: 1398.52
+ │    ├── [ordering: +8 opt(2)] [distribution: test] [limit hint: 3.00]
+ │    │    ├── best: (distribute G3="[ordering: +8 opt(2)]")
+ │    │    └── cost: 1598.54
  │    ├── [ordering: +8 opt(2)] [limit hint: 3.00]
  │    │    ├── best: (sort G3)
  │    │    └── cost: 1398.52
- │    ├── [ordering: +8 opt(2)] [limit hint: 3.00] [distribution: test]
- │    │    ├── best: (distribute G3="[ordering: +8 opt(2)]")
- │    │    └── cost: 1598.54
  │    └── []
  │         ├── best: (project G5 G6 c d)
  │         └── cost: 1149.04

pkg/sql/opt/memo/interner.go

@@ -644,8 +644,9 @@ func (h *hasher) HashPhysProps(val *physical.Required) {
 		}
 	}
 	h.HashOrderingChoice(val.Ordering)
-	h.HashFloat64(val.LimitHint)
 	h.HashDistribution(val.Distribution)
+	h.HashFloat64(val.LimitHint)
+	h.HashBool(val.RemoteBranch)
 }
 
 func (h *hasher) HashDistribution(val physical.Distribution) {

pkg/sql/opt/memo/interner_test.go

@@ -963,6 +963,16 @@ func TestInternerPhysProps(t *testing.T) {
 		LimitHint:    1,
 		Distribution: physical.Distribution{Regions: []string{"us-east", "us-west"}},
 	}
+	physProps10 := physical.Required{
+		Presentation: physical.Presentation{{Alias: "c", ID: 1}},
+		Ordering:     props.ParseOrderingChoice("+(1|2),+3 opt(4,5)"),
+		RemoteBranch: true,
+	}
+	physProps11 := physical.Required{
+		Presentation: physical.Presentation{{Alias: "c", ID: 1}},
+		Ordering:     props.ParseOrderingChoice("+(1|2),+3 opt(4,5)"),
+		RemoteBranch: true,
+	}
 
 	testCases := []struct {
 		phys    *physical.Required
@@ -978,6 +988,8 @@ func TestInternerPhysProps(t *testing.T) {
 		{phys: &physProps7, inCache: false},
 		{phys: &physProps8, inCache: false},
 		{phys: &physProps9, inCache: true},
+		{phys: &physProps10, inCache: false},
+		{phys: &physProps11, inCache: true},
 	}
 
 	inCache := make(map[*physical.Required]bool)

pkg/sql/opt/props/physical/required.go

@@ -41,6 +41,14 @@ type Required struct {
 	// is required or provided.
 	Ordering props.OrderingChoice
 
+	// Distribution specifies the physical distribution of result rows. This is
+	// defined as the set of regions that may contain result rows. If
+	// Distribution is not defined, then no particular distribution is required.
+	// Currently, the only operator in a plan tree that has a required
+	// distribution is the root, since data must always be returned to the gateway
+	// region.
+	Distribution Distribution
+
 	// LimitHint specifies a "soft limit" to the number of result rows that may
 	// be required of the expression. If requested, an expression will still need
 	// to return all result rows, but it can be optimized based on the assumption
@@ -50,13 +58,10 @@ type Required struct {
 	// using LimitHintInt64.
 	LimitHint float64
 
-	// Distribution specifies the physical distribution of result rows. This is
-	// defined as the set of regions that may contain result rows. If
-	// Distribution is not defined, then no particular distribution is required.
-	// Currently, the only operator in a plan tree that has a required
-	// distribution is the root, since data must always be returned to the gateway
-	// region.
-	Distribution Distribution
+	// RemoteBranch signals that the expression is on the remote side of a
+	// locality-optimized search. If the local branch fulfills the query, the
+	// remote branch is not executed.
+	RemoteBranch bool
 }
 
 // MinRequired are the default physical properties that require nothing and
@@ -66,7 +71,8 @@ var MinRequired = &Required{}
 // Defined is true if any physical property is defined. If none is defined, then
 // this is an instance of MinRequired.
 func (p *Required) Defined() bool {
-	return !p.Presentation.Any() || !p.Ordering.Any() || p.LimitHint != 0 || !p.Distribution.Any()
+	return !p.Presentation.Any() || !p.Ordering.Any() || !p.Distribution.Any() ||
+		p.LimitHint != 0 || p.RemoteBranch
 }
 
 // ColSet returns the set of columns used by any of the physical properties.
@@ -97,11 +103,14 @@ func (p *Required) String() string {
 	if !p.Ordering.Any() {
 		output("ordering", p.Ordering.Format)
 	}
+	if !p.Distribution.Any() {
+		output("distribution", p.Distribution.format)
+	}
 	if p.LimitHint != 0 {
 		output("limit hint", func(buf *bytes.Buffer) { fmt.Fprintf(buf, "%.2f", p.LimitHint) })
 	}
-	if !p.Distribution.Any() {
-		output("distribution", p.Distribution.format)
+	if p.RemoteBranch {
+		output("remote branch", func(buf *bytes.Buffer) { buf.WriteString("true") })
 	}
 
 	// Handle empty properties case.
@@ -114,7 +123,8 @@ func (p *Required) String() string {
 // Equals returns true if the two physical properties are identical.
 func (p *Required) Equals(rhs *Required) bool {
 	return p.Presentation.Equals(rhs.Presentation) && p.Ordering.Equals(&rhs.Ordering) &&
-		p.LimitHint == rhs.LimitHint && p.Distribution.Equals(rhs.Distribution)
+		p.Distribution.Equals(rhs.Distribution) &&
+		p.LimitHint == rhs.LimitHint && p.RemoteBranch == rhs.RemoteBranch
 }
 
 // LimitHintInt64 returns the limit hint converted to an int64.

pkg/sql/opt/xform/coster.go

@@ -625,6 +625,18 @@ func (c *coster) ComputeCost(candidate memo.RelExpr, required *physical.Required
 	// preferable, all else being equal.
 	cost.C += cpuCostFactor
 
+	// Within a locality optimized search, distribution costs are added to the
+	// remote branch, but not the local branch. Scale the remote branch costs by
+	// a factor reflecting the likelihood of executing that branch. Right now
+	// this probability is not estimated, so just use a default probability of
+	// 1/10.
+	// TODO(msirek): Add an estimation of the probability of executing the
+	// remote branch, e.g., compare the size of the limit hint with the expected
+	// row count of the local branch. Is there a better approach?
+	if required.RemoteBranch {
+		cost.C /= 10
+	}
+
 	// Add a one-time cost for any operator with unbounded cardinality. This
 	// ensures we prefer plans that push limits as far down the tree as possible,
 	// all else being equal.

pkg/sql/opt/xform/optimizer.go

@@ -618,21 +618,7 @@ func (o *Optimizer) optimizeGroupMember(
 			childCost, childOptimized := o.optimizeExpr(member.Child(i), childRequired)
 
 			// Accumulate cost of children.
-			if member.Op() == opt.LocalityOptimizedSearchOp && i > 0 {
-				// If the child ops are locality optimized, distribution costs are added
-				// to the remote branch, but not the local branch. Scale the remote
-				// branch costs by a factor reflecting the likelihood of executing that
-				// branch. Right now this probability is not estimated, so just use a
-				// default probability of 1/10.
-				// TODO(msirek): Add an estimation of the probability of executing the
-				//               remote branch, e.g., compare the size of the limit hint
-				//               with the expected row count of the local branch.
-				//               Is there a better approach?
-				childCost.C /= 10
-				cost.Add(childCost)
-			} else {
-				cost.Add(childCost)
-			}
+			cost.Add(childCost)
 
 			// If any child expression is not fully optimized, then the parent
 			// expression is also not fully optimized.

pkg/sql/opt/xform/physical_props.go

@@ -89,6 +89,7 @@ func BuildChildPhysicalProps(
 
 	childProps.Ordering = ordering.BuildChildRequired(mem, parent, &parentProps.Ordering, nth)
 	childProps.Distribution = distribution.BuildChildRequired(parent, &parentProps.Distribution, nth)
+	childProps.RemoteBranch = parentProps.RemoteBranch
 
 	switch parent.Op() {
 	case opt.LimitOp:
@@ -114,11 +115,17 @@ func BuildChildPhysicalProps(
 		childProps.LimitHint = parentProps.LimitHint
 
 	case opt.ExceptOp, opt.ExceptAllOp, opt.IntersectOp, opt.IntersectAllOp,
-		opt.UnionOp, opt.UnionAllOp, opt.LocalityOptimizedSearchOp:
+		opt.UnionOp, opt.UnionAllOp:
 		// TODO(celine): Set operation limits need further thought; for example,
 		// the right child of an ExceptOp should not be limited.
 		childProps.LimitHint = parentProps.LimitHint
 
+	case opt.LocalityOptimizedSearchOp:
+		childProps.LimitHint = parentProps.LimitHint
+		if nth == 1 {
+			childProps.RemoteBranch = true
+		}
+
 	case opt.DistinctOnOp:
 		distinctCount := parent.Relational().Statistics().RowCount
 		if parentProps.LimitHint > 0 {

pkg/sql/opt/xform/testdata/coster/zone

@@ -873,7 +873,7 @@ locality-optimized-search
       ├── constraint: /13/15/16: [/'west'/1/'foo' - /'west'/1/'foo']
       ├── cardinality: [0 - 1]
       ├── stats: [rows=1, distinct(13)=1, null(13)=0, distinct(15)=1, null(15)=0, distinct(16)=1, null(16)=0, distinct(13,15,16)=1, null(13,15,16)=0]
-      ├── cost: 5.17
+      ├── cost: 0.517
       ├── key: ()
       ├── fd: ()-->(13-16)
       └── prune: (13-16)
@@ -931,7 +931,7 @@ anti-join (lookup abc_part@bc_idx [as=a2])
  │    │         ├── constraint: /20/22/23: [/'west'/1/'foo' - /'west'/1/'foo']
  │    │         ├── cardinality: [0 - 1]
  │    │         ├── stats: [rows=1, distinct(20)=1, null(20)=0, distinct(22)=1, null(22)=0, distinct(23)=1, null(23)=0, distinct(20,22,23)=1, null(20,22,23)=0]
- │    │         ├── cost: 5.17
+ │    │         ├── cost: 0.517
  │    │         ├── key: ()
  │    │         └── fd: ()-->(20-23)
  │    └── filters (true)