From 6562afb475a05cbd0b1b09a9e8333f0ff3ffacbe Mon Sep 17 00:00:00 2001 From: Ti Chi Robot Date: Mon, 10 Feb 2025 10:13:25 +0800 Subject: [PATCH] expression: fix tikv crash when `bool like cast(bit as char)` (#57484) (#59316) close pingcap/tidb#56494 --- expression/builtin_cast.go | 18 +++++++--- expression/expr_to_pb_test.go | 10 ++++-- planner/core/integration_test.go | 19 +++++++++-- tests/realtikvtest/pushdowntest/BUILD.bazel | 16 +++++++++ tests/realtikvtest/pushdowntest/expr_test.go | 36 ++++++++++++++++++++ tests/realtikvtest/pushdowntest/main_test.go | 25 ++++++++++++++ 6 files changed, 116 insertions(+), 8 deletions(-) create mode 100644 tests/realtikvtest/pushdowntest/BUILD.bazel create mode 100644 tests/realtikvtest/pushdowntest/expr_test.go create mode 100644 tests/realtikvtest/pushdowntest/main_test.go diff --git a/expression/builtin_cast.go b/expression/builtin_cast.go index 763790399820d..45f99d8e62027 100644 --- a/expression/builtin_cast.go +++ b/expression/builtin_cast.go @@ -284,10 +284,20 @@ func (c *castAsStringFunctionClass) getFunction(ctx sessionctx.Context, args []E 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 { diff --git a/expression/expr_to_pb_test.go b/expression/expr_to_pb_test.go index 81f5434d3910a..4491b16dd7f1b 100644 --- a/expression/expr_to_pb_test.go +++ b/expression/expr_to_pb_test.go @@ -1472,7 +1472,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) @@ -1481,7 +1483,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) diff --git a/planner/core/integration_test.go b/planner/core/integration_test.go index b090149021ac7..0ffb33b96093f 100644 --- a/planner/core/integration_test.go +++ b/planner/core/integration_test.go @@ -1405,7 +1405,7 @@ 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) @@ -1413,7 +1413,7 @@ func TestBitColumnPushDown(t *testing.T) { tk.MustExec("insert into mysql.expr_pushdown_blacklist values('bit', 'tikv','');") tk.MustExec("admin reload expr_pushdown_blacklist;") rows = [][]interface{}{ - {"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"}, } @@ -5518,3 +5518,18 @@ func TestNestedVirtualGeneratedColumnUpdate(t *testing.T) { tk.MustExec("UPDATE test1 SET col7 = '{\"col10\":\"DDDDD\",\"col9\":[\"abcdefg\"]}';\n") tk.MustExec("DELETE FROM test1 WHERE col1 < 0;\n") } + +func TestCastBitAsString(t *testing.T) { + store := testkit.CreateMockStore(t) + tk := testkit.NewTestKit(t, store) + tk.MustExec("use test") + tk.MustExec("create table test(a bit(24))") + tk.MustExec("insert into test values('中')") + tk.MustQuery("select a from test where '中' like convert(a, char)").Check(testkit.Rows("中")) + tk.MustQuery("select a from test where false not like convert(a, char)").Check(testkit.Rows("中")) + tk.MustQuery("select a from test where false like convert(a, char)").Check(testkit.Rows()) + tk.MustExec("truncate table test") + tk.MustExec("insert into test values(0xffffff)") + err := tk.QueryToErr("select a from test where false not like convert(a, char)") + require.EqualError(t, err, "[tikv:3854]Cannot convert string '\\xFF\\xFF\\xFF' from binary to utf8mb4") +} diff --git a/tests/realtikvtest/pushdowntest/BUILD.bazel b/tests/realtikvtest/pushdowntest/BUILD.bazel new file mode 100644 index 0000000000000..498618fdaa4ad --- /dev/null +++ b/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 = [ + "//testkit", + "//tests/realtikvtest", + "@com_github_stretchr_testify//require", + ], +) diff --git a/tests/realtikvtest/pushdowntest/expr_test.go b/tests/realtikvtest/pushdowntest/expr_test.go new file mode 100644 index 0000000000000..207a4588972c9 --- /dev/null +++ b/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/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") +} diff --git a/tests/realtikvtest/pushdowntest/main_test.go b/tests/realtikvtest/pushdowntest/main_test.go new file mode 100644 index 0000000000000..fc34188dac29d --- /dev/null +++ b/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) +}