opt: fix incorrect filter elimination in join reordering

A bug in join reordering which incorrectly eliminated filters has been
fixed. The join reordering logic could sometimes eliminate filters of
the form `a=a` when constructing new inner-joins. It incorrectly assumed
these filters to be redundant because a column is always equivalent to
itself. However, `a=a` is a null-rejecting filter, so it can only be
omitted if `a` is guaranteed to be non-null by something else, like
another filter or a `NOT NULL` constraint.

There is no release note because, as far as we know, this bug is not
possible to hit in production. The only reproduction we have requires
optimization rules to be disabled, which can only happen in tests.

Fixes #146507

Release note: None

Author: mgartner

pkg/sql/opt/props/equiv_set.go

@@ -89,18 +89,20 @@ func (eq *EquivGroups) ContainsCol(col opt.ColumnID) bool {
 	return false
 }
 
-// AreColsEquiv indicates whether the given columns are equivalent.
-func (eq *EquivGroups) AreColsEquiv(left, right opt.ColumnID) bool {
+// AreColsEquiv indicates whether the given columns are equivalent. If "a" and
+// "b" are the same column, true is always returned, even for an empty
+// EquivGroups.
+func (eq *EquivGroups) AreColsEquiv(a, b opt.ColumnID) bool {
 	if buildutil.CrdbTestBuild {
 		defer eq.verify()
 	}
-	if left == right {
+	if a == b {
 		return true
 	}
 	for i := range eq.groups {
-		containsLeft, containsRight := eq.groups[i].Contains(left), eq.groups[i].Contains(right)
-		if containsLeft || containsRight {
-			return containsLeft && containsRight
+		containsA, containsB := eq.groups[i].Contains(a), eq.groups[i].Contains(b)
+		if containsA || containsB {
+			return containsA && containsB
 		}
 	}
 	return false

pkg/sql/opt/xform/join_order_builder.go

@@ -620,6 +620,9 @@ func (jb *JoinOrderBuilder) addJoins(s1, s2 vertexSet) {
 	jb.equivs.AddFromFDs(&jb.plans[s1].Relational().FuncDeps)
 	jb.equivs.AddFromFDs(&jb.plans[s2].Relational().FuncDeps)
 
+	notNullCols := jb.plans[s1].Relational().NotNullCols.Copy()
+	notNullCols.UnionWith(jb.plans[s2].Relational().NotNullCols)
+
 	// Gather all inner edges that connect the left and right relation sets.
 	var innerJoinFilters memo.FiltersExpr
 	for i, ok := jb.innerEdges.Next(0); ok; i, ok = jb.innerEdges.Next(i + 1) {
@@ -631,7 +634,11 @@ func (jb *JoinOrderBuilder) addJoins(s1, s2 vertexSet) {
 			// Record this edge as applied even if it's redundant, since redundant
 			// edges are trivially applied.
 			appliedEdges.Add(i)
-			if areFiltersRedundant(&jb.equivs, e.filters) {
+			redundant := areFiltersRedundant(&jb.equivs, e.filters, notNullCols)
+			// Update notNullCols based on null-rejecting filters to aid in
+			// finding subsequent redundant filters.
+			notNullCols.UnionWith(memo.NullColsRejectedByFilter(jb.ctx, jb.evalCtx, e.filters))
+			if redundant {
 				// Avoid adding redundant filters.
 				continue
 			}
@@ -887,7 +894,11 @@ func (jb *JoinOrderBuilder) addJoin(
 
 // areFiltersRedundant returns true if the given FiltersExpr contains a single
 // equality filter that is already represented by the given EquivGroups set.
-func areFiltersRedundant(equivs *props.EquivGroups, filters memo.FiltersExpr) bool {
+// notNullCols is the set of columns that are known to be non-null, either from
+// the logical properties of the join inputs, or from other filters.
+func areFiltersRedundant(
+	equivs *props.EquivGroups, filters memo.FiltersExpr, notNullCols opt.ColSet,
+) bool {
 	if len(filters) != 1 {
 		return false
 	}
@@ -900,6 +911,13 @@ func areFiltersRedundant(equivs *props.EquivGroups, filters memo.FiltersExpr) bo
 	if !ok1 || !ok2 {
 		return false
 	}
+	if !notNullCols.Contains(var1.Col) || !notNullCols.Contains(var2.Col) {
+		// An equality between columns is not redundant if either column is
+		// nullable because equality is null-rejecting. equivs allows for NULL
+		// values of equivalent columns, so we must check for nullable columns
+		// before checking for equivalence.
+		return false
+	}
 	return equivs.AreColsEquiv(var1.Col, var2.Col)
 }
 

pkg/sql/opt/xform/testdata/rules/join_order

@@ -3558,7 +3558,7 @@ right-join (hash)
 
 # Only 2 joins are considered (instead of 8) when the STRAIGHT hint is present in one join.
 reorderjoins format=hide-all
-SELECT * 
+SELECT *
 FROM straight_join1
 INNER STRAIGHT JOIN straight_join2 ON straight_join1.x = straight_join2.y
 INNER JOIN straight_join3 ON straight_join1.x = straight_join3.z
@@ -3608,7 +3608,7 @@ inner-join (hash)
 
 # No joins are considered when the STRAIGHT hint is present in both joins.
 reorderjoins format=hide-all
-SELECT * 
+SELECT *
 FROM straight_join1
 INNER STRAIGHT JOIN straight_join2 ON straight_join1.x = straight_join2.y
 INNER STRAIGHT JOIN straight_join3 ON straight_join1.x = straight_join3.z
@@ -3628,3 +3628,50 @@ inner-join (hash)
  ├── scan straight_join3
  └── filters
       └── straight_join1.x = z
+
+exec-ddl
+CREATE TABLE t146507 (
+  a INT,
+  b INT
+)
+----
+
+# Regression test for #146507. Join reordering must not eliminate the
+# null-rejecting filter t.b:2 = t.b:2. There should be no inner-joins with empty
+# filters.
+memo disable=(SimplifySameVarEqualities,PushFilterIntoJoinLeft,HoistUnboundFilterFromExistsSubquery)
+SELECT a FROM t146507 AS t WHERE b IN (SELECT t.b FROM t146507 AS t2)
+----
+memo (optimized, ~23KB, required=[presentation: a:1])
+ ├── G1: (project G2 G3 a)
+ │    └── [presentation: a:1]
+ │         ├── best: (project G2 G3 a)
+ │         └── cost: 1130.15
+ ├── G2: (semi-join G4 G5 G6) (project G7 G3 a b)
+ │    └── []
+ │         ├── best: (project G7 G3 a b)
+ │         └── cost: 1120.13
+ ├── G3: (projections)
+ ├── G4: (scan t146507 [as=t],cols=(1,2))
+ │    └── []
+ │         ├── best: (scan t146507 [as=t],cols=(1,2))
+ │         └── cost: 1078.52
+ ├── G5: (scan t146507 [as=t2],cols=())
+ │    ├── [limit hint: 1.00]
+ │    │    ├── best: (scan t146507 [as=t2],cols=())
+ │    │    └── cost: 19.05
+ │    └── []
+ │         ├── best: (scan t146507 [as=t2],cols=())
+ │         └── cost: 1058.32
+ ├── G6: (filters G8)
+ ├── G7: (inner-join G4 G9 G6) (inner-join G9 G4 G6)
+ │    └── []
+ │         ├── best: (inner-join G4 G9 G6)
+ │         └── cost: 1110.11
+ ├── G8: (eq G10 G10)
+ ├── G9: (limit G5 G11) (scan t146507 [as=t2],cols=(),lim=1)
+ │    └── []
+ │         ├── best: (scan t146507 [as=t2],cols=(),lim=1)
+ │         └── cost: 9.04
+ ├── G10: (variable t.b)
+ └── G11: (const 1)