expression: fix tikv crash when bool like cast(bit as char) (pingcap#57484) (pingcap#59321)

close pingcap#56494

Author: ti-chi-bot

pkg/expression/builtin_cast.go

@@ -282,10 +282,20 @@ func (c *castAsStringFunctionClass) getFunction(ctx BuildContext, args []Express
 	if err != nil {
 		return nil, err
 	}
-	if args[0].GetType().Hybrid() {
-		sig = &builtinCastStringAsStringSig{bf}
-		sig.setPbCode(tipb.ScalarFuncSig_CastStringAsString)
-		return sig, nil
+	if ft := args[0].GetType(); ft.Hybrid() {
+		castBitAsUnBinary := ft.GetType() == mysql.TypeBit && c.tp.GetCharset() != charset.CharsetBin
+		if !castBitAsUnBinary {
+			sig = &builtinCastStringAsStringSig{bf}
+			sig.setPbCode(tipb.ScalarFuncSig_CastStringAsString)
+			return sig, nil
+		}
+		// for type BIT, it maybe an invalid value for the specified charset, we need to convert it to binary first,
+		// and then convert it to the specified charset with `HandleBinaryLiteral` in the following code.
+		tp := types.NewFieldType(mysql.TypeString)
+		tp.SetCharset(charset.CharsetBin)
+		tp.SetCollate(charset.CollationBin)
+		tp.AddFlag(mysql.BinaryFlag)
+		args[0] = BuildCastFunction(ctx, args[0], tp)
 	}
 	argTp := args[0].GetType().EvalType()
 	switch argTp {

pkg/expression/builtin_convert_charset.go

@@ -169,6 +169,7 @@ type builtinInternalFromBinarySig struct {
 func (b *builtinInternalFromBinarySig) Clone() builtinFunc {
 	newSig := &builtinInternalFromBinarySig{}
 	newSig.cloneFrom(&b.baseBuiltinFunc)
+	newSig.cannotConvertStringAsWarning = b.cannotConvertStringAsWarning
 	return newSig
 }
 

pkg/expression/expr_to_pb_test.go

@@ -1580,7 +1580,9 @@ func TestExprPushDownToTiKV(t *testing.T) {
 	require.Len(t, pushed, 0)
 	require.Len(t, remained, len(exprs))
 
-	// Test Conv function
+	// Test Conv function, `conv` function for a BIT column should not be pushed down for its special behavior which
+	// is only handled in TiDB currently.
+	// see issue: https://github.com/pingcap/tidb/issues/51877
 	exprs = exprs[:0]
 	function, err = NewFunction(mock.NewContext(), ast.Conv, types.NewFieldType(mysql.TypeString), stringColumn, intColumn, intColumn)
 	require.NoError(t, err)
@@ -1589,7 +1591,11 @@ func TestExprPushDownToTiKV(t *testing.T) {
 	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)
+	// when conv a column with type BIT, a cast function will be used to cast bit to a binary string
+	castTp := types.NewFieldType(mysql.TypeString)
+	castTp.SetCharset(charset.CharsetBin)
+	castTp.SetCollate(charset.CollationBin)
+	castByteAsStringFunc, err := NewFunction(mock.NewContext(), ast.Cast, castTp, byteColumn)
 	require.NoError(t, err)
 	function, err = NewFunction(mock.NewContext(), ast.Conv, types.NewFieldType(mysql.TypeString), castByteAsStringFunc, intColumn, intColumn)
 	require.NoError(t, err)

pkg/planner/core/integration_test.go

@@ -330,15 +330,15 @@ func TestBitColumnPushDown(t *testing.T) {
 	tk.MustQuery(sql).Check(testkit.Rows("A"))
 	tk.MustQuery(fmt.Sprintf("explain analyze %s", sql)).CheckAt([]int{0, 3, 6}, rows)
 
-	rows[1][2] = `eq(cast(test.t1.a, var_string(1)), "A")`
+	rows[1][2] = `eq(cast(from_binary(cast(test.t1.a, binary(1))), var_string(1)), "A")`
 	sql = "select a from t1 where cast(a as char)='A'"
 	tk.MustQuery(sql).Check(testkit.Rows("A"))
 	tk.MustQuery(fmt.Sprintf("explain analyze %s", sql)).CheckAt([]int{0, 3, 6}, rows)
 
 	tk.MustExec("insert into mysql.expr_pushdown_blacklist values('bit', 'tikv','');")
 	tk.MustExec("admin reload expr_pushdown_blacklist;")
 	rows = [][]any{
-		{"Selection_5", "root", `eq(cast(test.t1.a, var_string(1)), "A")`},
+		{"Selection_5", "root", `eq(cast(from_binary(cast(test.t1.a, binary(1))), var_string(1)), "A")`},
 		{"└─TableReader_7", "root", "data:TableFullScan_6"},
 		{"  └─TableFullScan_6", "cop[tikv]", "keep order:false, stats:pseudo"},
 	}

tests/integrationtest/r/expression/cast.result

@@ -178,3 +178,18 @@ id	update_user
 2	李四
 4	李四
 1	张三
+drop table if exists test;
+create table test(a bit(24));
+insert into test values('中');
+select a from test where '中' like convert(a, char);
+a
+中
+select a from test where false not like convert(a, char);
+a
+中
+select a from test where false like convert(a, char);
+a
+truncate table test;
+insert into test values(0xffffff);
+select a from test where false not like convert(a, char);
+Error 1105 (HY000): Cannot convert string '\xFF\xFF\xFF' from binary to utf8mb4

tests/integrationtest/t/expression/cast.test

@@ -113,3 +113,14 @@ insert into test values(3,'张三');
 insert into test values(4,'李四');
 select * from test order by cast(update_user as char) desc , id limit 3;
 
+# issue #56494, cast bit as char
+drop table if exists test;
+create table test(a bit(24));
+insert into test values('中');
+select a from test where '中' like convert(a, char);
+select a from test where false not like convert(a, char);
+select a from test where false like convert(a, char);
+truncate table test;
+insert into test values(0xffffff);
+-- error 1105
+select a from test where false not like convert(a, char);

tests/realtikvtest/pushdowntest/BUILD.bazel

@@ -0,0 +1,16 @@
+load("@io_bazel_rules_go//go:def.bzl", "go_test")
+
+go_test(
+    name = "pushdowntest_test",
+    timeout = "short",
+    srcs = [
+        "expr_test.go",
+        "main_test.go",
+    ],
+    flaky = True,
+    deps = [
+        "//pkg/testkit",
+        "//tests/realtikvtest",
+        "@com_github_stretchr_testify//require",
+    ],
+)

tests/realtikvtest/pushdowntest/expr_test.go

@@ -0,0 +1,36 @@
+// Copyright 2024 PingCAP, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package pushdowntest
+
+import (
+	"testing"
+
+	"github.com/pingcap/tidb/pkg/testkit"
+	"github.com/pingcap/tidb/tests/realtikvtest"
+	"github.com/stretchr/testify/require"
+)
+
+// TestBitCastInTiKV see issue: https://github.com/pingcap/tidb/issues/56494
+func TestBitCastInTiKV(t *testing.T) {
+	store := realtikvtest.CreateMockStoreAndSetup(t)
+	tk := testkit.NewTestKit(t, store)
+	tk.MustExec("use test")
+	tk.MustExec("drop table if exists t1")
+	defer tk.MustExec("drop table if exists t1")
+	tk.MustExec("create table t1(a bit(24))")
+	tk.MustExec("insert into t1 values(0xffffff)")
+	err := tk.QueryToErr("select a from t1 where false not like convert(a, char)")
+	require.EqualError(t, err, "[tikv:3854]Cannot convert string '\\xFF\\xFF\\xFF' from binary to utf8mb4")
+}

tests/realtikvtest/pushdowntest/main_test.go

@@ -0,0 +1,25 @@
+// Copyright 2024 PingCAP, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package pushdowntest
+
+import (
+	"testing"
+
+	"github.com/pingcap/tidb/tests/realtikvtest"
+)
+
+func TestMain(m *testing.M) {
+	realtikvtest.RunTestMain(m)
+}