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: open CAST push down switcher default #12652

Merged
merged 31 commits into from Nov 7, 2019
Merged
Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
8e27d6b
expression: open CAST push down switcher default
lonng Oct 12, 2019
c275ef1
fix plan explain diff after cop cast push down
H-ZeX Oct 14, 2019
c6ebe83
Merge branch 'master' of https://github.com/pingcap/tidb into cast-pu…
H-ZeX Oct 16, 2019
3d31c27
fix ci failed
H-ZeX Oct 16, 2019
c33b5fa
fix ci failed
H-ZeX Oct 16, 2019
5f09713
fix ci failed
H-ZeX Oct 16, 2019
62639e8
go mod tidy
lonng Oct 16, 2019
3285154
Merge remote-tracking branch 'origin/master' into cast-push-down
lonng Oct 16, 2019
d98dfc4
Merge remote-tracking branch 'origin/master' into cast-push-down
lonng Oct 16, 2019
fec2b71
tiny tweak logic
lonng Oct 16, 2019
4432ba2
Merge branch 'master' into cast-push-down
lonng Oct 16, 2019
5ab6317
Merge branch 'master' into cast-push-down
H-ZeX Oct 16, 2019
3746056
fix decimal evaluation precesion issue
lonng Oct 17, 2019
8755851
Merge branch 'master' into cast-push-down
lonng Oct 17, 2019
799bd92
fix the CI failure
lonng Oct 17, 2019
7c2d019
Merge remote-tracking branch 'origin/master' into cast-push-down
lonng Oct 17, 2019
df9f35f
revert some changes
lonng Oct 17, 2019
7afdfba
Merge remote-tracking branch 'origin/master' into cast-push-down
lonng Oct 17, 2019
677f3c5
Merge branch 'master' into cast-push-down
lonng Oct 18, 2019
9b37da7
Merge branch 'master' into cast-push-down
lonng Oct 18, 2019
7607ac5
Merge branch 'master' into cast-push-down
lonng Oct 18, 2019
57b24be
Merge branch 'master' into cast-push-down
lonng Nov 5, 2019
9c620a7
Merge branch 'master' into cast-push-down
lonng Nov 5, 2019
6d0d3b2
Merge remote-tracking branch 'origin/master' into cast-push-down
lonng Nov 6, 2019
9acf7ad
address comment
lonng Nov 6, 2019
e606ac3
fix imports
lonng Nov 6, 2019
e46be5c
Merge branch 'master' into cast-push-down
lonng Nov 7, 2019
daf7a02
Merge branch 'master' into cast-push-down
lonng Nov 7, 2019
9fc8390
Merge branch 'master' into cast-push-down
lonng Nov 7, 2019
48d4832
Merge branch 'master' into cast-push-down
lonng Nov 7, 2019
a04215c
Merge branch 'master' into cast-push-down
lonng Nov 7, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 4 additions & 4 deletions cmd/explaintest/r/explain_easy.result
Expand Up @@ -466,10 +466,10 @@ insert tb values ('1');
explain select * from ta where a = 1;
id count task operator info
Projection_5 8000.00 root Column#1
└─Selection_6 8000.00 root eq(cast(Column#1), 1)
└─UnionScan_7 10000.00 root eq(cast(Column#1), 1)
└─TableReader_9 10000.00 root data:TableScan_8
└─TableScan_8 10000.00 cop[tikv] table:ta, range:[-inf,+inf], keep order:false, stats:pseudo
└─UnionScan_6 8000.00 root eq(cast(Column#1), 1)
└─TableReader_9 8000.00 root data:Selection_8
└─Selection_8 8000.00 cop[tikv] eq(cast(Column#1), 1)
└─TableScan_7 10000.00 cop[tikv] table:ta, range:[-inf,+inf], keep order:false, stats:pseudo
rollback;
drop table if exists t1, t2;
create table t1(a int, b int, c int, primary key(a, b));
Expand Down
2 changes: 1 addition & 1 deletion expression/aggregation/aggregation.go
Expand Up @@ -153,7 +153,7 @@ func (af *aggFunction) updateSum(sc *stmtctx.StatementContext, evalCtx *AggEvalu
return nil
}
}
evalCtx.Value, err = calculateSum(sc, evalCtx.Value, value)
evalCtx.Value, err = calculateSum(sc, evalCtx.Value, value, a.GetType())
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion expression/aggregation/avg.go
Expand Up @@ -35,7 +35,7 @@ func (af *avgFunction) updateAvg(sc *stmtctx.StatementContext, evalCtx *AggEvalu
if value.IsNull() {
return nil
}
evalCtx.Value, err = calculateSum(sc, evalCtx.Value, value)
evalCtx.Value, err = calculateSum(sc, evalCtx.Value, value, a.GetType())
if err != nil {
return err
}
Expand Down
20 changes: 18 additions & 2 deletions expression/aggregation/util.go
Expand Up @@ -15,6 +15,7 @@ package aggregation

import (
"github.com/pingcap/errors"
"github.com/pingcap/parser/mysql"
"github.com/pingcap/tidb/sessionctx/stmtctx"
"github.com/pingcap/tidb/types"
"github.com/pingcap/tidb/util/codec"
Expand Down Expand Up @@ -54,7 +55,7 @@ func (d *distinctChecker) Check(values []types.Datum) (bool, error) {
}

// calculateSum adds v to sum.
func calculateSum(sc *stmtctx.StatementContext, sum, v types.Datum) (data types.Datum, err error) {
func calculateSum(sc *stmtctx.StatementContext, sum, v types.Datum, retType *types.FieldType) (data types.Datum, err error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the relationship between pushdown CAST and calculateSum? Are there any test cases for sum?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are many test cases that will cover the sum (query with sum/avg aggregation functions). The is no direct relationship with CAST and sum and I modify it because of the previous implementation does not round appropriately (this BUG is detected by this PR).

// for avg and sum calculation
// avg and sum use decimal for integer and decimal type, use float for others
// see https://dev.mysql.com/doc/refman/5.7/en/group-by-functions.html
Expand All @@ -68,7 +69,22 @@ func calculateSum(sc *stmtctx.StatementContext, sum, v types.Datum) (data types.
data = types.NewDecimalDatum(d)
}
case types.KindMysqlDecimal:
data = types.CloneDatum(v)
dec := v.GetMysqlDecimal()
frac := retType.Decimal
if frac == types.UnspecifiedLength {
frac = mysql.MaxDecimalScale
}
if int(dec.GetDigitsFrac()) > frac {
to := new(types.MyDecimal)
err := dec.Round(to, frac, types.ModeHalfEven)
data = types.Datum{}
data.SetMysqlDecimal(to)
if err != nil {
return data, err
Copy link
Contributor

Choose a reason for hiding this comment

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

Why put return after creating data? If err != nil, to is not even integral.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, we should return the data even though overflow occurred (the caller should handle it in a proper way if the err is overflow or truncated).

}
} else {
data = types.CloneDatum(v)
}
default:
var f float64
f, err = v.ToFloat64(sc)
Expand Down
23 changes: 21 additions & 2 deletions expression/expr_to_pb.go
Expand Up @@ -352,12 +352,31 @@ func (pc PbConverter) canFuncBePushed(sf *ScalarFunction) bool {

// date functions.
ast.DateFormat:
_, disallowPushdown := DefaultExprPushdownBlacklist.Load().(map[string]struct{})[sf.FuncName.L]
return true && !disallowPushdown
return isPushdownEnabled(sf.FuncName.L)

case ast.Cast:
// Disable time related cast function temporarily
switch sf.Function.PbCode() {
case tipb.ScalarFuncSig_CastIntAsTime,
tipb.ScalarFuncSig_CastRealAsTime,
tipb.ScalarFuncSig_CastDecimalAsTime,
tipb.ScalarFuncSig_CastStringAsTime,
tipb.ScalarFuncSig_CastDurationAsTime,
tipb.ScalarFuncSig_CastJsonAsTime,
tipb.ScalarFuncSig_CastTimeAsTime:
return false
default:
return isPushdownEnabled(sf.FuncName.L)
}
}
return false
}

func isPushdownEnabled(name string) bool {
_, disallowPushdown := DefaultExprPushdownBlacklist.Load().(map[string]struct{})[name]
return !disallowPushdown
}

// DefaultExprPushdownBlacklist indicates the expressions which can not be pushed down to TiKV.
var DefaultExprPushdownBlacklist *atomic.Value

Expand Down
36 changes: 20 additions & 16 deletions expression/expr_to_pb_test.go
Expand Up @@ -266,6 +266,7 @@ func (s *testEvaluatorSuite) TestCompareFunc2Pb(c *C) {

func (s *testEvaluatorSuite) TestLikeFunc2Pb(c *C) {
var likeFuncs []Expression
var expects []string
sc := new(stmtctx.StatementContext)
client := new(mock.Client)

Expand All @@ -280,19 +281,21 @@ func (s *testEvaluatorSuite) TestLikeFunc2Pb(c *C) {
}
ctx := mock.NewContext()
retTp = types.NewFieldType(mysql.TypeUnspecified)
fc, err := NewFunction(ctx, ast.Like, retTp, args[0], args[1], args[3])
fc1, err := NewFunction(ctx, ast.Like, retTp, args[0], args[1], args[3])
c.Assert(err, IsNil)
likeFuncs = append(likeFuncs, fc)
expects = append(expects, `{"tp":10000,"children":[{"tp":5,"val":"c3RyaW5n","sig":0,"field_type":{"tp":254,"flag":0,"flen":-1,"decimal":-1,"collate":83,"charset":"utf8"}},{"tp":5,"val":"cGF0dGVybg==","sig":0,"field_type":{"tp":254,"flag":0,"flen":-1,"decimal":-1,"collate":83,"charset":"utf8"}},{"tp":10000,"val":"CAA=","children":[{"tp":5,"val":"XA==","sig":0,"field_type":{"tp":254,"flag":0,"flen":-1,"decimal":-1,"collate":83,"charset":"utf8"}}],"sig":30,"field_type":{"tp":8,"flag":128,"flen":-1,"decimal":0,"collate":63,"charset":"binary"}}],"sig":4310,"field_type":{"tp":8,"flag":128,"flen":1,"decimal":0,"collate":63,"charset":"binary"}}`)
likeFuncs = append(likeFuncs, fc1)

fc, err = NewFunction(ctx, ast.Like, retTp, args[0], args[2], args[3])
fc2, err := NewFunction(ctx, ast.Like, retTp, args[0], args[2], args[3])
c.Assert(err, IsNil)
likeFuncs = append(likeFuncs, fc)
expects = append(expects, `{"tp":10000,"children":[{"tp":5,"val":"c3RyaW5n","sig":0,"field_type":{"tp":254,"flag":0,"flen":-1,"decimal":-1,"collate":83,"charset":"utf8"}},{"tp":5,"val":"JWFiYyU=","sig":0,"field_type":{"tp":254,"flag":0,"flen":-1,"decimal":-1,"collate":83,"charset":"utf8"}},{"tp":10000,"val":"CAA=","children":[{"tp":5,"val":"XA==","sig":0,"field_type":{"tp":254,"flag":0,"flen":-1,"decimal":-1,"collate":83,"charset":"utf8"}}],"sig":30,"field_type":{"tp":8,"flag":128,"flen":-1,"decimal":0,"collate":63,"charset":"binary"}}],"sig":4310,"field_type":{"tp":8,"flag":128,"flen":1,"decimal":0,"collate":63,"charset":"binary"}}`)
likeFuncs = append(likeFuncs, fc2)

pbExprs := ExpressionsToPBList(sc, likeFuncs, client)
for _, pbExpr := range pbExprs {
for i, pbExpr := range pbExprs {
js, err := json.Marshal(pbExpr)
c.Assert(err, IsNil)
c.Assert(string(js), Equals, "null")
c.Assert(string(js), Equals, expects[i])
Copy link
Contributor

Choose a reason for hiding this comment

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

Why does this result change?

}
}

Expand Down Expand Up @@ -428,28 +431,29 @@ func (s *testEvaluatorSerialSuites) TestPushDownSwitcher(c *C) {
cases := []struct {
name string
sig tipb.ScalarFuncSig
argCnt int
enable bool
}{
{ast.And, tipb.ScalarFuncSig_BitAndSig, true},
{ast.Or, tipb.ScalarFuncSig_BitOrSig, false},
{ast.UnaryNot, tipb.ScalarFuncSig_UnaryNotInt, true},
{ast.And, tipb.ScalarFuncSig_BitAndSig, 2, true},
{ast.Or, tipb.ScalarFuncSig_BitOrSig, 2, false},
{ast.UnaryNot, tipb.ScalarFuncSig_UnaryNotInt, 1, true},
}
var enabled []string
for i, funcName := range cases {
args := []Expression{dg.genColumn(mysql.TypeLong, 1)}
if i+1 < len(cases) {
args = append(args, dg.genColumn(mysql.TypeLong, 2))
for _, cs := range cases {
args := make([]Expression, 0, cs.argCnt)
for i := 0; i < cs.argCnt; i++ {
args = append(args, dg.genColumn(mysql.TypeLong, int64(i)))
}
fc, err := NewFunction(
mock.NewContext(),
funcName.name,
cs.name,
types.NewFieldType(mysql.TypeUnspecified),
args...,
)
c.Assert(err, IsNil)
funcs = append(funcs, fc)
if funcName.enable {
enabled = append(enabled, funcName.name)
if cs.enable {
enabled = append(enabled, cs.name)
}
}

Expand Down
2 changes: 1 addition & 1 deletion planner/core/logical_plan_test.go
Expand Up @@ -86,7 +86,7 @@ func (s *testPlanSuite) TestPredicatePushDown(c *C) {
},
{
sql: "select * from t t1, t t2 where t1.a = t2.b and t2.b > 0 and t1.a = t1.c and t1.d like 'abc' and t2.d = t1.d",
best: "Join{DataScan(t1)->Sel([like(cast(Column#4), abc, 92)])->DataScan(t2)->Sel([like(cast(Column#16), abc, 92)])}(Column#1,Column#14)(Column#4,Column#16)->Projection",
best: "Join{DataScan(t1)->DataScan(t2)}(Column#1,Column#14)(Column#4,Column#16)->Projection",
},
{
sql: "select * from t ta join t tb on ta.d = tb.d and ta.d > 1 where tb.a = 0",
Expand Down
2 changes: 1 addition & 1 deletion planner/core/testdata/plan_suite_out.json
Expand Up @@ -1012,7 +1012,7 @@
},
{
"SQL": "select a from t where c = 'hanfei'",
"Best": "IndexReader(Index(t.c_d_e)[[NULL,+inf]])->Sel([eq(cast(Column#3), cast(hanfei))])->Projection"
"Best": "IndexReader(Index(t.c_d_e)[[NULL,+inf]]->Sel([eq(cast(Column#3), cast(hanfei))]))->Projection"
}
]
},
Expand Down