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.

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

expression: fix wrong result of Not/IsTrue/IsFalse functions (#10498) #15916

Merged
merged 4 commits into from
Apr 1, 2020
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
95 changes: 82 additions & 13 deletions expression/builtin_op.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,9 @@ var (
_ builtinFunc = &builtinRealIsNullSig{}
_ builtinFunc = &builtinStringIsNullSig{}
_ builtinFunc = &builtinTimeIsNullSig{}
_ builtinFunc = &builtinUnaryNotSig{}
_ builtinFunc = &builtinUnaryNotRealSig{}
_ builtinFunc = &builtinUnaryNotDecimalSig{}
_ builtinFunc = &builtinUnaryNotIntSig{}
)

type logicAndFunctionClass struct {
Expand Down Expand Up @@ -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)
Expand All @@ -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 {
Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
18 changes: 18 additions & 0 deletions expression/builtin_op_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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,
Expand Down
8 changes: 6 additions & 2 deletions expression/distsql_builtin.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down