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

builtin: logic op not compatibility with float #11587

Closed
wants to merge 3 commits into from
Closed

builtin: logic op not compatibility with float #11587

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ce04a0b
expression:add real sig for logicalAnd/Or
jingyugao Sep 11, 2019
f954b5a
code format
jingyugao Sep 11, 2019
4fa6acf
mod remove unused package
jingyugao Sep 11, 2019
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
104 changes: 90 additions & 14 deletions expression/builtin_op.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,10 @@ var (
)

var (
_ builtinFunc = &builtinLogicAndSig{}
_ builtinFunc = &builtinLogicOrSig{}
_ builtinFunc = &builtinIntLogicAndSig{}
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have to remove ast.And and ast.Or from canFuncBePushed since the implementation of these two functions are not consistent with TiKV now?
What's your opinion @XuHuaiyu ？

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 refer to this pr, #10498 . ShouId keep the builtinLogicAndSig signature compatible to tiKV?

_ builtinFunc = &builtinRealLogicAndSig{}
_ builtinFunc = &builtinIntLogicOrSig{}
_ builtinFunc = &builtinRealLogicOrSig{}
_ builtinFunc = &builtinLogicXorSig{}
_ builtinFunc = &builtinRealIsTrueSig{}
_ builtinFunc = &builtinDecimalIsTrueSig{}
Expand Down Expand Up @@ -67,24 +69,33 @@ func (c *logicAndFunctionClass) getFunction(ctx sessionctx.Context, args []Expre
if err != nil {
return nil, err
}
argTp0, argTp1 := args[0].GetType().EvalType(), args[1].GetType().EvalType()
if argTp0 == types.ETReal || argTp1 == types.ETReal || argTp0 == types.ETDecimal || argTp1 == types.ETDecimal {
bf := newBaseBuiltinFuncWithTp(ctx, args, types.ETInt, types.ETReal, types.ETReal)
sig := &builtinRealLogicAndSig{bf}
sig.setPbCode(tipb.ScalarFuncSig_RealLogicalAnd)
sig.tp.Flen = 1
return sig, nil
}

bf := newBaseBuiltinFuncWithTp(ctx, args, types.ETInt, types.ETInt, types.ETInt)
sig := &builtinLogicAndSig{bf}
sig.setPbCode(tipb.ScalarFuncSig_LogicalAnd)
sig := &builtinIntLogicAndSig{bf}
sig.setPbCode(tipb.ScalarFuncSig_IntLogicalAnd)
sig.tp.Flen = 1
return sig, nil
}

type builtinLogicAndSig struct {
type builtinIntLogicAndSig struct {
baseBuiltinFunc
}

func (b *builtinLogicAndSig) Clone() builtinFunc {
newSig := &builtinLogicAndSig{}
func (b *builtinIntLogicAndSig) Clone() builtinFunc {
newSig := &builtinIntLogicAndSig{}
newSig.cloneFrom(&b.baseBuiltinFunc)
return newSig
}

func (b *builtinLogicAndSig) evalInt(row chunk.Row) (int64, bool, error) {
func (b *builtinIntLogicAndSig) evalInt(row chunk.Row) (int64, bool, error) {
arg0, isNull0, err := b.args[0].EvalInt(b.ctx, row)
if err != nil || (!isNull0 && arg0 == 0) {
return 0, err != nil, err
Expand All @@ -99,6 +110,31 @@ func (b *builtinLogicAndSig) evalInt(row chunk.Row) (int64, bool, error) {
return 1, false, nil
}

type builtinRealLogicAndSig struct {
baseBuiltinFunc
}

func (b *builtinRealLogicAndSig) Clone() builtinFunc {
newSig := &builtinRealLogicAndSig{}
newSig.cloneFrom(&b.baseBuiltinFunc)
return newSig
}

func (b *builtinRealLogicAndSig) evalInt(row chunk.Row) (int64, bool, error) {
arg0, isNull0, err := b.args[0].EvalReal(b.ctx, row)
if err != nil || (!isNull0 && arg0 == 0) {
return 0, err != nil, err
}
arg1, isNull1, err := b.args[1].EvalReal(b.ctx, row)
if err != nil || (!isNull1 && arg1 == 0) {
return 0, err != nil, err
}
if isNull0 || isNull1 {
return 0, true, nil
}
return 1, false, nil
}

type logicOrFunctionClass struct {
baseFunctionClass
}
Expand All @@ -108,24 +144,64 @@ func (c *logicOrFunctionClass) getFunction(ctx sessionctx.Context, args []Expres
if err != nil {
return nil, err
}
argTp0, argTp1 := args[0].GetType().EvalType(), args[1].GetType().EvalType()
if argTp0 == types.ETReal || argTp1 == types.ETReal || argTp0 == types.ETDecimal || argTp1 == types.ETDecimal {
bf := newBaseBuiltinFuncWithTp(ctx, args, types.ETInt, types.ETReal, types.ETReal)
sig := &builtinRealLogicOrSig{bf}
sig.setPbCode(tipb.ScalarFuncSig_RealLogicalOr)
sig.tp.Flen = 1
return sig, nil
}

bf := newBaseBuiltinFuncWithTp(ctx, args, types.ETInt, types.ETInt, types.ETInt)
bf.tp.Flen = 1
sig := &builtinLogicOrSig{bf}
sig.setPbCode(tipb.ScalarFuncSig_LogicalOr)
sig := &builtinIntLogicOrSig{bf}
sig.setPbCode(tipb.ScalarFuncSig_IntLogicalOr)
return sig, nil
}

type builtinLogicOrSig struct {
type builtinRealLogicOrSig struct {
baseBuiltinFunc
}

func (b *builtinRealLogicOrSig) Clone() builtinFunc {
newSig := &builtinRealLogicOrSig{}
newSig.cloneFrom(&b.baseBuiltinFunc)
return newSig
}

func (b *builtinRealLogicOrSig) evalInt(row chunk.Row) (int64, bool, error) {
arg0, isNull0, err := b.args[0].EvalReal(b.ctx, row)
if err != nil {
return 0, true, err
}
if !isNull0 && arg0 != 0 {
return 1, false, nil
}
arg1, isNull1, err := b.args[1].EvalReal(b.ctx, row)
if err != nil {
return 0, true, err
}
if !isNull1 && arg1 != 0 {
return 1, false, nil
}
if isNull0 || isNull1 {
return 0, true, nil
}
return 0, false, nil
}

type builtinIntLogicOrSig struct {
baseBuiltinFunc
}

func (b *builtinLogicOrSig) Clone() builtinFunc {
newSig := &builtinLogicOrSig{}
func (b *builtinIntLogicOrSig) Clone() builtinFunc {
newSig := &builtinIntLogicOrSig{}
newSig.cloneFrom(&b.baseBuiltinFunc)
return newSig
}

func (b *builtinLogicOrSig) evalInt(row chunk.Row) (int64, bool, error) {
func (b *builtinIntLogicOrSig) evalInt(row chunk.Row) (int64, bool, error) {
arg0, isNull0, err := b.args[0].EvalInt(b.ctx, row)
if err != nil {
return 0, true, err
Expand Down
8 changes: 8 additions & 0 deletions expression/builtin_op_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,10 @@ func (s *testEvaluatorSuite) TestLogicAnd(c *C) {
{[]interface{}{0, nil}, 0, false, false},
{[]interface{}{nil, 0}, 0, false, false},
{[]interface{}{nil, 1}, 0, true, false},
{[]interface{}{0.1, 0.1}, 1, false, false},
{[]interface{}{0.1, 0}, 0, false, false},
{[]interface{}{0, 0.1}, 0, false, false},
{[]interface{}{nil, 0.1}, 0, true, false},

{[]interface{}{errors.New("must error"), 1}, 0, false, true},
}
Expand Down Expand Up @@ -305,6 +309,10 @@ func (s *testEvaluatorSuite) TestLogicOr(c *C) {
{[]interface{}{1, nil}, 1, false, false},
{[]interface{}{nil, 1}, 1, false, false},
{[]interface{}{nil, 0}, 0, true, false},
{[]interface{}{0.1, 0.1}, 1, false, false},
{[]interface{}{0.1, 0}, 1, false, false},
{[]interface{}{0, 0.1}, 1, false, false},
{[]interface{}{nil, 0.1}, 1, false, false},

{[]interface{}{errors.New("must error"), 1}, 0, false, true},
}
Expand Down
12 changes: 8 additions & 4 deletions expression/distsql_builtin.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,10 +305,14 @@ func getSignatureByPB(ctx sessionctx.Context, sigCode tipb.ScalarFuncSig, tp *ti
case tipb.ScalarFuncSig_FloorReal:
f = &builtinFloorRealSig{base}

case tipb.ScalarFuncSig_LogicalAnd:
f = &builtinLogicAndSig{base}
case tipb.ScalarFuncSig_LogicalOr:
f = &builtinLogicOrSig{base}
case tipb.ScalarFuncSig_IntLogicalAnd:
f = &builtinIntLogicAndSig{base}
case tipb.ScalarFuncSig_RealLogicalAnd:
f = &builtinRealLogicAndSig{base}
case tipb.ScalarFuncSig_IntLogicalOr:
f = &builtinIntLogicOrSig{base}
case tipb.ScalarFuncSig_RealLogicalOr:
f = &builtinRealLogicOrSig{base}
case tipb.ScalarFuncSig_LogicalXor:
f = &builtinLogicXorSig{base}
case tipb.ScalarFuncSig_BitAndSig:
Expand Down
4 changes: 2 additions & 2 deletions expression/distsql_builtin_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -453,12 +453,12 @@ func (s *testEvalSuite) TestEval(c *C) {
types.NewDecimalDatum(newMyDecimal(c, "1.23")),
},
{
scalarFunctionExpr(tipb.ScalarFuncSig_LogicalAnd,
scalarFunctionExpr(tipb.ScalarFuncSig_IntLogicalAnd,
toPBFieldType(newIntFieldType()), datumExpr(c, types.NewIntDatum(1)), datumExpr(c, types.NewIntDatum(1))),
types.NewIntDatum(1),
},
{
scalarFunctionExpr(tipb.ScalarFuncSig_LogicalOr,
scalarFunctionExpr(tipb.ScalarFuncSig_IntLogicalOr,
toPBFieldType(newIntFieldType()), datumExpr(c, types.NewIntDatum(1)), datumExpr(c, types.NewIntDatum(0))),
types.NewIntDatum(1),
},
Expand Down
2 changes: 1 addition & 1 deletion expression/expr_to_pb.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func ExpressionsToPB(sc *stmtctx.StatementContext, exprs []Expression, client kv
// Merge multiple converted pb expression into a CNF.
pbCNF = &tipb.Expr{
Tp: tipb.ExprType_ScalarFunc,
Sig: tipb.ScalarFuncSig_LogicalAnd,
Sig: tipb.ScalarFuncSig_IntLogicalAnd,
Children: []*tipb.Expr{pbCNF, pbExpr},
FieldType: ToPBFieldType(retTypeOfAnd),
}
Expand Down
2 changes: 1 addition & 1 deletion expression/util_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ func (s *testUtilSuite) TestClone(c *check.C) {
&builtinInetAtonSig{}, &builtinInetNtoaSig{}, &builtinInet6AtonSig{}, &builtinInet6NtoaSig{}, &builtinIsIPv4Sig{},
&builtinIsIPv4CompatSig{}, &builtinIsIPv4MappedSig{}, &builtinIsIPv6Sig{}, &builtinUUIDSig{}, &builtinNameConstIntSig{},
&builtinNameConstRealSig{}, &builtinNameConstDecimalSig{}, &builtinNameConstTimeSig{}, &builtinNameConstDurationSig{}, &builtinNameConstStringSig{},
&builtinNameConstJSONSig{}, &builtinLogicAndSig{}, &builtinLogicOrSig{}, &builtinLogicXorSig{}, &builtinRealIsTrueSig{},
&builtinNameConstJSONSig{}, &builtinIntLogicAndSig{}, &builtinRealLogicAndSig{}, &builtinIntLogicOrSig{}, &builtinRealLogicOrSig{}, &builtinLogicXorSig{}, &builtinRealIsTrueSig{},
&builtinDecimalIsTrueSig{}, &builtinIntIsTrueSig{}, &builtinRealIsFalseSig{}, &builtinDecimalIsFalseSig{}, &builtinIntIsFalseSig{},
&builtinUnaryMinusIntSig{}, &builtinDecimalIsNullSig{}, &builtinDurationIsNullSig{}, &builtinIntIsNullSig{}, &builtinRealIsNullSig{},
&builtinStringIsNullSig{}, &builtinTimeIsNullSig{}, &builtinUnaryNotRealSig{}, &builtinUnaryNotDecimalSig{}, &builtinUnaryNotIntSig{}, &builtinSleepSig{}, &builtinInIntSig{},
Expand Down
2 changes: 2 additions & 0 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,3 +75,5 @@ require (
sourcegraph.com/sourcegraph/appdash v0.0.0-20180531100431-4c381bd170b4
sourcegraph.com/sourcegraph/appdash-data v0.0.0-20151005221446-73f23eafcf67
)

replace github.com/pingcap/tipb => github.com/jingyugao/tipb v0.0.0-20190911070230-03bcdbb81a3f
4 changes: 2 additions & 2 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,8 @@ github.com/grpc-ecosystem/grpc-gateway v1.5.1/go.mod h1:RSKVYQBd5MCa4OVpNdGskqpg
github.com/hpcloud/tail v1.0.0 h1:nfCOvKYfkgYP8hkirhJocXT2+zOD8yUNjXaWfTlyFKI=
github.com/hpcloud/tail v1.0.0/go.mod h1:ab1qPbhIpdTxEkNHXyeSf5vhxWSCs/tWer42PpOxQnU=
github.com/inconshreveable/mousetrap v1.0.0/go.mod h1:PxqpIevigyE2G7u3NXJIT2ANytuPF1OarO4DADm73n8=
github.com/jingyugao/tipb v0.0.0-20190911070230-03bcdbb81a3f h1:mtQuOG0TM05qjRfmOAm5nMzo2igGZHW99faxOKfwFNw=
github.com/jingyugao/tipb v0.0.0-20190911070230-03bcdbb81a3f/go.mod h1:EBxLpOvivm5T+SDsuT+icoNOaQGLvz2kBmMfKzhmIJ4=
github.com/jonboulle/clockwork v0.1.0 h1:VKV+ZcuP6l3yW9doeqz6ziZGgcynBVQO+obU0+0hcPo=
github.com/jonboulle/clockwork v0.1.0/go.mod h1:Ii8DK3G1RaLaWxj9trq07+26W01tbo22gdxWY5EU2bo=
github.com/json-iterator/go v1.1.6 h1:MrUvLMLTMxbqFJ9kzlvat/rYZqZnW3u4wkLzWTaFwKs=
Expand Down Expand Up @@ -173,8 +175,6 @@ github.com/pingcap/pd v0.0.0-20190712044914-75a1f9f3062b h1:oS9PftxQqgcRouKhhdaB
github.com/pingcap/pd v0.0.0-20190712044914-75a1f9f3062b/go.mod h1:3DlDlFT7EF64A1bmb/tulZb6wbPSagm5G4p1AlhaEDs=
github.com/pingcap/tidb-tools v2.1.3-0.20190321065848-1e8b48f5c168+incompatible h1:MkWCxgZpJBgY2f4HtwWMMFzSBb3+JPzeJgF3VrXE/bU=
github.com/pingcap/tidb-tools v2.1.3-0.20190321065848-1e8b48f5c168+incompatible/go.mod h1:XGdcy9+yqlDSEMTpOXnwf3hiTeqrV6MN/u1se9N8yIM=
github.com/pingcap/tipb v0.0.0-20190806070524-16909e03435e h1:H7meq8QPmWGImOkHTQYAWw82zwIqndJaCDPVUknOHbM=
github.com/pingcap/tipb v0.0.0-20190806070524-16909e03435e/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