opt: fix logical properties calculation for locality optimized join

This commit fixes a bug when calculating the logical properties for
a locality optimized lookup join where we would incorrectly determine that
the crdb_region column was constant. In fact, crdb_region is only constant
in the local lookup expr of the join; the remote lookup expr contains
one or more other possible values for this column.

I don't know of any actual incorrect results caused by this issue, so
I'm not setting a release note. However, incorrect logical properties are
always bad and can in theory cause incorrect results.

Epic: None
Release note: None

pkg/sql/opt/lookupjoin/constraint_builder.go

@@ -79,10 +79,10 @@ type Constraint struct {
 	// be projected on the lookup join's input.
 	InputProjections memo.ProjectionsExpr
 
-	// ConstFilters contains constant equalities and ranges in either KeyCols or
-	// LookupExpr that are used to aid selectivity estimation. See
-	// memo.LookupJoinPrivate.ConstFilters.
-	ConstFilters memo.FiltersExpr
+	// AllLookupFilters contains equalities and other filters in either KeyCols or
+	// LookupExpr that are used to aid selectivity estimation and logical props
+	// calculation. See memo.LookupJoinPrivate.AllLookupFilters.
+	AllLookupFilters memo.FiltersExpr
 
 	// RemainingFilters contains explicit ON filters that are not represented by
 	// KeyCols or LookupExpr. These filters must be included as ON filters in
@@ -215,7 +215,7 @@ func (b *ConstraintBuilder) Build(
 	rightSideCols := make(opt.ColList, 0, numIndexKeyCols)
 	var inputProjections memo.ProjectionsExpr
 	var lookupExpr memo.FiltersExpr
-	var constFilters memo.FiltersExpr
+	var allLookupFilters memo.FiltersExpr
 	var filterOrdsToExclude intsets.Fast
 	foundLookupCols := false
 	lookupExprRequired := false
@@ -259,6 +259,7 @@ func (b *ConstraintBuilder) Build(
 		idxCol := b.table.IndexColumnID(index, j)
 		idxColIsDesc := index.Column(j).Descending
 		if eqIdx, ok := rightEq.Find(idxCol); ok {
+			allLookupFilters = append(allLookupFilters, allFilters[eqFilterOrds[eqIdx]])
 			addEqualityColumns(leftEq[eqIdx], idxCol)
 			filterOrdsToExclude.Add(eqFilterOrds[eqIdx])
 			foundEqualityCols = true
@@ -313,7 +314,7 @@ func (b *ConstraintBuilder) Build(
 				b.f.ConstructConstVal(foundVals[0], idxColType),
 				constColID,
 			))
-			constFilters = append(constFilters, allFilters[allIdx])
+			allLookupFilters = append(allLookupFilters, allFilters[allIdx])
 			addEqualityColumns(constColID, idxCol)
 			filterOrdsToExclude.Add(allIdx)
 			continue
@@ -336,7 +337,7 @@ func (b *ConstraintBuilder) Build(
 				})
 			}
 			lookupExpr = append(lookupExpr, valsFilter)
-			constFilters = append(constFilters, valsFilter)
+			allLookupFilters = append(allLookupFilters, allFilters[allIdx])
 			filterOrdsToExclude.Add(allIdx)
 			continue
 		}
@@ -349,12 +350,14 @@ func (b *ConstraintBuilder) Build(
 		if foundStart {
 			convertToLookupExpr()
 			lookupExpr = append(lookupExpr, allFilters[startIdx])
+			allLookupFilters = append(allLookupFilters, allFilters[startIdx])
 			filterOrdsToExclude.Add(startIdx)
 			foundLookupCols = true
 		}
 		if foundEnd {
 			convertToLookupExpr()
 			lookupExpr = append(lookupExpr, allFilters[endIdx])
+			allLookupFilters = append(allLookupFilters, allFilters[endIdx])
 			filterOrdsToExclude.Add(endIdx)
 			foundLookupCols = true
 		}
@@ -376,7 +379,7 @@ func (b *ConstraintBuilder) Build(
 			// A constant range filter could be found.
 			convertToLookupExpr()
 			lookupExpr = append(lookupExpr, *rangeFilter)
-			constFilters = append(constFilters, *rangeFilter)
+			allLookupFilters = append(allLookupFilters, allFilters[filterIdx])
 			filterOrdsToExclude.Add(filterIdx)
 			if remaining != nil {
 				remainingFilters = append(remainingFilters, *remaining)
@@ -405,7 +408,7 @@ func (b *ConstraintBuilder) Build(
 		RightSideCols:    rightSideCols,
 		LookupExpr:       lookupExpr,
 		InputProjections: inputProjections,
-		ConstFilters:     constFilters,
+		AllLookupFilters: allLookupFilters,
 	}
 
 	// Reduce the remaining filters.

pkg/sql/opt/memo/check_expr.go

@@ -224,12 +224,19 @@ func (m *Memo) CheckExpr(e opt.Expr) {
 		if t.Cols.SubsetOf(t.Input.Relational().OutputCols) {
 			panic(errors.AssertionFailedf("lookup join with no lookup columns"))
 		}
+		switch t.JoinType {
+		case opt.AntiJoinOp:
+			if len(t.RemoteLookupExpr) > 0 {
+				panic(errors.AssertionFailedf("anti join with a non-empty RemoteLookupExpr"))
+			}
+		}
 		var requiredCols opt.ColSet
 		requiredCols.UnionWith(t.Relational().OutputCols)
-		requiredCols.UnionWith(t.ConstFilters.OuterCols())
+		requiredCols.UnionWith(t.AllLookupFilters.OuterCols())
 		requiredCols.UnionWith(t.On.OuterCols())
 		requiredCols.UnionWith(t.KeyCols.ToSet())
 		requiredCols.UnionWith(t.LookupExpr.OuterCols())
+		requiredCols.UnionWith(t.RemoteLookupExpr.OuterCols())
 		idx := m.Metadata().Table(t.Table).Index(t.Index)
 		for i := range t.KeyCols {
 			requiredCols.Add(t.Table.ColumnID(idx.Column(i).Ordinal()))

pkg/sql/opt/memo/logical_props_builder.go

@@ -2283,7 +2283,7 @@ func (h *joinPropsHelper) init(b *logicalPropsBuilder, joinExpr RelExpr) {
 		ensureLookupJoinInputProps(join, &b.sb)
 		h.joinType = join.JoinType
 		h.rightProps = &join.lookupProps
-		h.filters = append(join.On, join.LookupExpr...)
+		h.filters = append(join.On, join.AllLookupFilters...)
 		b.addFiltersToFuncDep(h.filters, &h.filtersFD)
 		h.filterNotNullCols = b.rejectNullCols(h.filters)
 

pkg/sql/opt/memo/statistics_builder.go

@@ -3084,14 +3084,12 @@ func (sb *statisticsBuilder) filterRelExpr(
 func (sb *statisticsBuilder) applyFilters(
 	filters FiltersExpr, e RelExpr, relProps *props.Relational, skipOrTermAccounting bool,
 ) (numUnappliedConjuncts float64, constrainedCols, histCols opt.ColSet) {
-	// Special hack for lookup and inverted joins. Add constant filters from the
-	// equality conditions.
+	// Special hack for inverted joins. Add constant filters from the equality
+	// conditions.
 	// TODO(rytaft): the correct way to do this is probably to fully implement
 	// histograms in Project and Join expressions, and use them in
 	// selectivityFromEquivalencies. See Issue #38082.
 	switch t := e.(type) {
-	case *LookupJoinExpr:
-		filters = append(filters, t.ConstFilters...)
 	case *InvertedJoinExpr:
 		filters = append(filters, t.ConstFilters...)
 	}

pkg/sql/opt/ops/relational.opt

@@ -484,10 +484,12 @@ define LookupJoinPrivate {
     # both the input table and lookup table have REGIONAL BY ROW partitioning).
     ChildOfLocalityOptimizedSearch bool
 
-    # ConstFilters contains the constant filters that are represented as equality
-    # conditions on the KeyCols. These filters are needed by the statistics code to
-    # correctly estimate selectivity.
-    ConstFilters FiltersExpr
+    # AllLookupFilters synthesizes all the filters that are represented as
+    # equality conditions on the KeyCols, as well as the filters in LookupExpr
+    # and RemoteLookupExpr. These filters are needed by the statistics code to
+    # correctly estimate selectivity as well as by the logicalPropsBuilder for
+    # calculating logical properties.
+    AllLookupFilters FiltersExpr
 
     # Locking represents the row-level locking mode of the LookupJoin in the
     # lookup table. Most lookup joins leave this unset (Strength = ForNone),

pkg/sql/opt/xform/join_funcs.go

@@ -453,7 +453,7 @@ func (c *CustomFuncs) generateLookupJoinsImpl(
 		lookupJoin.DerivedEquivCols = lookupConstraint.DerivedEquivCols
 		lookupJoin.LookupExpr = lookupConstraint.LookupExpr
 		lookupJoin.On = lookupConstraint.RemainingFilters
-		lookupJoin.ConstFilters = lookupConstraint.ConstFilters
+		lookupJoin.AllLookupFilters = lookupConstraint.AllLookupFilters
 
 		// Wrap the input in a Project if any projections are required. The
 		// lookup join will project away these synthesized columns.
@@ -769,8 +769,8 @@ func (c *CustomFuncs) mapLookupJoin(
 		lookupJoin.RemoteLookupExpr = *remoteLookupExpr
 	})
 	lookupJoin.Cols = lookupJoin.Cols.CopyAndMaybeRemap(srcColsToDstCols)
-	constFilters := c.e.f.RemapCols(&lookupJoin.ConstFilters, srcColsToDstCols).(*memo.FiltersExpr)
-	lookupJoin.ConstFilters = *constFilters
+	allLookupFilters := c.e.f.RemapCols(&lookupJoin.AllLookupFilters, srcColsToDstCols).(*memo.FiltersExpr)
+	lookupJoin.AllLookupFilters = *allLookupFilters
 	on := c.e.f.RemapCols(&lookupJoin.On, srcColsToDstCols).(*memo.FiltersExpr)
 	lookupJoin.On = *on
 	lookupJoin.DerivedEquivCols = lookupJoin.DerivedEquivCols.CopyAndMaybeRemap(srcColsToDstCols)
@@ -1190,7 +1190,7 @@ func (c *CustomFuncs) ConvertIndexToLookupJoinPrivate(
 		KeyCols:               lookupCols,
 		Cols:                  outCols,
 		LookupColsAreTableKey: true,
-		ConstFilters:          nil,
+		AllLookupFilters:      nil,
 		Locking:               indexPrivate.Locking,
 		JoinPrivate:           memo.JoinPrivate{},
 	}
@@ -1296,6 +1296,15 @@ func (c *CustomFuncs) CreateLocalityOptimizedLookupJoinPrivate(
 	newPrivate.LookupExpr = lookupExpr
 	newPrivate.RemoteLookupExpr = remoteLookupExpr
 	newPrivate.LocalityOptimized = true
+	switch private.JoinType {
+	case opt.AntiJoinOp:
+		// Add the filters from the LookupExpr, because we need to account for the
+		// regions selected in our statistics estimation. This is only needed for
+		// anti join because it is the only locality optimized join that is split
+		// into two separate operators. Note that only lookupExpr is used for anti
+		// joins, not remoteLookupExpr.
+		newPrivate.AllLookupFilters = append(newPrivate.AllLookupFilters, lookupExpr...)
+	}
 	return &newPrivate
 }
 
@@ -2145,8 +2154,8 @@ func (c *CustomFuncs) mapInputSideOfLookupJoin(
 	mappedLookupJoinExpr.ContinuationCol = lookupJoinExpr.ContinuationCol
 	mappedLookupJoinExpr.LocalityOptimized = lookupJoinExpr.LocalityOptimized
 	mappedLookupJoinExpr.ChildOfLocalityOptimizedSearch = lookupJoinExpr.ChildOfLocalityOptimizedSearch
-	constFilters := c.e.f.RemapCols(&lookupJoinExpr.ConstFilters, colMap).(*memo.FiltersExpr)
-	mappedLookupJoinExpr.ConstFilters = *constFilters
+	allLookupFilters := c.e.f.RemapCols(&lookupJoinExpr.AllLookupFilters, colMap).(*memo.FiltersExpr)
+	mappedLookupJoinExpr.AllLookupFilters = *allLookupFilters
 	mappedLookupJoinExpr.Locking = lookupJoinExpr.Locking
 	return mappedLookupJoinExpr
 }

pkg/sql/opt/xform/testdata/rules/join

@@ -13398,7 +13398,7 @@ limit
  │    │    │         └── c_p_id:25 = p_id:12 [outer=(12,25), constraints=(/12: (/NULL - ]; /25: (/NULL - ]), fd=(12)==(25), (25)==(12)]
  │    │    ├── lookup columns are key
  │    │    ├── key: (24)
- │    │    ├── fd: ()-->(15,26), (24)-->(25), (12)-->(13,14), (13)~~>(12,14), (12)==(25), (25)==(12)
+ │    │    ├── fd: ()-->(26), (24)-->(25), (12)-->(13-15), (13)~~>(12,14,15), (12)==(25), (25)==(12)
  │    │    ├── limit hint: 1.00
  │    │    ├── scan child [as=c]
  │    │    │    ├── columns: c_id:24!null c_p_id:25 c.crdb_region:26!null
@@ -13457,7 +13457,7 @@ limit
  │    │    │         └── c_p_id:25 = id2:13 [outer=(13,25), constraints=(/13: (/NULL - ]; /25: (/NULL - ]), fd=(13)==(25), (25)==(13)]
  │    │    ├── lookup columns are key
  │    │    ├── key: (24)
- │    │    ├── fd: ()-->(15,26), (24)-->(25), (12)-->(13,14), (13)-->(12,14), (13)==(25), (25)==(13)
+ │    │    ├── fd: ()-->(26), (24)-->(25), (12)-->(13-15), (13)-->(12,14,15), (13)==(25), (25)==(13)
  │    │    ├── limit hint: 1.00
  │    │    ├── scan child3 [as=c]
  │    │    │    ├── columns: c_id:24!null c_p_id:25 c.crdb_region:26!null
@@ -13525,7 +13525,7 @@ limit
  │    │    │    │         └── c_p_id:25 = id2:13 [outer=(13,25), constraints=(/13: (/NULL - ]; /25: (/NULL - ]), fd=(13)==(25), (25)==(13)]
  │    │    │    ├── lookup columns are key
  │    │    │    ├── key: (24)
- │    │    │    ├── fd: ()-->(15,26), (24)-->(25), (12)-->(13), (13)-->(12), (13)==(25), (25)==(13)
+ │    │    │    ├── fd: ()-->(26), (24)-->(25), (12)-->(13,15), (13)-->(12,15), (13)==(25), (25)==(13)
  │    │    │    ├── limit hint: 29.67
  │    │    │    ├── scan child [as=c]
  │    │    │    │    ├── columns: c_id:24!null c_p_id:25 c.crdb_region:26!null