From 03aeb718c30d803c22eb75fbe84c59076d65fb4c Mon Sep 17 00:00:00 2001 From: Kenan Yao Date: Tue, 20 Aug 2019 23:15:09 +0800 Subject: [PATCH 1/2] expression: fix wrong result of `Not`/`IsTrue`/`IsFalse` functions (#10498) --- expression/builtin_op.go | 95 ++++++++++++++++++++++++++++++----- expression/builtin_op_test.go | 18 +++++++ expression/distsql_builtin.go | 8 ++- 3 files changed, 106 insertions(+), 15 deletions(-) diff --git a/expression/builtin_op.go b/expression/builtin_op.go index cf6dc0a71a857..0654f7fefd401 100644 --- a/expression/builtin_op.go +++ b/expression/builtin_op.go @@ -53,7 +53,9 @@ var ( _ builtinFunc = &builtinRealIsNullSig{} _ builtinFunc = &builtinStringIsNullSig{} _ builtinFunc = &builtinTimeIsNullSig{} - _ builtinFunc = &builtinUnaryNotSig{} + _ builtinFunc = &builtinUnaryNotRealSig{} + _ builtinFunc = &builtinUnaryNotDecimalSig{} + _ builtinFunc = &builtinUnaryNotIntSig{} ) type logicAndFunctionClass struct { @@ -408,8 +410,10 @@ func (c *isTrueOrFalseFunctionClass) getFunction(ctx sessionctx.Context, args [] } argTp := args[0].GetType().EvalType() - if argTp != types.ETReal && argTp != types.ETDecimal { + if argTp == types.ETTimestamp || argTp == types.ETDatetime || argTp == types.ETDuration { argTp = types.ETInt + } else if argTp == types.ETJson || argTp == types.ETString { + argTp = types.ETReal } bf := newBaseBuiltinFuncWithTp(ctx, args, types.ETInt, argTp) @@ -428,6 +432,8 @@ func (c *isTrueOrFalseFunctionClass) getFunction(ctx sessionctx.Context, args [] case types.ETInt: sig = &builtinIntIsTrueSig{bf, c.keepNull} sig.setPbCode(tipb.ScalarFuncSig_IntIsTrue) + default: + return nil, errors.Errorf("unexpected types.EvalType %v", argTp) } case opcode.IsFalsity: switch argTp { @@ -440,6 +446,8 @@ func (c *isTrueOrFalseFunctionClass) getFunction(ctx sessionctx.Context, args [] case types.ETInt: sig = &builtinIntIsFalseSig{bf, c.keepNull} sig.setPbCode(tipb.ScalarFuncSig_IntIsFalse) + default: + return nil, errors.Errorf("unexpected types.EvalType %v", argTp) } } return sig, nil @@ -637,33 +645,94 @@ func (c *unaryNotFunctionClass) getFunction(ctx sessionctx.Context, args []Expre return nil, err } - bf := newBaseBuiltinFuncWithTp(ctx, args, types.ETInt, types.ETInt) + argTp := args[0].GetType().EvalType() + if argTp == types.ETTimestamp || argTp == types.ETDatetime || argTp == types.ETDuration { + argTp = types.ETInt + } else if argTp == types.ETJson || argTp == types.ETString { + argTp = types.ETReal + } + + bf := newBaseBuiltinFuncWithTp(ctx, args, types.ETInt, argTp) bf.tp.Flen = 1 - sig := &builtinUnaryNotSig{bf} - sig.setPbCode(tipb.ScalarFuncSig_UnaryNot) + var sig builtinFunc + switch argTp { + case types.ETReal: + sig = &builtinUnaryNotRealSig{bf} + sig.setPbCode(tipb.ScalarFuncSig_UnaryNotReal) + case types.ETDecimal: + sig = &builtinUnaryNotDecimalSig{bf} + sig.setPbCode(tipb.ScalarFuncSig_UnaryNotDecimal) + case types.ETInt: + sig = &builtinUnaryNotIntSig{bf} + sig.setPbCode(tipb.ScalarFuncSig_UnaryNotInt) + default: + return nil, errors.Errorf("unexpected types.EvalType %v", argTp) + } return sig, nil } -type builtinUnaryNotSig struct { +type builtinUnaryNotRealSig struct { baseBuiltinFunc } -func (b *builtinUnaryNotSig) Clone() builtinFunc { - newSig := &builtinUnaryNotSig{} +func (b *builtinUnaryNotRealSig) Clone() builtinFunc { + newSig := &builtinUnaryNotRealSig{} newSig.cloneFrom(&b.baseBuiltinFunc) return newSig } -func (b *builtinUnaryNotSig) evalInt(row chunk.Row) (int64, bool, error) { +func (b *builtinUnaryNotRealSig) evalInt(row chunk.Row) (int64, bool, error) { + arg, isNull, err := b.args[0].EvalReal(b.ctx, row) + if isNull || err != nil { + return 0, true, err + } + if arg == 0 { + return 1, false, nil + } + return 0, false, nil +} + +type builtinUnaryNotDecimalSig struct { + baseBuiltinFunc +} + +func (b *builtinUnaryNotDecimalSig) Clone() builtinFunc { + newSig := &builtinUnaryNotDecimalSig{} + newSig.cloneFrom(&b.baseBuiltinFunc) + return newSig +} + +func (b *builtinUnaryNotDecimalSig) evalInt(row chunk.Row) (int64, bool, error) { + arg, isNull, err := b.args[0].EvalDecimal(b.ctx, row) + if isNull || err != nil { + return 0, true, err + } + if arg.IsZero() { + return 1, false, nil + } + return 0, false, nil +} + +type builtinUnaryNotIntSig struct { + baseBuiltinFunc +} + +func (b *builtinUnaryNotIntSig) Clone() builtinFunc { + newSig := &builtinUnaryNotIntSig{} + newSig.cloneFrom(&b.baseBuiltinFunc) + return newSig +} + +func (b *builtinUnaryNotIntSig) evalInt(row chunk.Row) (int64, bool, error) { arg, isNull, err := b.args[0].EvalInt(b.ctx, row) if isNull || err != nil { - return 0, isNull, err + return 0, true, err } - if arg != 0 { - return 0, false, nil + if arg == 0 { + return 1, false, nil } - return 1, false, nil + return 0, false, nil } type unaryMinusFunctionClass struct { diff --git a/expression/builtin_op_test.go b/expression/builtin_op_test.go index a45d488dfff95..24e3f48946190 100644 --- a/expression/builtin_op_test.go +++ b/expression/builtin_op_test.go @@ -466,6 +466,9 @@ func (s *testEvaluatorSuite) TestUnaryNot(c *C) { {[]interface{}{123}, 0, false, false}, {[]interface{}{-123}, 0, false, false}, {[]interface{}{"123"}, 0, false, false}, + {[]interface{}{float64(0.3)}, 0, false, false}, + {[]interface{}{"0.3"}, 0, false, false}, + {[]interface{}{types.NewDecFromFloatForTest(0.3)}, 0, false, false}, {[]interface{}{nil}, 0, true, false}, {[]interface{}{errors.New("must error")}, 0, false, true}, @@ -539,6 +542,21 @@ func (s *testEvaluatorSuite) TestIsTrueOrFalse(c *C) { isTrue: 0, isFalse: 1, }, + { + args: []interface{}{"0.3"}, + isTrue: 1, + isFalse: 0, + }, + { + args: []interface{}{float64(0.3)}, + isTrue: 1, + isFalse: 0, + }, + { + args: []interface{}{types.NewDecFromFloatForTest(0.3)}, + isTrue: 1, + isFalse: 0, + }, { args: []interface{}{nil}, isTrue: 0, diff --git a/expression/distsql_builtin.go b/expression/distsql_builtin.go index 9ddf5a7130ad8..6208e4bca512d 100644 --- a/expression/distsql_builtin.go +++ b/expression/distsql_builtin.go @@ -320,8 +320,12 @@ func getSignatureByPB(ctx sessionctx.Context, sigCode tipb.ScalarFuncSig, tp *ti case tipb.ScalarFuncSig_BitNegSig: f = &builtinBitNegSig{base} - case tipb.ScalarFuncSig_UnaryNot: - f = &builtinUnaryNotSig{base} + case tipb.ScalarFuncSig_UnaryNotReal: + f = &builtinUnaryNotRealSig{base} + case tipb.ScalarFuncSig_UnaryNotDecimal: + f = &builtinUnaryNotDecimalSig{base} + case tipb.ScalarFuncSig_UnaryNotInt: + f = &builtinUnaryNotIntSig{base} case tipb.ScalarFuncSig_UnaryMinusInt: f = &builtinUnaryMinusIntSig{base} case tipb.ScalarFuncSig_UnaryMinusReal: From c32687d632fe11445bfaf2ccf4af6452640bba80 Mon Sep 17 00:00:00 2001 From: Kenan Yao Date: Wed, 1 Apr 2020 14:50:19 +0800 Subject: [PATCH 2/2] update tipb dependency --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 358a87e3a4318..b1946845a726a 100644 --- a/go.mod +++ b/go.mod @@ -41,7 +41,7 @@ require ( github.com/pingcap/parser v3.0.12-0.20200317072324-41ea4b21f5aa+incompatible github.com/pingcap/pd v1.1.0-beta.0.20191223090411-ea2b748f6ee2 github.com/pingcap/tidb-tools v3.0.6-0.20191119150227-ff0a3c6e5763+incompatible - github.com/pingcap/tipb v0.0.0-20191120045257-1b9900292ab6 + github.com/pingcap/tipb v0.0.0-20200401051346-bec3080a5428 github.com/prometheus/client_golang v0.9.0 github.com/prometheus/client_model v0.0.0-20180712105110-5c3871d89910 github.com/prometheus/common v0.0.0-20181020173914-7e9e6cabbd39 // indirect diff --git a/go.sum b/go.sum index 8fa6663d66709..ab7f24759a579 100644 --- a/go.sum +++ b/go.sum @@ -162,8 +162,8 @@ github.com/pingcap/pd v1.1.0-beta.0.20191223090411-ea2b748f6ee2 h1:NL23b8tsg6M1Q github.com/pingcap/pd v1.1.0-beta.0.20191223090411-ea2b748f6ee2/go.mod h1:b4gaAPSxaVVtaB+EHamV4Nsv8JmTdjlw0cTKmp4+dRQ= github.com/pingcap/tidb-tools v3.0.6-0.20191119150227-ff0a3c6e5763+incompatible h1:I8HirWsu1MZp6t9G/g8yKCEjJJxtHooKakEgccvdJ4M= github.com/pingcap/tidb-tools v3.0.6-0.20191119150227-ff0a3c6e5763+incompatible/go.mod h1:XGdcy9+yqlDSEMTpOXnwf3hiTeqrV6MN/u1se9N8yIM= -github.com/pingcap/tipb v0.0.0-20191120045257-1b9900292ab6 h1:HPgqtaqIFIZXTvQNiZoJ9Y79AXz3pmDpYFL28KraTKE= -github.com/pingcap/tipb v0.0.0-20191120045257-1b9900292ab6/go.mod h1:RtkHW8WbcNxj8lsbzjaILci01CtYnYbIkQhjyZWrWVI= +github.com/pingcap/tipb v0.0.0-20200401051346-bec3080a5428 h1:u2eGnp74AlgviPKRcf49MESRp8RFAhURomJsL2XYC6o= +github.com/pingcap/tipb v0.0.0-20200401051346-bec3080a5428/go.mod h1:RtkHW8WbcNxj8lsbzjaILci01CtYnYbIkQhjyZWrWVI= github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pkg/errors v0.8.1 h1:iURUrRGxPUNPdy5/HRSm+Yj6okJ6UtLINN0Q9M4+h3I= github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=