Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

expression: fix different types compare error #21338

Merged
merged 24 commits into from
Dec 25, 2020
Merged

expression: fix different types compare error #21338

Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
313ab85
fix different types compare error and query for bit
zhaoxugang Nov 26, 2020
0ba309c
fix test
zhaoxugang Nov 28, 2020
0664e07
fix test
zhaoxugang Nov 28, 2020
2431753
fix test
zhaoxugang Nov 28, 2020
6ed6dcf
revert undignedFlag of typebit and evalReal not convert to uint when …
zhaoxugang Nov 29, 2020
6968a16
calc flag of result from cast function
zhaoxugang Dec 1, 2020
849beff
fix
zhaoxugang Dec 1, 2020
904bf51
Merge branch 'master' into different-types-compare
bb7133 Dec 3, 2020
fd27373
revert change to check race test
zhaoxugang Dec 5, 2020
579cd43
commit change by step
zhaoxugang Dec 5, 2020
6fba459
commit change by step
zhaoxugang Dec 5, 2020
ba4cc45
add comment,fix bug
zhaoxugang Dec 14, 2020
596dd27
Merge remote-tracking branch 'origin/master' into different-types-com…
zhaoxugang Dec 15, 2020
cd6cea4
add test
zhaoxugang Dec 15, 2020
0e0a148
Merge branch 'master' into different-types-compare
zhaoxugang Dec 15, 2020
a29360c
when param type are decimal and enum retuen real type
zhaoxugang Dec 19, 2020
7a6c730
fix
zhaoxugang Dec 20, 2020
9fcd148
fix
zhaoxugang Dec 20, 2020
5d2d0c1
fix
zhaoxugang Dec 20, 2020
671ab34
revert
zhaoxugang Dec 20, 2020
521d416
remove bit compare suit
zhaoxugang Dec 22, 2020
8d0794c
Remove useless code
zhaoxugang Dec 23, 2020
a6352f1
Merge branch 'master' into different-types-compare
zhaoxugang Dec 24, 2020
794473e
Merge branch 'master' into different-types-compare
ti-srebot Dec 25, 2020
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
32 changes: 32 additions & 0 deletions expression/builtin.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,9 @@ func newBaseBuiltinFuncWithTp(ctx sessionctx.Context, funcName string, args []Ex
return baseBuiltinFunc{}, errors.New("unexpected nil session ctx")
}

orgArgs := make([]Expression, len(args))
copy(orgArgs, args)

for i := range args {
switch argTps[i] {
case types.ETInt:
Expand All @@ -167,6 +170,7 @@ func newBaseBuiltinFuncWithTp(ctx sessionctx.Context, funcName string, args []Ex
case types.ETJson:
args[i] = WrapWithCastAsJSON(ctx, args[i])
}
generateRetFlag(args[i], argTps[i], orgArgs)
}

if err = checkIllegalMixCollation(funcName, args, retType); err != nil {
Expand Down Expand Up @@ -258,6 +262,34 @@ func newBaseBuiltinFuncWithTp(ctx sessionctx.Context, funcName string, args []Ex
return bf, nil
}

func generateRetFlag(res Expression, evalType types.EvalType, args []Expression) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I do not understand the logic of this function.
Can you add some comment for it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

if len(args) < 2 {
return
}
var retType = res.GetType()
lhs, rhs := args[0], args[1]
if fun, ok := res.(*ScalarFunction); (ok && fun.FuncName.L == ast.Cast) || res.GetType().Tp == mysql.TypeBit {
if evalType == types.ETInt || evalType == types.ETReal {
lfg, rfg := args[0].GetType().Flag, args[1].GetType().Flag
// set signedFlag to 0 when bit convert explicitly or implicitly to int or real
Copy link
Contributor

Choose a reason for hiding this comment

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

I can't understand the logic. In the case, all evalType.flag don't contain UnsignedFlag, I think. Could you provide an example?

drop table if exists t;
create table t(a bit(64), b double);
insert into t values(-21172, -11623);
select * from t where a < b;

I have a try, only add builtin_cast.goL487 and builtin_cast_vec.goL111, it also works. Is there any negligence in what I thought?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed builtin.goL173,it not work,for this case i see the step:
1.Invoke GetAccurateCmpType() output ETReal
2.Invoke newBaseBuiltinFuncWithTp()
in this method the param a will produce the scalarFunction named cast,retFlag=160,arg's retFlag=32 by WrapWithCastAsReal()
so in builtin_cast_vec.goL111 the unsignedArgs0=true,the a will change to uint.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why the flag for column a is 32..? It doesn't has the unsigned flag when creating table.

Copy link
Contributor

Choose a reason for hiding this comment

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

If cast a as double explicitly, it works...

[tidb]>  select * from t where cast(a as double) < b;
+----------+--------+
| a        | b      |
+----------+--------+
| �������L        | -11623 |
+----------+--------+
1 row in set (5.925 sec)

Copy link
Contributor Author

@zhaoxugang zhaoxugang Dec 20, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

in ddl_api.goL537 invoke col.Flag |= mysql.UnsignedFlag when create table,
you mean i should't change retFlag in generateRetFlag, but cast bit to double?

Copy link
Contributor

Choose a reason for hiding this comment

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

I have a try in mysql8.0.22.. So the behavior in tidb is right? It maybe a bug in mysql8.0.21 and the previous version?

MySQL [test]> select version();
+-----------+
| version() |
+-----------+
| 8.0.22    |
+-----------+
1 row in set (0.000 sec)

MySQL [test]> drop table if exists t;
Query OK, 0 rows affected (0.006 sec)

MySQL [test]> create table t(a bit(64), b double);
Query OK, 0 rows affected (0.009 sec)

MySQL [test]> insert into t values(-21172, -11623);
Query OK, 1 row affected (0.002 sec)

MySQL [test]> select * from t where a < b;
Empty set (0.000 sec)

But in mysql8.0.21,

MySQL [test]> select version();
+-----------+
| version() |
+-----------+
| 8.0.21    |
+-----------+
1 row in set (0.000 sec)

MySQL [test]> drop table if exists t;
Query OK, 0 rows affected (0.007 sec)

MySQL [test]> create table t(a bit(64), b double);
Query OK, 0 rows affected (0.008 sec)

MySQL [test]> insert into t values(-21172, -11623);
Query OK, 1 row affected (0.001 sec)

MySQL [test]> select * from t where a < b;
+----------+--------+
| a        | b      |
+----------+--------+
| �������L        | -11623 |
+----------+--------+
1 row in set (0.000 sec)

if mysql.TypeBit == lhs.GetType().Tp || mysql.TypeBit == rhs.GetType().Tp {
retType.Flag |= mysql.UnsignedFlag
if mysql.TypeBit == rhs.GetType().Tp {
retType.Flag &= ^(mysql.UnsignedFlag ^ lfg)
} else {
retType.Flag &= ^(mysql.UnsignedFlag ^ rfg)
}
}
} else if evalType == types.ETDecimal && (mysql.TypeEnum == lhs.GetType().Tp || mysql.TypeEnum == rhs.GetType().Tp) {
// set Flen and Decimal as unspecified length(-1) when the enum type is compare with decimal type,avoid loss of data,
// like the enum'z'(26), wher Flen=2,Decimal=2 the enum=26.00 but when Flen= 1,Decimal=2 the enum=9
wshwsh12 marked this conversation as resolved.
Show resolved Hide resolved
if mysql.TypeNewDecimal == retType.Tp {
retType.Flen, retType.Decimal = types.UnspecifiedLength, types.UnspecifiedLength
}
}
}
}

// newBaseBuiltinFuncWithFieldType create BaseBuiltinFunc with FieldType charset and collation.
// do not check and compute collation.
func newBaseBuiltinFuncWithFieldType(ctx sessionctx.Context, tp *types.FieldType, args []Expression) (baseBuiltinFunc, error) {
Expand Down
2 changes: 1 addition & 1 deletion expression/builtin_cast.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -485,7 +485,7 @@ func (b *builtinCastIntAsRealSig) evalReal(row chunk.Row) (res float64, isNull b
if isNull || err != nil {
return res, isNull, err
}
if unsignedArgs0 := mysql.HasUnsignedFlag(b.args[0].GetType().Flag); !mysql.HasUnsignedFlag(b.tp.Flag) && !unsignedArgs0 {
if unsignedArgs0 := mysql.HasUnsignedFlag(b.args[0].GetType().Flag); !mysql.HasUnsignedFlag(b.tp.Flag) && (!unsignedArgs0 || mysql.TypeBit == b.args[0].GetType().Tp) {
res = float64(val)
} else if b.inUnion && !unsignedArgs0 && val < 0 {
// Round up to 0 if the value is negative but the expression eval type is unsigned in `UNION` statement
Expand Down
2 changes: 1 addition & 1 deletion expression/builtin_cast_vec.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ func (b *builtinCastIntAsRealSig) vecEvalReal(input *chunk.Chunk, result *chunk.
if result.IsNull(i) {
continue
}
if !hasUnsignedFlag0 && !hasUnsignedFlag1 {
if !hasUnsignedFlag0 && (!hasUnsignedFlag1 || b.args[0].GetType().Tp == mysql.TypeBit) {
rs[i] = float64(i64s[i])
} else if b.inUnion && !hasUnsignedFlag1 && i64s[i] < 0 {
// Round up to 0 if the value is negative but the expression eval type is unsigned in `UNION` statement
Expand Down
20 changes: 20 additions & 0 deletions expression/integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8293,6 +8293,26 @@ func (s *testIntegrationSuite) TestIssue12205(c *C) {
testkit.Rows("Warning 1292 Truncated incorrect time value: '18446744072635875000'"))
}

// for issue 20128
func (s *testIntegrationSerialSuite) TestIssue20128(c *C) {
collate.SetNewCollationEnabledForTest(true)
defer collate.SetNewCollationEnabledForTest(false)
wshwsh12 marked this conversation as resolved.
Show resolved Hide resolved
tk := testkit.NewTestKit(c, s.store)
tk.MustExec("use test")
tk.MustExec("drop table if exists t;")
tk.MustExec("create table t(b enum('a','b','c','d','e','f','g','h','i','j','k','l','m','n','o','p','q','r','s','t','u','v','w','x','y','z') DEFAULT NULL, c decimal(40,20));")
tk.MustExec("insert into t values('z', 19.18040000000000000000);")
tk.MustExec("insert into t values('z', 26.18040000000000000000);")
tk.MustExec("insert into t values('z', 25.18040000000000000000);")
tk.MustQuery("select * from t where t.b > t.c;").Check(testkit.Rows("z 19.18040000000000000000", "z 25.18040000000000000000"))
tk.MustQuery("select * from t where t.b < t.c;").Check(testkit.Rows("z 26.18040000000000000000"))

tk.MustExec("drop table if exists t;")
tk.MustExec("create table t(a bit(64), b double);")
tk.MustExec("insert into t values(-21172, -11623);")
tk.MustQuery("select b from t where a < b;").Check(testkit.Rows("-11623"))
}

func (s *testIntegrationSuite2) TestCastCoer(c *C) {
tk := testkit.NewTestKit(c, s.store)

Expand Down