Skip to content

Commit

Permalink
expression: fix tikv crash when bool like cast(bit as char) (pingca…
Browse files Browse the repository at this point in the history
  • Loading branch information
ti-chi-bot authored Feb 10, 2025
1 parent 7365a7b commit 6562afb
Show file tree
Hide file tree
Showing 6 changed files with 116 additions and 8 deletions.
18 changes: 14 additions & 4 deletions expression/builtin_cast.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
10 changes: 8 additions & 2 deletions expression/expr_to_pb_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down
19 changes: 17 additions & 2 deletions planner/core/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1405,15 +1405,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 = [][]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"},
}
Expand Down Expand Up @@ -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")
}
16 changes: 16 additions & 0 deletions tests/realtikvtest/pushdowntest/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -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",
],
)
36 changes: 36 additions & 0 deletions tests/realtikvtest/pushdowntest/expr_test.go
Original file line number Diff line number Diff line change
@@ -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")
}
25 changes: 25 additions & 0 deletions tests/realtikvtest/pushdowntest/main_test.go
Original file line number Diff line number Diff line change
@@ -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)
}

0 comments on commit 6562afb

Please sign in to comment.