Skip to content

Commit

Permalink
*: fix panic when check null rejection for from_unixtime (#12413)
Browse files Browse the repository at this point in the history
  • Loading branch information
zz-jason authored and sre-bot committed Sep 30, 2019
1 parent 365e7bf commit 01dce81
Show file tree
Hide file tree
Showing 5 changed files with 111 additions and 45 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 = int(types.MaxFsp)
} else if isArg0Con {
arg0, _, err1 := args[0].EvalDecimal(ctx, chunk.Row{})
if err1 != nil {
return sig, 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 > int(types.MaxFsp) {
fsp = int(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 int8, row chunk.Row, arg Expression) (res types.Time, isNull bool, err error) {
Expand Down
73 changes: 45 additions & 28 deletions planner/core/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ package core_test
import (
. "github.com/pingcap/check"
"github.com/pingcap/parser/mysql"
"github.com/pingcap/tidb/domain"
"github.com/pingcap/tidb/kv"
"github.com/pingcap/tidb/util/testkit"
"github.com/pingcap/tidb/util/testutil"
)
Expand All @@ -24,6 +26,8 @@ var _ = Suite(&testIntegrationSuite{})

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

func (s *testIntegrationSuite) SetUpSuite(c *C) {
Expand All @@ -36,14 +40,20 @@ func (s *testIntegrationSuite) TearDownSuite(c *C) {
c.Assert(s.testData.GenerateOutputIfNeeded(), IsNil)
}

func (s *testIntegrationSuite) TestShowSubquery(c *C) {
store, dom, err := newStoreWithBootstrap()
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 @@ -74,13 +84,7 @@ func (s *testIntegrationSuite) TestShowSubquery(c *C) {
}

func (s *testIntegrationSuite) TestPpdWithSetVar(c *C) {
store, dom, err := newStoreWithBootstrap()
c.Assert(err, IsNil)
tk := testkit.NewTestKit(c, store)
defer func() {
dom.Close()
store.Close()
}()
tk := testkit.NewTestKit(c, s.store)
tk.MustExec("use test")
tk.MustExec("drop table if exists t")
tk.MustExec("create table t(c1 int, c2 varchar(255))")
Expand All @@ -91,13 +95,7 @@ func (s *testIntegrationSuite) TestPpdWithSetVar(c *C) {
}

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

tk.MustExec("use test")
tk.MustExec("drop table if exists bit_col_t")
Expand All @@ -110,19 +108,14 @@ func (s *testIntegrationSuite) TestBitColErrorMessage(c *C) {
}

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

tk.MustExec("use test")
tk.MustExec("drop table if exists tbl")
tk.MustExec("create table tbl(a int, b int, c int, key idx_b_c(b,c))")
tk.MustExec("insert into tbl values(1,1,1),(2,2,2),(3,3,3),(4,4,4),(5,5,5)")
tk.MustExec("analyze table tbl")

var input []string
var output []struct {
SQL string
Expand All @@ -137,3 +130,27 @@ func (s *testIntegrationSuite) TestPushLimitDownIndexLookUpReader(c *C) {
tk.MustQuery(tt).Check(testkit.Rows(output[i].Plan...))
}
}

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...))
}
}
7 changes: 7 additions & 0 deletions planner/core/testdata/integration_suite_in.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,12 @@
// 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);"
]
}
]
18 changes: 18 additions & 0 deletions planner/core/testdata/integration_suite_out.json
Original file line number Diff line number Diff line change
Expand Up @@ -43,5 +43,23 @@
]
}
]
},
{
"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 9990.00 root inner join, inner:Selection_13, equal:[eq(Column#1, Column#4)]",
"├─TableReader_12 9990.00 root data:Selection_11",
"│ └─Selection_11 9990.00 cop not(isnull(Column#1))",
"│ └─TableScan_10 10000.00 cop table:t1, range:[-inf,+inf], keep order:false, stats:pseudo",
"└─Selection_13 7992.00 root from_unixtime(cast(Column#5))",
" └─TableReader_16 9990.00 root data:Selection_15",
" └─Selection_15 9990.00 cop not(isnull(Column#4))",
" └─TableScan_14 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 @@ -189,6 +189,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 01dce81

Please sign in to comment.