opt: never pick unbounded generic plans over bounded custom plans

The optimizer will no longer choose a generic query plan with unbounded
cardinality over a custom query plan with bounded cardinality,
regardless of `optimizer_prefer_bounded_cardinality`.

In order to implement this behavior, a flag had to be added to
`memo.Cost` that is separate from `UnboundedCardinalityPenalty`. The
penalty is only added to the cost when
`optimizer_prefer_bounded_cardinality` is enabled. The new flag is
unconditionally set, allowing the decision between a generic and custom
plan to work as expected regardless of the session setting. This is a
tad confusing, so I did my best to document the differences clearly.

Fixes #155159

Release note (performance improvement): The optimizer chooses suboptimal
generic query plans in fewer cases.

Author: mgartner

pkg/sql/logictest/testdata/logic_test/generic

@@ -470,3 +470,73 @@ query T match(plan\stype)
 EXPLAIN ANALYZE EXECUTE p(1, 2);
 ----
 plan type: custom
+
+statement ok
+DEALLOCATE p
+
+# Regression test for #155159. Do not choose a generic query plan with unbounded
+# cardinality when the custom plans have bounded cardinality.
+statement ok
+CREATE TABLE t155159 (
+  id INT PRIMARY KEY,
+  a INT,
+  b INT,
+  INDEX (a, b)
+)
+
+statement ok
+SET plan_cache_mode = auto
+
+statement ok
+SET optimizer_prefer_bounded_cardinality = false
+
+statement ok
+PREPARE p AS SELECT id FROM t155159 WHERE a = $1 AND b >= $2 ORDER BY b, id LIMIT 250
+
+statement ok
+EXECUTE p (33, 44)
+
+statement ok
+EXECUTE p (33, 44)
+
+statement ok
+EXECUTE p (33, 44)
+
+statement ok
+EXECUTE p (33, 44)
+
+statement ok
+EXECUTE p (33, 44)
+
+query T
+EXPLAIN ANALYZE EXECUTE p (33, 44)
+----
+planning time: 10µs
+execution time: 100µs
+distribution: <hidden>
+vectorized: <hidden>
+plan type: custom
+maximum memory usage: <hidden>
+DistSQL network usage: <hidden>
+regions: <hidden>
+isolation level: serializable
+priority: normal
+quality of service: regular
+·
+• scan
+  sql nodes: <hidden>
+  kv nodes: <hidden>
+  regions: <hidden>
+  actual row count: 0
+  KV time: 0µs
+  KV rows decoded: 0
+  KV bytes read: 0 B
+  KV gRPC calls: 0
+  estimated max memory allocated: 0 B
+  missing stats
+  table: t155159@t155159_a_b_idx
+  spans: [/33/44 - /33]
+  limit: 250
+
+statement ok
+DEALLOCATE p

pkg/sql/opt/memo/cost.go

@@ -20,6 +20,12 @@ type Cost struct {
 		// fullScanCount is the number of full table or index scans in a
 		// sub-plan, up to 255.
 		fullScanCount uint8
+		// unboundedCardinality is true if the operator or any of its
+		// descendants have no guaranteed upperbound on the number of rows that
+		// they can produce. It is similar to UnboundedCardinalityPenalty, but
+		// different in that it is used to propagate the same information up the
+		// tree without affecting cost comparisons.
+		unboundedCardinality bool
 	}
 }
 
@@ -59,6 +65,7 @@ func (c *Cost) Add(other Cost) {
 	} else {
 		c.aux.fullScanCount += other.aux.fullScanCount
 	}
+	c.aux.unboundedCardinality = c.aux.unboundedCardinality || other.aux.unboundedCardinality
 }
 
 // FullScanCount returns the number of full scans in the cost.
@@ -68,11 +75,29 @@ func (c Cost) FullScanCount() uint8 {
 
 // IncrFullScanCount increments that auxiliary full scan count within c.
 func (c *Cost) IncrFullScanCount() {
-	if c.aux.fullScanCount == math.MaxUint8 {
-		// Avoid overflow.
-		return
+	// Avoid overflow.
+	if c.aux.fullScanCount < math.MaxUint8 {
+		c.aux.fullScanCount++
 	}
-	c.aux.fullScanCount++
+}
+
+// HasUnboundedCardinality returns true if any expression in the tree has no
+// guaranteed upperbound on the number of rows that it will produce.
+//
+// NOTE: The returned value is independent of the UnboundedCardinalityPenalty
+// and true may be returned when the penalty is not set. It has no effect on
+// cost comparisons.
+func (c Cost) HasUnboundedCardinality() bool {
+	return c.aux.unboundedCardinality
+}
+
+// SetUnboundedCardinality is called to indicate that an expression has no
+// guaranteed upperbound on the number of rows that it will produce.
+//
+// NOTE: This flag does not affect cost comparisons and is independent of the
+// UnboundedCardinalityPenalty.
+func (c *Cost) SetUnboundedCardinality() {
+	c.aux.unboundedCardinality = true
 }
 
 // Penalties is an ordered bitmask where each bit indicates a cost penalty. The

pkg/sql/opt/memo/cost_test.go

@@ -8,7 +8,8 @@ package memo
 import "testing"
 
 type testAux struct {
-	fullScanCount uint8
+	fullScanCount        uint8
+	unboundedCardinality bool
 }
 
 func TestCostLess(t *testing.T) {
@@ -38,7 +39,8 @@ func TestCostLess(t *testing.T) {
 		{Cost{C: 2.0}, Cost{C: 1.0, Penalties: UnboundedCardinalityPenalty}, true},
 		{Cost{C: 1.0, Penalties: UnboundedCardinalityPenalty}, Cost{C: 2.0}, false},
 		// Auxiliary information should not affect the comparison.
-		{Cost{C: 1.0, aux: testAux{0}}, Cost{C: 1.0, aux: testAux{1}}, false},
+		{Cost{C: 1.0, aux: testAux{0, false}}, Cost{C: 1.0, aux: testAux{1, true}}, false},
+		{Cost{C: 1.0, aux: testAux{1, true}}, Cost{C: 1.0, aux: testAux{0, false}}, false},
 	}
 	for _, tc := range testCases {
 		if tc.left.Less(tc.right) != tc.expected {
@@ -58,8 +60,8 @@ func TestCostAdd(t *testing.T) {
 		{Cost{C: 1.0, Penalties: FullScanPenalty}, Cost{C: 2.0}, Cost{C: 3.0, Penalties: FullScanPenalty}},
 		{Cost{C: 1.0}, Cost{C: 2.0, Penalties: HugeCostPenalty}, Cost{C: 3.0, Penalties: HugeCostPenalty}},
 		{Cost{C: 1.0, Penalties: UnboundedCardinalityPenalty}, Cost{C: 2.0, Penalties: HugeCostPenalty}, Cost{C: 3.0, Penalties: HugeCostPenalty | UnboundedCardinalityPenalty}},
-		{Cost{C: 1.0, aux: testAux{1}}, Cost{C: 1.0, aux: testAux{2}}, Cost{C: 2.0, aux: testAux{3}}},
-		{Cost{C: 1.0, aux: testAux{200}}, Cost{C: 1.0, aux: testAux{100}}, Cost{C: 2.0, aux: testAux{255}}},
+		{Cost{C: 1.0, aux: testAux{1, false}}, Cost{C: 1.0, aux: testAux{2, true}}, Cost{C: 2.0, aux: testAux{3, true}}},
+		{Cost{C: 1.0, aux: testAux{200, true}}, Cost{C: 1.0, aux: testAux{100, false}}, Cost{C: 2.0, aux: testAux{255, true}}},
 	}
 	for _, tc := range testCases {
 		tc.left.Add(tc.right)

pkg/sql/opt/xform/coster.go

@@ -638,12 +638,14 @@ func (c *coster) ComputeCost(candidate memo.RelExpr, required *physical.Required
 	}
 
 	// 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.
+	// ensures we prefer plans that push limits as far down the tree as
+	// possible, all else being equal.
 	//
-	// Also add a cost flag for unbounded cardinality.
+	// Also add a cost flag for unbounded cardinality, and a penalty if the
+	// corresponding session setting is enabled.
 	if candidate.Relational().Cardinality.IsUnbounded() {
 		cost.C += cpuCostFactor
+		cost.SetUnboundedCardinality()
 		if c.evalCtx.SessionData().OptimizerPreferBoundedCardinality {
 			cost.Penalties |= memo.UnboundedCardinalityPenalty
 		}

pkg/sql/prep/statement.go

@@ -187,9 +187,14 @@ func (p *planCosts) NumCustom() int {
 // average cost of the custom plans.
 func (p *planCosts) IsGenericOptimal() bool {
 	// Check cost flags and full scan counts.
-	if gc := p.generic.FullScanCount(); gc > 0 || p.generic.Penalties != memo.NoPenalties {
+	if gc := p.generic.FullScanCount(); gc > 0 ||
+		p.generic.HasUnboundedCardinality() ||
+		p.generic.Penalties != memo.NoPenalties {
 		for i := 0; i < p.custom.length; i++ {
-			if p.custom.costs[i].Penalties < p.generic.Penalties || gc > p.custom.costs[i].FullScanCount() {
+			custom := &p.custom.costs[i]
+			if custom.Penalties < p.generic.Penalties ||
+				(p.generic.HasUnboundedCardinality() && !custom.HasUnboundedCardinality()) ||
+				gc > custom.FullScanCount() {
 				return false
 			}
 		}