Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 14 additions & 4 deletions pkg/expression/builtin_cast.go
Original file line number Diff line number Diff line change
Expand Up @@ -300,10 +300,20 @@ func (c *castAsStringFunctionClass) getFunction(ctx BuildContext, args []Express
if err != nil {
return nil, err
}
if args[0].GetType(ctx.GetEvalCtx()).Hybrid() {
sig = &builtinCastStringAsStringSig{bf}
sig.setPbCode(tipb.ScalarFuncSig_CastStringAsString)
return sig, nil
if ft := args[0].GetType(ctx.GetEvalCtx()); ft.Hybrid() {
castBitAsUnBinary := ft.GetType() == mysql.TypeBit && c.tp.GetCharset() != charset.CharsetBin
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For other type that store binary string like Blob, will TiDB meet the same problem?

Copy link
Collaborator Author

@lcwangchao lcwangchao Nov 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, only the "hybrid" types will address this issue. Their "EvalType" may return ETInt, but when converting them to strings, we should be recognized as bytes instead.

I did not handle other hybrid types like enum and set in this PR, because in most time their should have a legal encoding and another reason is to introduce less logic change in this fix.

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(ctx.GetEvalCtx()).EvalType()
switch argTp {
Expand Down
1 change: 1 addition & 0 deletions pkg/expression/builtin_convert_charset.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@ type builtinInternalFromBinarySig struct {
func (b *builtinInternalFromBinarySig) Clone() builtinFunc {
newSig := &builtinInternalFromBinarySig{}
newSig.cloneFrom(&b.baseBuiltinFunc)
newSig.cannotConvertStringAsWarning = b.cannotConvertStringAsWarning
return newSig
}

Expand Down
10 changes: 8 additions & 2 deletions pkg/expression/expr_to_pb_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1581,7 +1581,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 @@ -1590,7 +1592,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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function can not be push down to TiKV because from_binary can not be push down to TiKV?

Copy link
Collaborator Author

@lcwangchao lcwangchao Nov 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, I think it's the issue: #51877 The tidb have some special logic for function conv. When converting a BIT type, it uses conv(cast(bit as char)) which uses cast to cast bit to string, but conv should also handles the string as a binary number instead of string...

See:

if x.FuncName.L == ast.Cast {
arg0 := x.GetArgs()[0]
if arg0.GetType(ctx).Hybrid() || IsBinaryLiteral(arg0) {
str, isNull, err = arg0.EvalString(ctx, row)
if isNull || err != nil {
return str, isNull, err
}
d := types.NewStringDatum(str)
str = d.GetBinaryLiteral().ToBitLiteralString(true)
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a comment in this test

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test is not related with this issue, but we should update it to set to casted type as bin collations to make the test pass.

require.NoError(t, err)
function, err = NewFunction(mock.NewContext(), ast.Conv, types.NewFieldType(mysql.TypeString), castByteAsStringFunc, intColumn, intColumn)
require.NoError(t, err)
Expand Down
4 changes: 2 additions & 2 deletions pkg/planner/core/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -309,15 +309,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"},
}
Expand Down
15 changes: 15 additions & 0 deletions tests/integrationtest/r/expression/cast.result
Original file line number Diff line number Diff line change
Expand Up @@ -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
11 changes: 11 additions & 0 deletions tests/integrationtest/t/expression/cast.test
Original file line number Diff line number Diff line change
Expand Up @@ -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);
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 = [
"//pkg/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/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")
}
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)
}