From 52f8912a828a380e8b34384487f7cbc460d67069 Mon Sep 17 00:00:00 2001 From: Reminiscent Date: Wed, 28 Jul 2021 22:38:55 +0800 Subject: [PATCH 1/8] planner: don't change the point range when we convert the same type datum --- util/ranger/ranger.go | 4 ++++ util/ranger/ranger_test.go | 29 ++++++++++++++++++++++++++++- 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/util/ranger/ranger.go b/util/ranger/ranger.go index d69c3dbc02392..223d43e2ccd65 100644 --- a/util/ranger/ranger.go +++ b/util/ranger/ranger.go @@ -123,6 +123,10 @@ func convertPoint(sc *stmtctx.StatementContext, point *point, tp *types.FieldTyp if valCmpCasted == 0 { return npoint, nil } + isSameType := tp.Tp == point.value.Kind() + if isSameType { + return npoint, nil + } if npoint.start { if npoint.excl { if valCmpCasted < 0 { diff --git a/util/ranger/ranger_test.go b/util/ranger/ranger_test.go index 29e1c3fc4a7bf..4cccc45a2af3f 100644 --- a/util/ranger/ranger_test.go +++ b/util/ranger/ranger_test.go @@ -1133,7 +1133,7 @@ func (s *testRangerSuite) TestColumnRange(c *C) { exprStr: `c > 111.11111111`, accessConds: "[gt(test.t.c, 111.11111111)]", filterConds: "[]", - resultStr: "[[111.111115,+inf]]", + resultStr: "[(111.111115,+inf]]", length: types.UnspecifiedLength, }, { @@ -1254,6 +1254,33 @@ func (s *testRangerSuite) TestIndexRangeElimininatedProjection(c *C) { )) } +func (s *testRangerSuite) TestIssue26638(c *C) { + defer testleak.AfterTest(c)() + dom, store, err := newDomainStoreWithBootstrap(c) + defer func() { + dom.Close() + store.Close() + }() + c.Assert(err, IsNil) + testKit := testkit.NewTestKit(c, store) + testKit.MustExec("use test") + testKit.MustExec("drop table if exists t") + testKit.MustExec("create table t(a float, unique index uidx(a));") + testKit.MustExec("insert into t values(9.46347e37);") + testKit.MustExec("analyze table t") + testKit.MustQuery("explain format = 'brief' select * from t where a in (-1.56018e38, -1.96716e38, 9.46347e37);").Check(testkit.Rows( + "Batch_Point_Get 1.00 root table:t, index:uidx(a) keep order:false, desc:false", + )) + testKit.MustQuery("select * from t where a in (-1.56018e38, -1.96716e38, 9.46347e37);").Check(testkit.Rows( + "94634700000000000000000000000000000000", + )) + testKit.MustExec("prepare stmt from 'select * from t where a in (?, ?, ?);';") + testKit.MustExec("set @a=-1.56018e38, @b=-1.96716e38, @c=9.46347e37;") + testKit.MustQuery("execute stmt using @a, @b, @c;").Check(testkit.Rows( + "94634700000000000000000000000000000000", + )) +} + func (s *testRangerSuite) TestCompIndexInExprCorrCol(c *C) { defer testleak.AfterTest(c)() dom, store, err := newDomainStoreWithBootstrap(c) From 329934075daaaa4b075a70a4161189ef6537c65d Mon Sep 17 00:00:00 2001 From: Reminiscent Date: Sun, 22 Aug 2021 10:46:49 +0800 Subject: [PATCH 2/8] remove the wrong changes --- util/ranger/ranger.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/util/ranger/ranger.go b/util/ranger/ranger.go index 223d43e2ccd65..d69c3dbc02392 100644 --- a/util/ranger/ranger.go +++ b/util/ranger/ranger.go @@ -123,10 +123,6 @@ func convertPoint(sc *stmtctx.StatementContext, point *point, tp *types.FieldTyp if valCmpCasted == 0 { return npoint, nil } - isSameType := tp.Tp == point.value.Kind() - if isSameType { - return npoint, nil - } if npoint.start { if npoint.excl { if valCmpCasted < 0 { From e2367c7cbbc7c21ac6292bbec97aa58861717d28 Mon Sep 17 00:00:00 2001 From: Reminiscent Date: Mon, 23 Aug 2021 15:35:34 +0800 Subject: [PATCH 3/8] 1.change the datum value based on the column type 2. add more test cases --- planner/core/point_get_plan.go | 46 +++++++++++++++++++---------- planner/core/point_get_plan_test.go | 29 ++++++++++++++++++ 2 files changed, 59 insertions(+), 16 deletions(-) diff --git a/planner/core/point_get_plan.go b/planner/core/point_get_plan.go index 8038ff2e42550..f245c2bf26020 100644 --- a/planner/core/point_get_plan.go +++ b/planner/core/point_get_plan.go @@ -556,6 +556,7 @@ func newBatchPointGetPlan( handleCol *model.ColumnInfo, tbl *model.TableInfo, schema *expression.Schema, names []*types.FieldName, whereColNames []string, indexHints []*ast.IndexHint, ) *BatchPointGetPlan { + stmtCtx := ctx.GetSessionVars().StmtCtx statsInfo := &property.StatsInfo{RowCount: float64(len(patternInExpr.List))} var partitionExpr *tables.PartitionExpr if tbl.GetPartitionInfo() != nil { @@ -593,16 +594,8 @@ func newBatchPointGetPlan( if d.IsNull() { return nil } - if !checkCanConvertInPointGet(handleCol, d) { - return nil - } - intDatum, err := d.ConvertTo(ctx.GetSessionVars().StmtCtx, &handleCol.FieldType) - if err != nil { - return nil - } - // The converted result must be same as original datum - cmp, err := intDatum.CompareDatum(ctx.GetSessionVars().StmtCtx, &d) - if err != nil || cmp != 0 { + intDatum := getPointGetValue(stmtCtx, handleCol, &d) + if intDatum == nil { return nil } handles[i] = kv.IntHandle(intDatum.GetInt64()) @@ -686,12 +679,14 @@ func newBatchPointGetPlan( permIndex := permutations[index] switch innerX := inner.(type) { case *driver.ValueExpr: - if !checkCanConvertInPointGet(colInfos[index], innerX.Datum) { + dval := getPointGetValue(stmtCtx, colInfos[index], &innerX.Datum) + if dval == nil { return nil } values[permIndex] = innerX.Datum case *driver.ParamMarkerExpr: - if !checkCanConvertInPointGet(colInfos[index], innerX.Datum) { + dval := getPointGetValue(stmtCtx, colInfos[index], &innerX.Datum) + if dval == nil { return nil } values[permIndex] = innerX.Datum @@ -706,18 +701,20 @@ func newBatchPointGetPlan( if len(whereColNames) != 1 { return nil } - if !checkCanConvertInPointGet(colInfos[0], x.Datum) { + dval := getPointGetValue(stmtCtx, colInfos[0], &x.Datum) + if dval == nil { return nil } - values = []types.Datum{x.Datum} + values = []types.Datum{*dval} case *driver.ParamMarkerExpr: if len(whereColNames) != 1 { return nil } - if !checkCanConvertInPointGet(colInfos[0], x.Datum) { + dval := getPointGetValue(stmtCtx, colInfos[0], &x.Datum) + if dval == nil { return nil } - values = []types.Datum{x.Datum} + values = []types.Datum{*dval} valuesParams = []*driver.ParamMarkerExpr{x} default: return nil @@ -1229,6 +1226,23 @@ func getNameValuePairs(stmtCtx *stmtctx.StatementContext, tbl *model.TableInfo, return nil, false } +func getPointGetValue(stmtCtx *stmtctx.StatementContext, col *model.ColumnInfo, d *types.Datum) *types.Datum { + if !checkCanConvertInPointGet(col, *d) { + return nil + } + dVal, err := d.ConvertTo(stmtCtx, &col.FieldType) + if err != nil { + return nil + } + // The converted result must be same as original datum. + // Compare them based on the dVal's type. + cmp, err := dVal.CompareDatum(stmtCtx, d) + if err != nil || cmp != 0 { + return nil + } + return &dVal +} + func checkCanConvertInPointGet(col *model.ColumnInfo, d types.Datum) bool { kind := d.Kind() switch col.FieldType.EvalType() { diff --git a/planner/core/point_get_plan_test.go b/planner/core/point_get_plan_test.go index 9d426ed719cfc..338ebffcc9002 100644 --- a/planner/core/point_get_plan_test.go +++ b/planner/core/point_get_plan_test.go @@ -899,6 +899,35 @@ func (s *testPointGetSuite) TestIssue18042(c *C) { tk.MustExec("drop table t") } +func (s *testPointGetSuite) TestIssue26638(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 float, unique index uidx(a));") + tk.MustExec("insert into t values(9.46347e37), (1.1);") + tk.MustQuery("explain format='brief' select * from t where a = 9.46347e37;").Check(testkit.Rows("TableDual 0.00 root rows:0")) + tk.MustQuery("explain format='brief' select * from t where a in (-1.56018e38, -1.96716e38, 9.46347e37);").Check(testkit.Rows("TableDual 0.00 root rows:0")) + tk.MustQuery("explain format='brief' select * from t where a in (1.1, 9.46347e37);").Check(testkit.Rows("TableDual 0.00 root rows:0")) + tk.MustExec("prepare stmt from 'select * from t where a in (?, ?, ?);';") + tk.MustExec("prepare stmt1 from 'select * from t where a = ?;';") + tk.MustExec("prepare stmt2 from 'select * from t where a in (?, ?);';") + tk.MustExec("set @a=-1.56018e38, @b=-1.96716e38, @c=9.46347e37, @d=1.1, @e=0, @f=-1, @g=1, @h=2, @i=-1.1;") + tk.MustQuery("execute stmt using @a,@b,@c;").Check(testkit.Rows()) + tk.MustQuery("execute stmt1 using @c;").Check(testkit.Rows()) + tk.MustQuery("execute stmt2 using @c, @d;").Check(testkit.Rows()) + tk.MustExec("drop table if exists t2;") + tk.MustExec("create table t2(a float, b float, c float, primary key(a, b, c));") + tk.MustExec("insert into t2 values(-1, 0, 1), (-1.1, 0, 1.1), (-1.56018e38, -1.96716e38, 9.46347e37), (0, 1, 2);") + tk.MustQuery("explain format='brief' select * from t2 where (a, b, c) in ((-1.1, 0, 1.1), (-1.56018e38, -1.96716e38, 9.46347e37));").Check(testkit.Rows("TableDual 0.00 root rows:0")) + tk.MustQuery("select * from t2 where (a, b, c) in ((-1.1, 0, 1.1), (-1.56018e38, -1.96716e38, 9.46347e37), (-1, 0, 1));").Check(testkit.Rows("-1 0 1")) + tk.MustQuery("select * from t2 where (a, b, c) in ((-1.1, 0, 1.1), (-1.56018e38, -1.96716e38, 9.46347e37), (0, 1, 2));").Check(testkit.Rows("0 1 2")) + tk.MustExec("prepare stmt3 from 'select * from t2 where (a, b, c) in ((?, ?, ?), (?, ?, ?));';") + tk.MustExec("prepare stmt4 from 'select * from t2 where (a, b, c) in ((?, ?, ?), (?, ?, ?), (?, ?, ?));';") + tk.MustQuery("execute stmt3 using @i,@e,@d,@a,@b,@c;").Check(testkit.Rows()) + tk.MustQuery("execute stmt4 using @i,@e,@d,@a,@b,@c,@f,@e,@g;").Check(testkit.Rows("-1 0 1")) + tk.MustQuery("execute stmt4 using @i,@e,@d,@a,@b,@c,@e,@g,@h;").Check(testkit.Rows("0 1 2")) +} + func (s *testPointGetSuite) TestIssue23511(c *C) { tk := testkit.NewTestKit(c, s.store) tk.MustExec("use test") From d18cf88fbaaebb37fafde184a6518bc1d1ac9f1f Mon Sep 17 00:00:00 2001 From: Reminiscent Date: Tue, 24 Aug 2021 18:25:28 +0800 Subject: [PATCH 4/8] fix the enum type(TODO: remove the return error param) --- types/enum.go | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/types/enum.go b/types/enum.go index d21c63ab18c7b..d60e924ecfefb 100644 --- a/types/enum.go +++ b/types/enum.go @@ -15,8 +15,6 @@ package types import ( - "strconv" - "github.com/pingcap/errors" "github.com/pingcap/tidb/util/collate" "github.com/pingcap/tidb/util/stringutil" @@ -48,15 +46,11 @@ func (e Enum) ToNumber() float64 { // ParseEnum creates a Enum with item name or value. func ParseEnum(elems []string, name string, collation string) (Enum, error) { - if enumName, err := ParseEnumName(elems, name, collation); err == nil { + if enumName, err := ParseEnumName(elems, name, collation); err != nil { return enumName, nil } - // name doesn't exist, maybe an integer? - if num, err := strconv.ParseUint(name, 0, 64); err == nil { - return ParseEnumValue(elems, num) - } - - return Enum{}, errors.Errorf("item %s is not in enum %v", name, elems) + // If the string name is not in the enum collection, an empty enum will be returned. + return Enum{}, nil } // ParseEnumName creates a Enum with item name. From c096c9d89be7bb5269ef658644a896a3306b586e Mon Sep 17 00:00:00 2001 From: Reminiscent Date: Tue, 24 Aug 2021 18:28:42 +0800 Subject: [PATCH 5/8] fixup --- types/enum.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/types/enum.go b/types/enum.go index d60e924ecfefb..ba3cacdfe10b1 100644 --- a/types/enum.go +++ b/types/enum.go @@ -46,7 +46,7 @@ func (e Enum) ToNumber() float64 { // ParseEnum creates a Enum with item name or value. func ParseEnum(elems []string, name string, collation string) (Enum, error) { - if enumName, err := ParseEnumName(elems, name, collation); err != nil { + if enumName, err := ParseEnumName(elems, name, collation); err == nil { return enumName, nil } // If the string name is not in the enum collection, an empty enum will be returned. From 331d5103b1a86a9df68c645992fe193827d8f3f6 Mon Sep 17 00:00:00 2001 From: Reminiscent Date: Wed, 25 Aug 2021 16:18:07 +0800 Subject: [PATCH 6/8] fix ut --- types/convert_test.go | 2 +- types/enum_test.go | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/types/convert_test.go b/types/convert_test.go index 4f17cb2012420..a87e04747ecd1 100644 --- a/types/convert_test.go +++ b/types/convert_test.go @@ -311,7 +311,7 @@ func (s *testTypeConvertSuite) TestConvertType(c *C) { c.Assert(err, IsNil) c.Assert(v, DeepEquals, Enum{Name: "b", Value: 2}) _, err = Convert("d", ft) - c.Assert(err, NotNil) + c.Assert(v, DeepEquals, Enum{}) v, err = Convert(4, ft) c.Assert(terror.ErrorEqual(err, ErrTruncated), IsTrue, Commentf("err %v", err)) c.Assert(v, DeepEquals, Enum{}) diff --git a/types/enum_test.go b/types/enum_test.go index b37457368e0f7..1a23bec6e86e1 100644 --- a/types/enum_test.go +++ b/types/enum_test.go @@ -54,7 +54,7 @@ func (s *testEnumSuite) TestEnum(c *C) { for _, t := range tbl { e, err := ParseEnum(t.Elems, t.Name, mysql.DefaultCollationName) if t.Expected == 0 { - c.Assert(err, NotNil) + c.Assert(e, DeepEquals, Enum{}) c.Assert(e.ToNumber(), Equals, float64(0)) c.Assert(e.String(), Equals, "") continue @@ -68,7 +68,7 @@ func (s *testEnumSuite) TestEnum(c *C) { for _, t := range tbl { e, err := ParseEnum(t.Elems, t.Name, "utf8_unicode_ci") if t.Expected == 0 { - c.Assert(err, NotNil) + c.Assert(e, DeepEquals, Enum{}) c.Assert(e.ToNumber(), Equals, float64(0)) c.Assert(e.String(), Equals, "") continue @@ -82,7 +82,7 @@ func (s *testEnumSuite) TestEnum(c *C) { for _, t := range citbl { e, err := ParseEnum(t.Elems, t.Name, "utf8_general_ci") if t.Expected == 0 { - c.Assert(err, NotNil) + c.Assert(e, DeepEquals, Enum{}) c.Assert(e.ToNumber(), Equals, float64(0)) c.Assert(e.String(), Equals, "") continue @@ -105,7 +105,7 @@ func (s *testEnumSuite) TestEnum(c *C) { for _, t := range tblNumber { e, err := ParseEnumValue(t.Elems, t.Number) if t.Expected == 0 { - c.Assert(err, NotNil) + c.Assert(e, DeepEquals, Enum{}) continue } From 82edc56bac5ec5900c2f45164a7a71b5292c3290 Mon Sep 17 00:00:00 2001 From: Reminiscent Date: Tue, 31 Aug 2021 14:11:49 +0800 Subject: [PATCH 7/8] revert the change for ParseEnum --- types/convert_test.go | 2 +- types/enum.go | 10 ++++++++-- types/enum_test.go | 8 ++++---- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/types/convert_test.go b/types/convert_test.go index a87e04747ecd1..4f17cb2012420 100644 --- a/types/convert_test.go +++ b/types/convert_test.go @@ -311,7 +311,7 @@ func (s *testTypeConvertSuite) TestConvertType(c *C) { c.Assert(err, IsNil) c.Assert(v, DeepEquals, Enum{Name: "b", Value: 2}) _, err = Convert("d", ft) - c.Assert(v, DeepEquals, Enum{}) + c.Assert(err, NotNil) v, err = Convert(4, ft) c.Assert(terror.ErrorEqual(err, ErrTruncated), IsTrue, Commentf("err %v", err)) c.Assert(v, DeepEquals, Enum{}) diff --git a/types/enum.go b/types/enum.go index ba3cacdfe10b1..d21c63ab18c7b 100644 --- a/types/enum.go +++ b/types/enum.go @@ -15,6 +15,8 @@ package types import ( + "strconv" + "github.com/pingcap/errors" "github.com/pingcap/tidb/util/collate" "github.com/pingcap/tidb/util/stringutil" @@ -49,8 +51,12 @@ func ParseEnum(elems []string, name string, collation string) (Enum, error) { if enumName, err := ParseEnumName(elems, name, collation); err == nil { return enumName, nil } - // If the string name is not in the enum collection, an empty enum will be returned. - return Enum{}, nil + // name doesn't exist, maybe an integer? + if num, err := strconv.ParseUint(name, 0, 64); err == nil { + return ParseEnumValue(elems, num) + } + + return Enum{}, errors.Errorf("item %s is not in enum %v", name, elems) } // ParseEnumName creates a Enum with item name. diff --git a/types/enum_test.go b/types/enum_test.go index 1a23bec6e86e1..b37457368e0f7 100644 --- a/types/enum_test.go +++ b/types/enum_test.go @@ -54,7 +54,7 @@ func (s *testEnumSuite) TestEnum(c *C) { for _, t := range tbl { e, err := ParseEnum(t.Elems, t.Name, mysql.DefaultCollationName) if t.Expected == 0 { - c.Assert(e, DeepEquals, Enum{}) + c.Assert(err, NotNil) c.Assert(e.ToNumber(), Equals, float64(0)) c.Assert(e.String(), Equals, "") continue @@ -68,7 +68,7 @@ func (s *testEnumSuite) TestEnum(c *C) { for _, t := range tbl { e, err := ParseEnum(t.Elems, t.Name, "utf8_unicode_ci") if t.Expected == 0 { - c.Assert(e, DeepEquals, Enum{}) + c.Assert(err, NotNil) c.Assert(e.ToNumber(), Equals, float64(0)) c.Assert(e.String(), Equals, "") continue @@ -82,7 +82,7 @@ func (s *testEnumSuite) TestEnum(c *C) { for _, t := range citbl { e, err := ParseEnum(t.Elems, t.Name, "utf8_general_ci") if t.Expected == 0 { - c.Assert(e, DeepEquals, Enum{}) + c.Assert(err, NotNil) c.Assert(e.ToNumber(), Equals, float64(0)) c.Assert(e.String(), Equals, "") continue @@ -105,7 +105,7 @@ func (s *testEnumSuite) TestEnum(c *C) { for _, t := range tblNumber { e, err := ParseEnumValue(t.Elems, t.Number) if t.Expected == 0 { - c.Assert(e, DeepEquals, Enum{}) + c.Assert(err, NotNil) continue } From fd2f31da79f28a79dca41546c086023ca7f5ec67 Mon Sep 17 00:00:00 2001 From: Reminiscent Date: Mon, 22 Nov 2021 15:47:58 +0800 Subject: [PATCH 8/8] add more comments --- planner/core/point_get_plan_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/planner/core/point_get_plan_test.go b/planner/core/point_get_plan_test.go index 338ebffcc9002..300370b60dee6 100644 --- a/planner/core/point_get_plan_test.go +++ b/planner/core/point_get_plan_test.go @@ -905,6 +905,7 @@ func (s *testPointGetSuite) TestIssue26638(c *C) { tk.MustExec("drop table if exists t") tk.MustExec("create table t(a float, unique index uidx(a));") tk.MustExec("insert into t values(9.46347e37), (1.1);") + // If we do not define the precision for the float type. We can not use the equal/ in conditions to find the result. We can only use like to find the result. There is no such restriction for the double type. tk.MustQuery("explain format='brief' select * from t where a = 9.46347e37;").Check(testkit.Rows("TableDual 0.00 root rows:0")) tk.MustQuery("explain format='brief' select * from t where a in (-1.56018e38, -1.96716e38, 9.46347e37);").Check(testkit.Rows("TableDual 0.00 root rows:0")) tk.MustQuery("explain format='brief' select * from t where a in (1.1, 9.46347e37);").Check(testkit.Rows("TableDual 0.00 root rows:0"))