Skip to content

Commit

Permalink
*: fix panic when check null rejection for from_unixtime (pingcap#1…
Browse files Browse the repository at this point in the history
  • Loading branch information
zz-jason committed Oct 8, 2019
1 parent 7418da6 commit 72e3242
Show file tree
Hide file tree
Showing 5 changed files with 138 additions and 24 deletions.
38 changes: 21 additions & 17 deletions expression/builtin_time.go
Original file line number Diff line number Diff line change
Expand Up @@ -1571,26 +1571,30 @@ func (c *fromUnixTimeFunctionClass) getFunction(ctx sessionctx.Context, args []E
_, isArg0Con := args[0].(*Constant)
isArg0Str := args[0].GetType().EvalType() == types.ETString
bf := newBaseBuiltinFuncWithTp(ctx, args, retTp, argTps...)
if len(args) == 1 {
if isArg0Str {
bf.tp.Decimal = types.MaxFsp
} else if isArg0Con {
arg0, _, err1 := args[0].EvalDecimal(ctx, chunk.Row{})
if err1 != nil {
return sig, errors.Trace(err1)
}

if len(args) > 1 {
bf.tp.Flen = args[1].GetType().Flen
return &builtinFromUnixTime2ArgSig{bf}, nil
}

// Calculate the time fsp.
switch {
case isArg0Str:
bf.tp.Decimal = int(types.MaxFsp)
case isArg0Con:
arg0, arg0IsNull, err0 := args[0].EvalDecimal(ctx, chunk.Row{})
if err0 != nil {
return nil, err0
}

bf.tp.Decimal = int(types.MaxFsp)
if !arg0IsNull {
fsp := int(arg0.GetDigitsFrac())
if fsp > types.MaxFsp {
fsp = types.MaxFsp
}
bf.tp.Decimal = fsp
bf.tp.Decimal = mathutil.Min(fsp, int(types.MaxFsp))
}
sig = &builtinFromUnixTime1ArgSig{bf}
} else {
bf.tp.Flen = args[1].GetType().Flen
sig = &builtinFromUnixTime2ArgSig{bf}
}
return sig, nil

return &builtinFromUnixTime1ArgSig{bf}, nil
}

func evalFromUnixTime(ctx sessionctx.Context, fsp int, row chunk.Row, arg Expression) (res types.Time, isNull bool, err error) {
Expand Down
60 changes: 53 additions & 7 deletions planner/core/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,22 +15,44 @@ package core_test

import (
. "github.com/pingcap/check"
"github.com/pingcap/tidb/domain"
"github.com/pingcap/tidb/kv"
"github.com/pingcap/tidb/util/testkit"
"github.com/pingcap/tidb/util/testutil"
)

var _ = Suite(&testIntegrationSuite{})

type testIntegrationSuite struct {
testData testutil.TestData
store kv.Storage
dom *domain.Domain
}

func (s *testIntegrationSuite) TestShowSubquery(c *C) {
store, dom, err := newStoreWithBootstrap()
func (s *testIntegrationSuite) SetUpSuite(c *C) {
var err error
s.testData, err = testutil.LoadTestSuiteData("testdata", "integration_suite")
c.Assert(err, IsNil)
}

func (s *testIntegrationSuite) TearDownSuite(c *C) {
c.Assert(s.testData.GenerateOutputIfNeeded(), IsNil)
}

func (s *testIntegrationSuite) SetUpTest(c *C) {
var err error
s.store, s.dom, err = newStoreWithBootstrap()
c.Assert(err, IsNil)
}

func (s *testIntegrationSuite) TearDownTest(c *C) {
s.dom.Close()
err := s.store.Close()
c.Assert(err, IsNil)
tk := testkit.NewTestKit(c, store)
defer func() {
dom.Close()
store.Close()
}()
}

func (s *testIntegrationSuite) TestShowSubquery(c *C) {
tk := testkit.NewTestKit(c, s.store)
tk.MustExec("use test")
tk.MustExec("drop table if exists t")
tk.MustExec("create table t(a varchar(10), b int, c int)")
Expand Down Expand Up @@ -59,3 +81,27 @@ func (s *testIntegrationSuite) TestShowSubquery(c *C) {
"a varchar(10) YES <nil> ",
))
}

func (s *testIntegrationSuite) TestIsFromUnixtimeNullRejective(c *C) {
tk := testkit.NewTestKit(c, s.store)
tk.MustExec("use test")
tk.MustExec(`drop table if exists t;`)
tk.MustExec(`create table t(a bigint, b bigint);`)
s.runTestsWithTestData("TestIsFromUnixtimeNullRejective", tk, c)
}

func (s *testIntegrationSuite) runTestsWithTestData(caseName string, tk *testkit.TestKit, c *C) {
var input []string
var output []struct {
SQL string
Plan []string
}
s.testData.GetTestCasesByName(caseName, c, &input, &output)
for i, tt := range input {
s.testData.OnRecord(func() {
output[i].SQL = tt
output[i].Plan = s.testData.ConvertRowsToStrings(tk.MustQuery(tt).Rows())
})
tk.MustQuery(tt).Check(testkit.Rows(output[i].Plan...))
}
}
22 changes: 22 additions & 0 deletions planner/core/testdata/integration_suite_in.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
[
{
"name": "TestPushLimitDownIndexLookUpReader",
"cases": [
// Limit should be pushed down into IndexLookUpReader, row count of IndexLookUpReader and TableScan should be 1.00.
"explain select * from tbl use index(idx_b_c) where b > 1 limit 2,1",
// Projection atop IndexLookUpReader, Limit should be pushed down into IndexLookUpReader, and Projection should have row count 1.00 as well.
"explain select * from tbl use index(idx_b_c) where b > 1 order by b desc limit 2,1",
// Limit should be pushed down into IndexLookUpReader when Selection on top of IndexScan.
"explain select * from tbl use index(idx_b_c) where b > 1 and c > 1 limit 2,1",
// Limit should NOT be pushed down into IndexLookUpReader when Selection on top of TableScan.
"explain select * from tbl use index(idx_b_c) where b > 1 and a > 1 limit 2,1"
]
},
{
"name": "TestIsFromUnixtimeNullRejective",
"cases": [
// fix #12385
"explain select * from t t1 left join t t2 on t1.a=t2.a where from_unixtime(t2.b);"
]
}
]
22 changes: 22 additions & 0 deletions planner/core/testdata/integration_suite_out.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
[
{
"Name": "TestPushLimitDownIndexLookUpReader",
"Cases": null
},
{
"Name": "TestIsFromUnixtimeNullRejective",
"Cases": [
{
"SQL": "explain select * from t t1 left join t t2 on t1.a=t2.a where from_unixtime(t2.b);",
"Plan": [
"HashLeftJoin_8 10000.00 root inner join, inner:Selection_12, equal:[eq(test.t1.a, test.t2.a)]",
"├─TableReader_11 10000.00 root data:TableScan_10",
"│ └─TableScan_10 10000.00 cop table:t1, range:[-inf,+inf], keep order:false, stats:pseudo",
"└─Selection_12 8000.00 root from_unixtime(cast(test.t2.b))",
" └─TableReader_14 10000.00 root data:TableScan_13",
" └─TableScan_13 10000.00 cop table:t2, range:[-inf,+inf], keep order:false, stats:pseudo"
]
}
]
}
]
20 changes: 20 additions & 0 deletions util/testutil/testutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,26 @@ func loadTestSuiteCases(filePath string) (res []testCases, err error) {
return res, err
}

// GetTestCasesByName gets the test cases for a test function by its name.
func (t *TestData) GetTestCasesByName(caseName string, c *check.C, in interface{}, out interface{}) {
casesIdx, ok := t.funcMap[caseName]
c.Assert(ok, check.IsTrue, check.Commentf("Must get test %s", caseName))
err := json.Unmarshal(*t.input[casesIdx].Cases, in)
c.Assert(err, check.IsNil)
if !record {
err = json.Unmarshal(*t.output[casesIdx].Cases, out)
c.Assert(err, check.IsNil)
} else {
// Init for generate output file.
inputLen := reflect.ValueOf(in).Elem().Len()
v := reflect.ValueOf(out).Elem()
if v.Kind() == reflect.Slice {
v.Set(reflect.MakeSlice(v.Type(), inputLen, inputLen))
}
}
t.output[casesIdx].decodedOut = out
}

// GetTestCases gets the test cases for a test function.
func (t *TestData) GetTestCases(c *check.C, in interface{}, out interface{}) {
// Extract caller's name.
Expand Down

0 comments on commit 72e3242

Please sign in to comment.