Skip to content

Commit

Permalink
opt: do not impose type width on CASE branches
Browse files Browse the repository at this point in the history
The optimizer no longer forces the branches of CASE-like expressions to
have the same type-width. This matches the behavior of Postgres.

Fixes cockroachdb#127889
Informs cockroachdb#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`.
  • Loading branch information
mgartner committed Aug 16, 2024
1 parent 8432222 commit 2860e1c
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 9 deletions.
36 changes: 36 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/case
Original file line number Diff line number Diff line change
Expand Up @@ -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
6 changes: 1 addition & 5 deletions pkg/sql/logictest/testdata/logic_test/typing
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -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
Expand Down
8 changes: 4 additions & 4 deletions pkg/sql/opt/optbuilder/scalar.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 2860e1c

Please sign in to comment.