opt: do not remap columns with non-identical types

This commit limits the remapping of column during join elimination to
only destination columns with identical types to their equivalent source
columns. This prevents the optimizer from producing an incorrect query
plan. See the test cases for examples.

Fixes cockroachdb#107850

There is no release note because this bug does not exist in any
releases.

Release note: None

Author: mgartner

pkg/sql/logictest/testdata/logic_test/join

@@ -1217,3 +1217,24 @@ SELECT * FROM (SELECT * FROM t106371 ORDER BY y LIMIT 1) a
 JOIN (SELECT DISTINCT ON (x) * FROM (SELECT * FROM t106371 WHERE y = 2)) b ON a.x = b.x;
 ----
 1  1  1  2
+
+# Regression test for #107850. Do not eliminate joins and remap columns that are
+# equivalent but have non-identical types.
+subtest regression_107850
+
+statement ok
+CREATE TABLE t107850a (
+  d2 DECIMAL(10, 2) NOT NULL UNIQUE
+);
+CREATE TABLE t107850b (
+  d0 DECIMAL(10, 0) NOT NULL REFERENCES t107850a (d2)
+);
+INSERT INTO t107850a VALUES (1.00);
+INSERT INTO t107850b VALUES (1);
+
+query R
+SELECT d2 FROM t107850a JOIN t107850b ON d2 = d0
+----
+1.00
+
+subtest end

pkg/sql/opt/norm/general_funcs.go

@@ -308,15 +308,34 @@ func (c *CustomFuncs) MakeBoolCol() opt.ColumnID {
 // CanRemapCols returns true if it's possible to remap every column in the
 // "from" set to a column in the "to" set using the given FDs.
 func (c *CustomFuncs) CanRemapCols(from, to opt.ColSet, fds *props.FuncDepSet) bool {
-	for col, ok := from.Next(0); ok; col, ok = from.Next(col + 1) {
-		if !fds.ComputeEquivGroup(col).Intersects(to) {
+	for fromCol, ok := from.Next(0); ok; fromCol, ok = from.Next(fromCol + 1) {
+		if _, ok := c.remapColWithIdenticalType(fromCol, to, fds); !ok {
 			// It is not possible to remap this column to one from the "to" set.
 			return false
 		}
 	}
 	return true
 }
 
+// remapColWithIdenticalType returns a column in the "to" set that is equivalent
+// to "fromCol" and has the identical type. It returns ok=false if no such
+// column exists.
+func (c *CustomFuncs) remapColWithIdenticalType(
+	fromCol opt.ColumnID, to opt.ColSet, fds *props.FuncDepSet,
+) (_ opt.ColumnID, ok bool) {
+	md := c.mem.Metadata()
+	fromColType := md.ColumnMeta(fromCol).Type
+	equivCols := fds.ComputeEquivGroup(fromCol)
+	equivCols.IntersectionWith(to)
+	for equivCol, ok := equivCols.Next(0); ok; equivCol, ok = equivCols.Next(equivCol + 1) {
+		equivColType := md.ColumnMeta(equivCol).Type
+		if fromColType.Identical(equivColType) {
+			return equivCol, true
+		}
+	}
+	return 0, false
+}
+
 // ----------------------------------------------------------------------
 //
 // Outer column functions

pkg/sql/opt/norm/project_funcs.go

@@ -859,14 +859,10 @@ func (c *CustomFuncs) ProjectRemappedCols(
 ) (projections memo.ProjectionsExpr) {
 	for col, ok := from.Next(0); ok; col, ok = from.Next(col + 1) {
 		if !to.Contains(col) {
-			// TODO(mgartner): We don't need to compute the entire equivalence group,
-			// we only need to find the first equivalent column that's in the to set.
-			candidates := fds.ComputeEquivGroup(col)
-			candidates.IntersectionWith(to)
-			if candidates.Empty() {
+			toCol, ok := c.remapColWithIdenticalType(col, to, fds)
+			if !ok {
 				panic(errors.AssertionFailedf("cannot remap column %v", col))
 			}
-			toCol, _ := candidates.Next(0)
 			projections = append(
 				projections,
 				c.f.ConstructProjectionsItem(c.f.ConstructVariable(toCol), col),
@@ -881,18 +877,6 @@ func (c *CustomFuncs) ProjectRemappedCols(
 func (c *CustomFuncs) RemapProjectionCols(
 	projections memo.ProjectionsExpr, to opt.ColSet, fds *props.FuncDepSet,
 ) memo.ProjectionsExpr {
-	getReplacement := func(col opt.ColumnID) opt.ColumnID {
-		// TODO(mgartner): We don't need to compute the entire equivalence group, we
-		// only need to find the first equivalent column that's in the to set.
-		candidates := fds.ComputeEquivGroup(col)
-		candidates.IntersectionWith(to)
-		if candidates.Empty() {
-			panic(errors.AssertionFailedf("cannot remap column"))
-		}
-		replacement, _ := candidates.Next(0)
-		return replacement
-	}
-
 	// Use the projection outer columns to filter out columns that are synthesized
 	// within the projections themselves (e.g. in a subquery).
 	outerCols := c.ProjectionOuterCols(projections)
@@ -902,7 +886,11 @@ func (c *CustomFuncs) RemapProjectionCols(
 	replace = func(e opt.Expr) opt.Expr {
 		if v, ok := e.(*memo.VariableExpr); ok && !to.Contains(v.Col) && outerCols.Contains(v.Col) {
 			// This variable needs to be remapped.
-			return c.f.ConstructVariable(getReplacement(v.Col))
+			toCol, ok := c.remapColWithIdenticalType(v.Col, to, fds)
+			if !ok {
+				panic(errors.AssertionFailedf("cannot remap column %v", v.Col))
+			}
+			return c.f.ConstructVariable(toCol)
 		}
 		return c.f.Replace(e, replace)
 	}

pkg/sql/opt/norm/testdata/rules/project

@@ -266,6 +266,14 @@ project
  └── projections
       └── child.parent_id:2 [as=parent_id:9, outer=(2)]
 
+exec-ddl
+DROP TABLE child
+----
+
+exec-ddl
+DROP TABLE parent
+----
+
 # The join can be eliminated as long as it doesn't require remapping with
 # OptimizerUseImprovedJoinElimination disabled.
 norm set=optimizer_use_improved_join_elimination=false expect=EliminateJoinUnderProjectLeft
@@ -395,6 +403,87 @@ project
                      └── projections
                           └── NULL [as="?column?":5]
 
+# Regression test for #107850 - columns should not be mapped if their types are
+# not identical.
+exec-ddl
+CREATE TABLE parent (
+  id DECIMAL(10, 2) PRIMARY KEY
+)
+----
+
+exec-ddl
+CREATE TABLE child (
+  id INT PRIMARY KEY,
+  parent_id DECIMAL(10, 0) NOT NULL REFERENCES parent(id),
+  parent_id2 DECIMAL(10, 2) NOT NULL REFERENCES parent(id)
+)
+----
+
+norm expect-not=EliminateJoinUnderProjectRight
+SELECT parent.id FROM parent JOIN child ON parent.id = child.parent_id
+----
+project
+ ├── columns: id:1!null
+ ├── immutable
+ └── inner-join (hash)
+      ├── columns: parent.id:1!null parent_id:5!null
+      ├── multiplicity: left-rows(zero-or-more), right-rows(exactly-one)
+      ├── immutable
+      ├── fd: (1)==(5), (5)==(1)
+      ├── scan parent
+      │    ├── columns: parent.id:1!null
+      │    └── key: (1)
+      ├── scan child
+      │    └── columns: parent_id:5!null
+      └── filters
+           └── parent.id:1 = parent_id:5 [outer=(1,5), immutable, constraints=(/1: (/NULL - ]; /5: (/NULL - ]), fd=(1)==(5), (5)==(1)]
+
+# The projected column with the identical type, child.parent_id2, should be
+# remapped if there are multiple equivalent columns.
+norm
+SELECT parent.id FROM parent JOIN child ON parent.id = child.parent_id AND child.parent_id = child.parent_id2
+----
+project
+ ├── columns: id:1!null
+ ├── immutable
+ ├── select
+ │    ├── columns: parent_id:5!null parent_id2:6!null
+ │    ├── immutable
+ │    ├── fd: (5)==(6), (6)==(5)
+ │    ├── scan child
+ │    │    └── columns: parent_id:5!null parent_id2:6!null
+ │    └── filters
+ │         └── parent_id:5 = parent_id2:6 [outer=(5,6), immutable, constraints=(/5: (/NULL - ]; /6: (/NULL - ]), fd=(5)==(6), (6)==(5)]
+ └── projections
+      └── parent_id2:6 [as=parent.id:1, outer=(6)]
+
+# In a projection expression the column with the identical type,
+# child.parent_id2, should be remapped if there are multiple equivalent columns.
+norm
+SELECT 1 + parent.id FROM parent JOIN child ON parent.id = child.parent_id AND child.parent_id = child.parent_id2
+----
+project
+ ├── columns: "?column?":9!null
+ ├── immutable
+ ├── select
+ │    ├── columns: parent_id:5!null parent_id2:6!null
+ │    ├── immutable
+ │    ├── fd: (5)==(6), (6)==(5)
+ │    ├── scan child
+ │    │    └── columns: parent_id:5!null parent_id2:6!null
+ │    └── filters
+ │         └── parent_id:5 = parent_id2:6 [outer=(5,6), immutable, constraints=(/5: (/NULL - ]; /6: (/NULL - ]), fd=(5)==(6), (6)==(5)]
+ └── projections
+      └── parent_id2:6 + 1 [as="?column?":9, outer=(6), immutable]
+
+exec-ddl
+DROP TABLE child
+----
+
+exec-ddl
+DROP TABLE parent
+----
+
 # --------------------------------------------------
 # EliminateProject
 # --------------------------------------------------