expression: use the correct type when eval decimal and float session …

…var (#51395) (#53415) close #43527
pingcap · May 21, 2024 · 3e08750 · 3e08750
1 parent 7af62de
commit 3e08750
Show file tree

Hide file tree

Showing 5 changed files with 76 additions and 5 deletions.
diff --git a/pkg/expression/builtin_other.go b/pkg/expression/builtin_other.go
@@ -1053,7 +1053,11 @@ func (b *builtinGetRealVarSig) evalReal(row chunk.Row) (float64, bool, error) {
 	}
 	varName = strings.ToLower(varName)
 	if v, ok := sessionVars.GetUserVarVal(varName); ok {
-		return v.GetFloat64(), false, nil
+		d, err := v.ToFloat64(sessionVars.StmtCtx)
+		if err != nil {
+			return 0, false, err
+		}
+		return d, false, nil
 	}
 	return 0, true, nil
 }
@@ -1093,7 +1097,11 @@ func (b *builtinGetDecimalVarSig) evalDecimal(row chunk.Row) (*types.MyDecimal,
 	}
 	varName = strings.ToLower(varName)
 	if v, ok := sessionVars.GetUserVarVal(varName); ok {
-		return v.GetMysqlDecimal(), false, nil
+		d, err := v.ToDecimal(sessionVars.StmtCtx)
+		if err != nil {
+			return nil, false, err
+		}
+		return d, false, nil
 	}
 	return nil, true, nil
 }

diff --git a/pkg/expression/builtin_other_test.go b/pkg/expression/builtin_other_test.go
@@ -171,6 +171,33 @@ func TestGetVar(t *testing.T) {
 	}
 }
 
+func TestTypeConversion(t *testing.T) {
+	ctx := createContext(t)
+	// Set value as int64
+	key := "a"
+	val := int64(3)
+	ctx.GetSessionVars().SetUserVarVal(key, types.NewDatum(val))
+	tp := types.NewFieldType(mysql.TypeLonglong)
+	ctx.GetSessionVars().SetUserVarType(key, tp)
+
+	args := []any{"a"}
+	// To Decimal.
+	tp = types.NewFieldType(mysql.TypeNewDecimal)
+	fn, err := BuildGetVarFunction(ctx, datumsToConstants(types.MakeDatums(args...))[0], tp)
+	require.NoError(t, err)
+	d, err := fn.Eval(chunk.Row{})
+	require.NoError(t, err)
+	des := types.NewDecFromInt(3)
+	require.Equal(t, des, d.GetValue())
+	// To Float.
+	tp = types.NewFieldType(mysql.TypeDouble)
+	fn, err = BuildGetVarFunction(ctx, datumsToConstants(types.MakeDatums(args...))[0], tp)
+	require.NoError(t, err)
+	d, err = fn.Eval(chunk.Row{})
+	require.NoError(t, err)
+	require.Equal(t, float64(3), d.GetValue())
+}
+
 func TestValues(t *testing.T) {
 	ctx := createContext(t)
 	fc := &valuesFunctionClass{baseFunctionClass{ast.Values, 0, 0}, 1, types.NewFieldType(mysql.TypeVarchar)}

diff --git a/pkg/expression/builtin_other_vec.go b/pkg/expression/builtin_other_vec.go
@@ -386,6 +386,8 @@ func (b *builtinGetRealVarSig) vectorized() bool {
 	return true
 }
 
+// NOTE: get/set variable vectorized eval was disabled. See more in
+// https://github.com/pingcap/tidb/pull/8412
 func (b *builtinGetRealVarSig) vecEvalReal(input *chunk.Chunk, result *chunk.Column) error {
 	n := input.NumRows()
 	buf0, err := b.bufAllocator.get()
@@ -406,7 +408,11 @@ func (b *builtinGetRealVarSig) vecEvalReal(input *chunk.Chunk, result *chunk.Col
 		}
 		varName := strings.ToLower(buf0.GetString(i))
 		if v, ok := sessionVars.GetUserVarVal(varName); ok {
-			f64s[i] = v.GetFloat64()
+			d, err := v.ToFloat64(sessionVars.StmtCtx)
+			if err != nil {
+				return err
+			}
+			f64s[i] = d
 			continue
 		}
 		result.SetNull(i, true)
@@ -418,6 +424,8 @@ func (b *builtinGetDecimalVarSig) vectorized() bool {
 	return true
 }
 
+// NOTE: get/set variable vectorized eval was disabled. See more in
+// https://github.com/pingcap/tidb/pull/8412
 func (b *builtinGetDecimalVarSig) vecEvalDecimal(input *chunk.Chunk, result *chunk.Column) error {
 	n := input.NumRows()
 	buf0, err := b.bufAllocator.get()
@@ -438,7 +446,11 @@ func (b *builtinGetDecimalVarSig) vecEvalDecimal(input *chunk.Chunk, result *chu
 		}
 		varName := strings.ToLower(buf0.GetString(i))
 		if v, ok := sessionVars.GetUserVarVal(varName); ok {
-			decs[i] = *v.GetMysqlDecimal()
+			d, err := v.ToDecimal(sessionVars.StmtCtx)
+			if err != nil {
+				return err
+			}
+			decs[i] = *d
 			continue
 		}
 		result.SetNull(i, true)

diff --git a/pkg/expression/integration_test/BUILD.bazel b/pkg/expression/integration_test/BUILD.bazel
@@ -8,7 +8,7 @@ go_test(
         "main_test.go",
     ],
     flaky = True,
-    shard_count = 28,
+    shard_count = 29,
     deps = [
         "//pkg/config",
         "//pkg/domain",

diff --git a/pkg/expression/integration_test/integration_test.go b/pkg/expression/integration_test/integration_test.go
@@ -3271,3 +3271,27 @@ func TestTiDBRowChecksumBuiltin(t *testing.T) {
 	tk.MustGetDBError("select tidb_row_checksum() from t", expression.ErrNotSupportedYet)
 	tk.MustGetDBError("select tidb_row_checksum() from t where id > 0", expression.ErrNotSupportedYet)
 }
+
+func TestIssue43527(t *testing.T) {
+	store := testkit.CreateMockStore(t)
+	tk := testkit.NewTestKit(t, store)
+	tk.MustExec("use test")
+	tk.MustExec("create table test (a datetime, b bigint, c decimal(10, 2), d float)")
+	tk.MustExec("insert into test values('2010-10-10 10:10:10', 100, 100.01, 100)")
+	// Decimal.
+	tk.MustQuery(
+		"SELECT @total := @total + c FROM (SELECT c FROM test) AS temp, (SELECT @total := 200) AS T1",
+	).Check(testkit.Rows("300.01"))
+	// Float.
+	tk.MustQuery(
+		"SELECT @total := @total + d FROM (SELECT d FROM test) AS temp, (SELECT @total := 200) AS T1",
+	).Check(testkit.Rows("300"))
+	tk.MustExec("insert into test values('2010-10-10 10:10:10', 100, 100.01, 100)")
+	// Vectorized.
+	// NOTE: Because https://github.com/pingcap/tidb/pull/8412 disabled the vectorized execution of get or set variable,
+	// the following test case will not be executed in vectorized mode.
+	// It will be executed in the normal mode.
+	tk.MustQuery(
+		"SELECT @total := @total + d FROM (SELECT d FROM test) AS temp, (SELECT @total := b FROM test) AS T1 where @total >= 100",
+	).Check(testkit.Rows("200", "300", "400", "500"))
+}