modified GeneralizeType to match rules for Case statement, need to test

Author: angelamayxie

enginetest/queries/script_queries.go

@@ -8727,7 +8727,8 @@ where
 				},
 			},
 			{
-				Query: "select if(t0.c0 = 1, t0.c0, 128) as ref0 from t0",
+				Query:    "select if(t0.c0 = 1, t0.c0, 128) as ref0 from t0",
+				Expected: []sql.Row{{128}},
 			},
 		},
 	},

sql/expression/case.go

@@ -43,71 +43,14 @@ func NewCase(expr sql.Expression, branches []CaseBranch, elseExpr sql.Expression
 	return &Case{expr, branches, elseExpr}
 }
 
-// From the description of operator typing here:
-// https://dev.mysql.com/doc/refman/8.0/en/flow-control-functions.html#operator_case
-func combinedCaseBranchType(left, right sql.Type) sql.Type {
-	if left == types.Null {
-		return right
-	}
-	if right == types.Null {
-		return left
-	}
-
-	// Our current implementation of StringType.Convert(enum), does not match MySQL's behavior.
-	// So, we make sure to return Enums in this particular case.
-	// More details: https://github.com/dolthub/dolt/issues/8598
-	if types.IsEnum(left) && types.IsEnum(right) {
-		return right
-	}
-	if types.IsSet(left) && types.IsSet(right) {
-		return right
-	}
-	if types.IsTextOnly(left) && types.IsTextOnly(right) {
-		return types.LongText
-	}
-	if types.IsTextBlob(left) && types.IsTextBlob(right) {
-		return types.LongBlob
-	}
-	if types.IsTime(left) && types.IsTime(right) {
-		if left == right {
-			return left
-		}
-		return types.DatetimeMaxPrecision
-	}
-	if types.IsNumber(left) && types.IsNumber(right) {
-		if left == types.Float64 || right == types.Float64 {
-			return types.Float64
-		}
-		if left == types.Float32 || right == types.Float32 {
-			return types.Float32
-		}
-		if types.IsDecimal(left) || types.IsDecimal(right) {
-			return types.MustCreateDecimalType(65, 10)
-		}
-		if left == types.Uint64 && types.IsSigned(right) ||
-			right == types.Uint64 && types.IsSigned(left) {
-			return types.MustCreateDecimalType(65, 10)
-		}
-		if !types.IsSigned(left) && !types.IsSigned(right) {
-			return types.Uint64
-		} else {
-			return types.Int64
-		}
-	}
-	if types.IsJSON(left) && types.IsJSON(right) {
-		return types.JSON
-	}
-	return types.LongText
-}
-
 // Type implements the sql.Expression interface.
 func (c *Case) Type() sql.Type {
 	curr := types.Null
 	for _, b := range c.Branches {
-		curr = combinedCaseBranchType(curr, b.Value.Type())
+		curr = types.GeneralizeTypes(curr, b.Value.Type())
 	}
 	if c.Else != nil {
-		curr = combinedCaseBranchType(curr, c.Else.Type())
+		curr = types.GeneralizeTypes(curr, c.Else.Type())
 	}
 	return curr
 }

sql/expression/function/if.go

@@ -89,7 +89,9 @@ func (f *If) Eval(ctx *sql.Context, row sql.Row) (interface{}, error) {
 			return nil, err
 		}
 	}
-	eval, _, err = f.Type().Convert(ctx, eval)
+	if ret, _, err := f.Type().Convert(ctx, eval); err == nil {
+		return ret, nil
+	}
 	return eval, err
 }
 

sql/expression/function/ifnull.go

@@ -52,20 +52,26 @@ func (f *IfNull) Description() string {
 
 // Eval implements the Expression interface.
 func (f *IfNull) Eval(ctx *sql.Context, row sql.Row) (interface{}, error) {
+	t := f.Type()
+
 	left, err := f.LeftChild.Eval(ctx, row)
 	if err != nil {
 		return nil, err
 	}
 	if left != nil {
-		left, _, err = f.Type().Convert(ctx, left)
+		if ret, _, err := t.Convert(ctx, left); err == nil {
+			return ret, nil
+		}
 		return left, err
 	}
 
 	right, err := f.RightChild.Eval(ctx, row)
 	if err != nil {
 		return nil, err
 	}
-	right, _, err = f.Type().Convert(ctx, right)
+	if ret, _, err := t.Convert(ctx, right); err == nil {
+		return ret, nil
+	}
 	return right, err
 }
 

sql/types/conversion.go

@@ -555,24 +555,166 @@ func TypesEqual(a, b sql.Type) bool {
 	}
 }
 
+// generalizeNumberTypes assumes both inputs return true for IsNumber
+func generalizeNumberTypes(a, b sql.Type) sql.Type {
+	if a == Float64 || b == Float64 {
+		return Float64
+	}
+	if a == Float32 || b == Float32 {
+		return Float32
+	}
+
+	if IsDecimal(a) || IsDecimal(b) {
+		// TODO: match precision and scale to that of the decimal type, check if defines column
+		return MustCreateDecimalType(DecimalTypeMaxPrecision, DecimalTypeMaxScale)
+	}
+
+	aIsSigned := IsSigned(a)
+	bIsSigned := IsSigned(b)
+
+	if a == Uint64 || b == Uint64 {
+		if aIsSigned || bIsSigned {
+			return MustCreateDecimalType(DecimalTypeMaxPrecision, 0)
+		}
+		return Uint64
+	}
+
+	if a == Int64 || b == Int64 {
+		return Int64
+	}
+
+	if a == Uint32 || b == Uint32 {
+		if aIsSigned || bIsSigned {
+			return Int64
+		}
+		return Uint32
+	}
+
+	if a == Int32 || b == Int32 {
+		return Int32
+	}
+
+	if a == Uint24 || b == Uint24 {
+		if aIsSigned || bIsSigned {
+			return Int32
+		}
+	}
+
+	if a == Int24 || b == Int24 {
+		return Int24
+	}
+
+	if a == Uint16 || b == Uint16 {
+		if aIsSigned || bIsSigned {
+			return Int24
+		}
+		return Uint16
+	}
+
+	if a == Int16 || b == Int16 {
+		return Int16
+	}
+
+	if a == Uint8 || b == Uint8 {
+		if aIsSigned || bIsSigned {
+			return Int16
+		}
+		return Uint8
+	}
+
+	if a == Int8 || b == Int8 {
+		return Int8
+	}
+
+	if IsBoolean(a) && IsBoolean(b) {
+		return Boolean
+	}
+
+	return Int64
+}
+
 // GeneralizeTypes returns the more "general" of two types as defined by
-// https://dev.mysql.com/doc/refman/8.4/en/flow-control-functions.html#function_if and
-// https://dev.mysql.com/doc/refman/8.4/en/flow-control-functions.html#function_ifnull
-// TODO: Currently returns the most general type via Promote(). Update to match MySQL (pick the more general of the two
-// given types)
+// https://dev.mysql.com/doc/refman/8.4/en/flow-control-functions.html
+// TODO: Create and handle "Illegal mix of collations" error
 func GeneralizeTypes(a, b sql.Type) sql.Type {
-	if IsText(a) || IsText(b) {
-		// TODO: handle case-sensitive strings
-		return LongText
+	if a == Null {
+		return b
+	}
+	if b == Null {
+		return a
 	}
 
-	if IsFloat(a) || IsFloat(b) {
-		return Float64
+	if IsJSON(a) && IsJSON(b) {
+		return JSON
 	}
 
-	if a == Null {
-		return b.Promote()
+	if IsGeometry(a) && IsGeometry(b) {
+		return a
 	}
 
-	return a.Promote()
+	if IsEnum(a) && IsEnum(b) {
+		return a
+	}
+
+	if IsSet(a) && IsSet(b) {
+		return a
+	}
+
+	aIsTimespan := IsTimespan(a)
+	bIsTimespan := IsTimespan(b)
+	if aIsTimespan && bIsTimespan {
+		return a
+	}
+	if (IsTime(a) || aIsTimespan) && (IsTime(b) || bIsTimespan) {
+		if IsDateType(a) && IsDateType(b) {
+			return Date
+		}
+		if IsTimestampType(a) && IsTimestampType(b) {
+			// TODO: match precision to max precision of the two timestamps
+			return TimestampMaxPrecision
+		}
+		// TODO: match precision to max precision of the two time types
+		return DatetimeMaxPrecision
+	}
+
+	if IsBlobType(a) || IsBlobType(b) {
+		return Blob
+	}
+
+	aIsBit := IsBit(a)
+	bIsBit := IsBit(b)
+	if aIsBit && bIsBit {
+		// TODO: match max bits to max of max bits between a and b
+		return a.Promote()
+	}
+	if aIsBit {
+		a = Int64
+	}
+	if bIsBit {
+		b = Int64
+	}
+
+	aIsYear := IsYear(a)
+	bIsYear := IsYear(b)
+	if aIsYear && bIsYear {
+		return a
+	}
+	if aIsYear {
+		a = Int32
+	}
+	if bIsYear {
+		b = Int32
+	}
+
+	if IsNumber(a) && IsNumber(b) {
+		if svt, ok := a.(sql.SystemVariableType); ok {
+			a = svt.UnderlyingType()
+		}
+		if svt, ok := a.(sql.SystemVariableType); ok {
+			b = svt.UnderlyingType()
+		}
+		return generalizeNumberTypes(a, b)
+	}
+	// TODO: decide if we want to make this VarChar to match MySQL, match VarChar length to max of two types
+	return LongText
 }

sql/types/conversion_test.go

@@ -168,9 +168,9 @@ func TestGeneralizeTypes(t *testing.T) {
 		{Text, Text, LongText},
 		{Text, Float64, LongText},
 		{Int64, Text, LongText},
-		{Float32, Float32, Float64},
+		{Float32, Float32, Float32},
 		{Int64, Float64, Float64},
-		{Int32, Int32, Int64},
+		{Int32, Int32, Int32},
 		{Null, Null, Null},
 	}
 	for _, test := range tests {