From 2860e1c3f12bb43e1b39e58c7c07fca6fe06b8dc Mon Sep 17 00:00:00 2001
From: Marcus Gartner <marcus@cockroachlabs.com>
Date: Wed, 14 Aug 2024 16:13:55 -0400
Subject: [PATCH] opt: do not impose type width on CASE branches

The optimizer no longer forces the branches of CASE-like expressions to
have the same type-width. This matches the behavior of Postgres.

Fixes #127889
Informs #108360

Release note (bug fix): A bug has been fixed that caused incorrect
evaluation of `CASE`, `COALESCE`, and `IF` expressions with branches
producing fixed-width string-like types, such as `CHAR`.
---
 pkg/sql/logictest/testdata/logic_test/case   | 36 ++++++++++++++++++++
 pkg/sql/logictest/testdata/logic_test/typing |  6 +---
 pkg/sql/opt/optbuilder/scalar.go             |  8 ++---
 3 files changed, 41 insertions(+), 9 deletions(-)

diff --git a/pkg/sql/logictest/testdata/logic_test/case b/pkg/sql/logictest/testdata/logic_test/case
index 6c6e40dbba01..3777f2bc9d42 100644
--- a/pkg/sql/logictest/testdata/logic_test/case
+++ b/pkg/sql/logictest/testdata/logic_test/case
@@ -48,3 +48,39 @@ SELECT CASE WHEN f = 0
 FROM a;
 
 subtest end
+
+# Regression test for #127889. CASE-like expressions should not impose type
+# widths of one branch on other branches.
+subtest regression_127889
+
+query T
+SELECT CASE WHEN true THEN 'foo'::TEXT ELSE 'b'::CHAR END
+----
+foo
+
+query T
+SELECT COALESCE(NULL::CHAR, 'bar'::CHAR(2))
+----
+ba
+
+query T
+SELECT IF(false, 'foo'::CHAR, 'bar'::CHAR(2))
+----
+ba
+
+query T
+SELECT CASE WHEN false THEN 'b'::CHAR ELSE 'foo'::TEXT END;
+----
+foo
+
+query T
+SELECT (CASE WHEN false THEN 'b'::CHAR ELSE 'foo'::TEXT END)::CHAR;
+----
+f
+
+query T
+SELECT (CASE WHEN false THEN 'b'::CHAR ELSE 'foo'::TEXT END)::BPCHAR;
+----
+foo
+
+subtest end
diff --git a/pkg/sql/logictest/testdata/logic_test/typing b/pkg/sql/logictest/testdata/logic_test/typing
index 78923573e608..4a411a75f888 100644
--- a/pkg/sql/logictest/testdata/logic_test/typing
+++ b/pkg/sql/logictest/testdata/logic_test/typing
@@ -289,15 +289,12 @@ WHERE t102110_1.t NOT BETWEEN t102110_1.t AND
   (CASE WHEN NULL THEN t102110_2.c ELSE t102110_1.t END);
 ----
 
-# IF is typed differently (Postgres does not support IF).
 query T
 SELECT t102110_1.t FROM t102110_1, t102110_2
 WHERE t102110_1.t NOT BETWEEN t102110_1.t AND
   IF(NULL, t102110_2.c, t102110_1.t);
 ----
-tt
 
-# TODO(#108360): implicit casts from STRING to CHAR should use bpchar.
 statement ok
 CREATE TABLE t108360_1 (t TEXT);
 INSERT INTO t108360_1 VALUES ('tt');
@@ -306,13 +303,12 @@ statement ok
 CREATE TABLE t108360_2 (c CHAR);
 INSERT INTO t108360_2 VALUES ('c');
 
-# This returns one row with 'tt' in Postgres. The results are different because
-# Postgres casts t108360_1.t to bpchar rather than char.
 query T
 SELECT (CASE WHEN t108360_1.t > t108360_2.c THEN t108360_1.t ELSE t108360_2.c END)
 FROM t108360_1, t108360_2
 WHERE t108360_1.t = (CASE WHEN t108360_1.t > t108360_2.c THEN t108360_1.t ELSE t108360_2.c END);
 ----
+tt
 
 # Static analysis types should never make it to execution.
 statement ok
diff --git a/pkg/sql/opt/optbuilder/scalar.go b/pkg/sql/opt/optbuilder/scalar.go
index 5755c1035106..5f1d2d01f8db 100644
--- a/pkg/sql/opt/optbuilder/scalar.go
+++ b/pkg/sql/opt/optbuilder/scalar.go
@@ -249,7 +249,7 @@ func (b *Builder) buildScalar(
 		for i := range t.Whens {
 			condExpr := t.Whens[i].Cond.(tree.TypedExpr)
 			cond := b.buildScalar(condExpr, inScope, nil, nil, colRefs)
-			valExpr, ok := eval.ReType(t.Whens[i].Val.(tree.TypedExpr), valType)
+			valExpr, ok := eval.ReType(t.Whens[i].Val.(tree.TypedExpr), valType.WithoutTypeModifiers())
 			if !ok {
 				panic(pgerror.Newf(
 					pgcode.DatatypeMismatch,
@@ -263,7 +263,7 @@ func (b *Builder) buildScalar(
 		// Add the ELSE expression to the end of whens as a raw scalar expression.
 		var orElse opt.ScalarExpr
 		if t.Else != nil {
-			elseExpr, ok := eval.ReType(t.Else.(tree.TypedExpr), valType)
+			elseExpr, ok := eval.ReType(t.Else.(tree.TypedExpr), valType.WithoutTypeModifiers())
 			if !ok {
 				panic(pgerror.Newf(
 					pgcode.DatatypeMismatch,
@@ -289,7 +289,7 @@ func (b *Builder) buildScalar(
 			// The type of the CoalesceExpr might be different than the inputs (e.g.
 			// when they are NULL). Force all inputs to be the same type, so that we
 			// build coalesce operator with the correct type.
-			expr, ok := eval.ReType(t.TypedExprAt(i), typ)
+			expr, ok := eval.ReType(t.TypedExprAt(i), typ.WithoutTypeModifiers())
 			if !ok {
 				panic(pgerror.Newf(
 					pgcode.DatatypeMismatch,
@@ -339,7 +339,7 @@ func (b *Builder) buildScalar(
 		ifTrueExpr := reType(t.True.(tree.TypedExpr), valType)
 		ifTrue := b.buildScalar(ifTrueExpr, inScope, nil, nil, colRefs)
 		whens := memo.ScalarListExpr{b.factory.ConstructWhen(memo.TrueSingleton, ifTrue)}
-		orElseExpr, ok := eval.ReType(t.Else.(tree.TypedExpr), valType)
+		orElseExpr, ok := eval.ReType(t.Else.(tree.TypedExpr), valType.WithoutTypeModifiers())
 		if !ok {
 			panic(pgerror.Newf(
 				pgcode.DatatypeMismatch,