opt: avoid more types of bad generic query plans

mgartner · mgartner · commit e318d31cef2d · 2025-11-04T15:05:25.000-05:00
This commit extends #155163. Instead of tracking an auxiliary boolean value to indicate that a plan has one or more expressions with unbounded cardinality, the `memo.Cost` struct now tracks the number of read expressions (i.e., expressions that perform KV reads) that have unbounded cardinality. This helps to avoid picking generic query plans that will perform significantly worse than their related custom query plans. Fixes #156690 Release note: None
diff --git a/pkg/sql/logictest/testdata/logic_test/generic b/pkg/sql/logictest/testdata/logic_test/generic
@@ -474,8 +474,9 @@ 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.
+# Regression test for #155159 and #156690. Do not choose a generic query plan
+# with reads that have unbounded cardinality when the reads in the custom plan
+# have bounded cardinality.
 statement ok
 CREATE TABLE t155159 (
   id INT PRIMARY KEY,
@@ -540,3 +541,84 @@ quality of service: regular
 
 statement ok
 DEALLOCATE p
+
+statement ok
+PREPARE p AS
+SELECT * FROM (
+  SELECT id FROM t155159 WHERE a = $1 AND b >= $2
+  ORDER BY b, id
+  LIMIT 250
+)
+UNION
+SELECT * FROM (
+  SELECT id FROM t155159 WHERE a = $3 AND b >= $4
+)
+
+statement ok
+EXECUTE p (33, 44, 55, 66)
+
+statement ok
+EXECUTE p (33, 44, 55, 66)
+
+statement ok
+EXECUTE p (33, 44, 55, 66)
+
+statement ok
+EXECUTE p (33, 44, 55, 66)
+
+statement ok
+EXECUTE p (33, 44, 55, 66)
+
+query T
+EXPLAIN ANALYZE EXECUTE p (33, 44, 55, 66);
+----
+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
+·
+• union
+│ sql nodes: <hidden>
+│ regions: <hidden>
+│ actual row count: 0
+│ execution time: 0µs
+│ estimated max memory allocated: 0 B
+│
+├── • 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
+│
+└── • 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: [/55/66 - /55]
+
+statement ok
+DEALLOCATE p
diff --git a/pkg/sql/opt/memo/cost.go b/pkg/sql/opt/memo/cost.go
@@ -22,14 +22,12 @@ type Cost struct {
 	// cost is compared to other costs with Less.
 	aux 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
+		// sub-plan, up to 65535.
+		fullScanCount uint16
+		// unboundedReadCount is the number of read expressions (e.g., scans,
+		// lookup joins, etc.) in a sub-plan that have no upper-bound
+		// cardinality, up to 65535.
+		unboundedReadCount uint16
 	}
 }
 
@@ -63,45 +61,28 @@ func (c Cost) Less(other Cost) bool {
 func (c *Cost) Add(other Cost) {
 	c.C += other.C
 	c.Penalties |= other.Penalties
-	if c.aux.fullScanCount > math.MaxUint8-other.aux.fullScanCount {
-		// Avoid overflow.
-		c.aux.fullScanCount = math.MaxUint8
-	} else {
-		c.aux.fullScanCount += other.aux.fullScanCount
-	}
-	c.aux.unboundedCardinality = c.aux.unboundedCardinality || other.aux.unboundedCardinality
+	c.aux.fullScanCount = addUint16(c.aux.fullScanCount, other.aux.fullScanCount)
+	c.aux.unboundedReadCount = addUint16(c.aux.unboundedReadCount, other.aux.unboundedReadCount)
 }
 
 // FullScanCount returns the number of full scans in the cost.
-func (c Cost) FullScanCount() uint8 {
+func (c Cost) FullScanCount() uint16 {
 	return c.aux.fullScanCount
 }
 
 // IncrFullScanCount increments that auxiliary full scan count within c.
 func (c *Cost) IncrFullScanCount() {
-	// Avoid overflow.
-	if c.aux.fullScanCount < math.MaxUint8 {
-		c.aux.fullScanCount++
-	}
+	c.aux.fullScanCount = addUint16(c.aux.fullScanCount, 1)
 }
 
-// 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
+// UnboundedReadCount returns the number of full scans in the cost.
+func (c Cost) UnboundedReadCount() uint16 {
+	return c.aux.unboundedReadCount
 }
 
-// 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
+// IncrUnboundedReadCount increments that auxiliary full scan count within c.
+func (c *Cost) IncrUnboundedReadCount() {
+	c.aux.unboundedReadCount = addUint16(c.aux.unboundedReadCount, 1)
 }
 
 // Penalties is an ordered bitmask where each bit indicates a cost penalty. The
@@ -142,12 +123,11 @@ const (
 //	<Cost> is the floating point cost value.
 //	<Penalties> contains "H", "F", or "U" for HugeCostPenalty, FullScanPenalty,
 //	  and UnboundedCardinalityPenalty, respectively.
-//	<aux> contains a number for full scan count and "u" for
-//	  unboundedCardinality.
+//	<aux> contains the number of full scans and unbounded reads.
 //
-// For example, the summary "1.23:HF:5fu" indicates a cost of 1.23 with the
-// HugeCostPenalty and FullScanPenalty penalties, 5 full scans, and the
-// unboundedCardinality flag set.
+// For example, the summary "1.23:HF:5f6u" indicates a cost of 1.23 with the
+// HugeCostPenalty and FullScanPenalty penalties, 5 full scans, and 6 unbounded
+// reads.
 func (c Cost) Summary() string {
 	var sb strings.Builder
 	_, _ = fmt.Fprintf(&sb, "%.9g:", c.C)
@@ -161,8 +141,13 @@ func (c Cost) Summary() string {
 		sb.WriteByte('U')
 	}
 	_, _ = fmt.Fprintf(&sb, ":%df", c.aux.fullScanCount)
-	if c.aux.unboundedCardinality {
-		sb.WriteByte('u')
-	}
+	_, _ = fmt.Fprintf(&sb, "%du", c.aux.unboundedReadCount)
 	return sb.String()
 }
+
+func addUint16(a, b uint16) uint16 {
+	if a > math.MaxUint16-b {
+		return math.MaxUint16
+	}
+	return a + b
+}
diff --git a/pkg/sql/opt/memo/cost_test.go b/pkg/sql/opt/memo/cost_test.go
@@ -8,8 +8,8 @@ package memo
 import "testing"
 
 type testAux struct {
-	fullScanCount        uint8
-	unboundedCardinality bool
+	fullScanCount      uint16
+	unboundedReadCount uint16
 }
 
 func TestCostLess(t *testing.T) {
@@ -39,8 +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, 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},
+		{Cost{C: 1.0, aux: testAux{0, 0}}, Cost{C: 1.0, aux: testAux{1, 1}}, false},
+		{Cost{C: 1.0, aux: testAux{1, 1}}, Cost{C: 1.0, aux: testAux{0, 0}}, false},
 	}
 	for _, tc := range testCases {
 		if tc.left.Less(tc.right) != tc.expected {
@@ -60,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, 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}}},
+		{Cost{C: 1.0, aux: testAux{1, 4}}, Cost{C: 1.0, aux: testAux{2, 5}}, Cost{C: 2.0, aux: testAux{3, 9}}},
+		{Cost{C: 1.0, aux: testAux{65530, 65530}}, Cost{C: 1.0, aux: testAux{100, 100}}, Cost{C: 2.0, aux: testAux{65535, 65535}}},
 	}
 	for _, tc := range testCases {
 		tc.left.Add(tc.right)
@@ -76,18 +76,18 @@ func TestCostSummary(t *testing.T) {
 		c        Cost
 		expected string
 	}{
-		{Cost{C: 1.0}, "1::0f"},
-		{Cost{C: 1.23}, "1.23::0f"},
-		{Cost{C: 1.23456}, "1.23456::0f"},
-		{Cost{C: 1.23, Penalties: HugeCostPenalty}, "1.23:H:0f"},
-		{Cost{C: 1.23, Penalties: FullScanPenalty}, "1.23:F:0f"},
-		{Cost{C: 1.23, Penalties: UnboundedCardinalityPenalty}, "1.23:U:0f"},
-		{Cost{C: 1.23, Penalties: HugeCostPenalty | FullScanPenalty | UnboundedCardinalityPenalty}, "1.23:HFU:0f"},
-		{Cost{C: 1.23, Penalties: HugeCostPenalty | FullScanPenalty | UnboundedCardinalityPenalty}, "1.23:HFU:0f"},
-		{Cost{C: 1.23, aux: testAux{5, false}}, "1.23::5f"},
-		{Cost{C: 1.23, aux: testAux{0, true}}, "1.23::0fu"},
-		{Cost{C: 1.23, aux: testAux{5, true}}, "1.23::5fu"},
-		{Cost{C: 1.23, Penalties: HugeCostPenalty | FullScanPenalty, aux: testAux{5, true}}, "1.23:HF:5fu"},
+		{Cost{C: 1.0}, "1::0f0u"},
+		{Cost{C: 1.23}, "1.23::0f0u"},
+		{Cost{C: 1.23456}, "1.23456::0f0u"},
+		{Cost{C: 1.23, Penalties: HugeCostPenalty}, "1.23:H:0f0u"},
+		{Cost{C: 1.23, Penalties: FullScanPenalty}, "1.23:F:0f0u"},
+		{Cost{C: 1.23, Penalties: UnboundedCardinalityPenalty}, "1.23:U:0f0u"},
+		{Cost{C: 1.23, Penalties: HugeCostPenalty | FullScanPenalty | UnboundedCardinalityPenalty}, "1.23:HFU:0f0u"},
+		{Cost{C: 1.23, Penalties: HugeCostPenalty | FullScanPenalty | UnboundedCardinalityPenalty}, "1.23:HFU:0f0u"},
+		{Cost{C: 1.23, aux: testAux{5, 0}}, "1.23::5f0u"},
+		{Cost{C: 1.23, aux: testAux{0, 6}}, "1.23::0f6u"},
+		{Cost{C: 1.23, aux: testAux{5, 10}}, "1.23::5f10u"},
+		{Cost{C: 1.23, Penalties: HugeCostPenalty | FullScanPenalty, aux: testAux{5, 9}}, "1.23:HF:5f9u"},
 	}
 	for _, tc := range testCases {
 		if r := tc.c.Summary(); r != tc.expected {
diff --git a/pkg/sql/opt/xform/coster.go b/pkg/sql/opt/xform/coster.go
@@ -641,14 +641,19 @@ func (c *coster) ComputeCost(candidate memo.RelExpr, required *physical.Required
 	// 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, and a penalty if the
-	// corresponding session setting is enabled.
+	// Also add a penalty if the corresponding session setting is enabled and
+	// increment the unbounded read count for expressions that read from an
+	// index.
 	if candidate.Relational().Cardinality.IsUnbounded() {
 		cost.C += cpuCostFactor
-		cost.SetUnboundedCardinality()
 		if c.evalCtx.SessionData().OptimizerPreferBoundedCardinality {
 			cost.Penalties |= memo.UnboundedCardinalityPenalty
 		}
+		switch candidate.Op() {
+		case opt.ScanOp, opt.PlaceholderScanOp, opt.LookupJoinOp, opt.IndexJoinOp,
+			opt.InvertedJoinOp, opt.ZigzagJoinOp, opt.VectorSearchOp:
+			cost.IncrUnboundedReadCount()
+		}
 	}
 
 	if !cost.Less(memo.MaxCost) {
diff --git a/pkg/sql/prep/statement.go b/pkg/sql/prep/statement.go
@@ -187,14 +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.HasUnboundedCardinality() ||
-		p.generic.Penalties != memo.NoPenalties {
+	genFullScans := p.generic.FullScanCount()
+	genUnboundedReads := p.generic.UnboundedReadCount()
+	if genFullScans > 0 || genUnboundedReads > 0 || p.generic.Penalties != memo.NoPenalties {
 		for i := 0; i < p.custom.length; i++ {
 			custom := &p.custom.costs[i]
 			if custom.Penalties < p.generic.Penalties ||
-				(p.generic.HasUnboundedCardinality() && !custom.HasUnboundedCardinality()) ||
-				gc > custom.FullScanCount() {
+				genFullScans > custom.FullScanCount() ||
+				genUnboundedReads > custom.UnboundedReadCount() {
 				return false
 			}
 		}
@@ -229,7 +229,7 @@ func (p *planCosts) avgCustom() memo.Cost {
 //
 // A full example:
 //
-//	custom costs: 1.5 [3]{1.25:U:0u 1.75:U:0u 1.50:U:0u}, generic cost: 4.56:U:0u
+//	custom costs: 1.5 [3]{1.25:U:0f0u 1.75:U:0f0u 1.50:U:0f0u}, generic cost: 4.56:U:0f0u
 func (p *planCosts) Summary() string {
 	var sb strings.Builder
 	sb.WriteString("custom costs: ")
