From c9731573efac05361828e6749dc3dd03ed046de9 Mon Sep 17 00:00:00 2001 From: yibin Date: Thu, 23 May 2024 18:43:21 +0800 Subject: [PATCH] expression: Disallow conv fuction with hybrid type argement to be pushed down to TiKV (#53502) close pingcap/tidb#51877 --- pkg/expression/expr_to_pb_test.go | 19 +++++++++++++++++++ pkg/expression/infer_pushdown.go | 13 ++++++++++++- 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/pkg/expression/expr_to_pb_test.go b/pkg/expression/expr_to_pb_test.go index f912e5f6e4d91..d64af182e2e69 100644 --- a/pkg/expression/expr_to_pb_test.go +++ b/pkg/expression/expr_to_pb_test.go @@ -1456,6 +1456,7 @@ func TestExprPushDownToTiKV(t *testing.T) { //datetimeColumn := genColumn(mysql.TypeDatetime, 6) binaryStringColumn := genColumn(mysql.TypeString, 7) dateColumn := genColumn(mysql.TypeDate, 8) + byteColumn := genColumn(mysql.TypeBit, 9) binaryStringColumn.RetType.SetCollate(charset.CollationBin) // Test exprs that cannot be pushed. @@ -1497,6 +1498,24 @@ func TestExprPushDownToTiKV(t *testing.T) { require.Len(t, pushed, 0) require.Len(t, remained, len(exprs)) + // Test Conv function + exprs = exprs[:0] + function, err = NewFunction(mock.NewContext(), ast.Conv, types.NewFieldType(mysql.TypeString), stringColumn, intColumn, intColumn) + require.NoError(t, err) + exprs = append(exprs, function) + pushed, remained = PushDownExprs(pushDownCtx, exprs, kv.TiKV) + require.Len(t, pushed, len(exprs)) + require.Len(t, remained, 0) + exprs = exprs[:0] + castByteAsStringFunc, err := NewFunction(mock.NewContext(), ast.Cast, types.NewFieldType(mysql.TypeString), byteColumn) + require.NoError(t, err) + function, err = NewFunction(mock.NewContext(), ast.Conv, types.NewFieldType(mysql.TypeString), castByteAsStringFunc, intColumn, intColumn) + require.NoError(t, err) + exprs = append(exprs, function) + pushed, remained = PushDownExprs(pushDownCtx, exprs, kv.TiKV) + require.Len(t, pushed, 0) + require.Len(t, remained, len(exprs)) + // Test exprs that can be pushed. exprs = exprs[:0] pushed = pushed[:0] diff --git a/pkg/expression/infer_pushdown.go b/pkg/expression/infer_pushdown.go index ab50c1a2adb60..dbcef86688935 100644 --- a/pkg/expression/infer_pushdown.go +++ b/pkg/expression/infer_pushdown.go @@ -177,7 +177,7 @@ func scalarExprSupportedByTiKV(sf *ScalarFunction) bool { // Rust use the llvm math functions, which have different precision with Golang/MySQL(cmath) // open the following switchers if we implement them in coprocessor via `cmath` ast.Sin, ast.Asin, ast.Cos, ast.Acos /* ast.Tan */, ast.Atan, ast.Atan2, ast.Cot, - ast.Radians, ast.Degrees, ast.Conv, ast.CRC32, + ast.Radians, ast.Degrees, ast.CRC32, // control flow functions. ast.Case, ast.If, ast.Ifnull, ast.Coalesce, @@ -221,6 +221,17 @@ func scalarExprSupportedByTiKV(sf *ScalarFunction) bool { /*ast.InetNtoa, ast.InetAton, ast.Inet6Ntoa, ast.Inet6Aton, ast.IsIPv4, ast.IsIPv4Compat, ast.IsIPv4Mapped, ast.IsIPv6,*/ ast.UUID: + return true + // Rust use the llvm math functions, which have different precision with Golang/MySQL(cmath) + // open the following switchers if we implement them in coprocessor via `cmath` + case ast.Conv: + arg0 := sf.GetArgs()[0] + // To be aligned with MySQL, tidb handles hybrid type argument and binary literal specially, tikv can't be consistent with tidb now. + if f, ok := arg0.(*ScalarFunction); ok { + if f.FuncName.L == ast.Cast && (f.GetArgs()[0].GetType().Hybrid() || IsBinaryLiteral(f.GetArgs()[0])) { + return false + } + } return true case ast.Round: switch sf.Function.PbCode() {