expression, parser: fix issue #3691, cast compatibility #3894

winkyao · 2017-07-26T03:54:49Z

fix #3691, also fix #645

this pr will fail the jenkins, so should be merged together.

…scalar function

…olved

Merge branch winkyao/fix_issue_3762 into winkyao/cast_compability

…/cast_compability

…s 2, should return datum, not row

…/fix_issue_3762

…o handle overflow gracely

…/fix_issue_3762

into winkyao/fix_issue_3762

…/fix_issue_3762

into winkyao/fix_issue_3762

…/cast_compability

…/tidb into winkyao/cast_compability

XuHuaiyu · 2017-07-28T02:36:54Z

executor/executor_test.go

@@ -1097,6 +1099,75 @@ func (s *testSuite) TestBuiltin(c *C) {
 result = tk.MustQuery("select cast(-1 as unsigned)")
 result.Check(testkit.Rows("18446744073709551615"))

+ // Fix issue #3691, cast compabilities.


s/ compabilities/ compatibility

XuHuaiyu · 2017-07-28T02:39:04Z

expression/builtin_cast.go

@@ -23,12 +23,16 @@ package expression

 import (
 "strconv"
+ "strings"
+


remove this empty line.

XuHuaiyu · 2017-07-28T02:39:15Z

expression/builtin_cast.go

@@ -23,12 +23,16 @@ package expression

 import (
 "strconv"
+ "strings"
+
+ "math"


move math to line 25

Why make check ignore this?

XuHuaiyu · 2017-07-28T02:40:20Z

expression/builtin_cast.go

@@ -542,22 +546,24 @@ func (b *builtinCastDecimalAsIntSig) evalInt(row []types.Datum) (res int64, isNu
 if isNull || err != nil {
 return res, isNull, errors.Trace(err)
 }
+
+ // despite of unsigned or signed, Round is needed


Round is needed for both unsigned and signed.

XuHuaiyu · 2017-07-28T02:41:21Z

plan/typeinfer_test.go

@@ -106,6 +106,7 @@ func (s *testPlanSuite) TestInferType(c *C) {
 {"substr(c_binary, c_int)", mysql.TypeVarString, charset.CharsetBin, mysql.BinaryFlag, 20, types.UnspecifiedLength},
 {"uuid()", mysql.TypeVarString, charset.CharsetUTF8, 0, 36, types.UnspecifiedLength},
 {"bit_length(c_char)", mysql.TypeLonglong, charset.CharsetBin, mysql.BinaryFlag, 10, 0},
+


remove this line?

XuHuaiyu · 2017-07-28T02:41:50Z

sessionctx/variable/session.go

@@ -422,6 +423,19 @@ func (sc *StatementContext) HandleTruncate(err error) error {
 return err
 }

+// HandleOverflow treat ErrOverflow as warnings or returns the error bases on the StmtCtx.OverflowAsWarning state.


s/ treat/ treats

XuHuaiyu · 2017-07-28T02:43:22Z

util/types/datum.go

@@ -1131,6 +1131,11 @@ func ProduceDecWithSpecifiedTp(dec *MyDecimal, tp *FieldType, sc *variable.State
 }
 }
 }
+
+ if terror.ErrorEqual(err, ErrOverflow) {


should we put this check into HandleOverflow ?

@XuHuaiyu no, because the variable.session.go should not import "types" package, because "types" use "variable" package, it will cause circle import problem

XuHuaiyu · 2017-07-28T02:48:05Z

util/types/errors.go

@@ -37,35 +39,46 @@ var (
 ErrWrongFieldSpec = terror.ClassTypes.New(codeWrongFieldSpec, "Wrong Field Spec")
 // ErrBadNumber is return when parsing an invalid binary decimal number.
 ErrBadNumber = terror.ClassTypes.New(codeBadNumber, "Bad Number")
+ // ErrCastSignedOverflow is returned when positive out-of-range integer, and convert to it's negative complement
+ ErrCastSignedOverflow = terror.ClassTypes.New(codeUnknown, msgCastSignedOverflow)
+ // ErrCastNegIntToUnsigned is returned when a negative integer cast to unsigned


s/ ErrCastNegIntToUnsigned/ ErrCastNegIntAsUnsigned be better?

s/ cast/ be casted
s/ unsigned/ an unsigned int

add . at the end.

XuHuaiyu · 2017-07-28T03:03:54Z

util/types/errors.go

@@ -37,35 +39,46 @@ var (
 ErrWrongFieldSpec = terror.ClassTypes.New(codeWrongFieldSpec, "Wrong Field Spec")
 // ErrBadNumber is return when parsing an invalid binary decimal number.
 ErrBadNumber = terror.ClassTypes.New(codeBadNumber, "Bad Number")
+ // ErrCastSignedOverflow is returned when positive out-of-range integer, and convert to it's negative complement


s/ ErrCastSignedOverflow/ ErrCastAsSignedOverflow be better?

add . at the end

jackysp · 2017-07-28T03:44:23Z

executor/prepared.go

+ // see https://dev.mysql.com/doc/refman/5.7/en/sql-mode.html#sql-mode-strict
+ // said "For statements such as SELECT that do not change data, invalid values
+ // generate a warning in strict mode, not an error."
+ // and and https://dev.mysql.com/doc/refman/5.7/en/out-of-range-and-overflow.html


jackysp · 2017-07-28T03:45:51Z

expression/builtin_cast.go

@@ -23,12 +23,16 @@ package expression

 import (
 "strconv"
+ "strings"
+
+ "math"


Why make check ignore this?

jackysp · 2017-07-28T03:49:17Z

expression/builtin_cast.go

- uintRes, err = types.ConvertFloatToUint(sc, floatVal, types.UnsignedUpperBound[mysql.TypeLonglong], mysql.TypeDouble)
- res = int64(uintRes)
+ var ures uint64
+ ures, err = to.ToUint()


The old name uintRes may be more readable.

winkyao · 2017-07-29T06:38:39Z

@XuHuaiyu @zz-jason PTAL

XuHuaiyu · 2017-07-31T03:30:19Z

util/types/errors.go

- msgOverflow = mysql.MySQLErrName[mysql.ErrDataOutOfRange]
+ msgOverflow = mysql.MySQLErrName[mysql.ErrDataOutOfRange]
+ msgTruncatedWrongVal = mysql.MySQLErrName[mysql.ErrTruncatedWrongValue]
+ msgCastSignedOverflow = "Cast to signed converted positive out-of-range integer to it's negative complement"


s/ msgCastSignedOverflow/ msgCastAsSignedOverflow
s/ msgCastNegIntToUnsigned/ msgCastNegIntAsUnsigned

XuHuaiyu · 2017-07-31T03:32:45Z

expression/builtin_cast.go

@@ -636,6 +641,27 @@ type builtinCastStringAsIntSig struct {
 baseIntBuiltinFunc
 }

+func (b *builtinCastStringAsIntSig) handleOverflow(origRes int64, origStr string, origErr error, isNegative bool) (res int64, err error) {


may add a comment for this.

XuHuaiyu · 2017-07-31T03:38:32Z

expression/builtin_cast_test.go

+ sc.OverflowAsWarning = true
+
+ // cast('18446744073709551616' as unsigned);
+ tp1 := types.NewFieldType(mysql.TypeLonglong)


will this be more readable?

tp1 := &types.FieldType{ Tp: Flag: Charset: .... }

XuHuaiyu · 2017-07-31T03:39:05Z

expression/builtin_cast_test.go

+ // create table t1(s1 time);
+ // insert into t1 values('11:11:11');
+ // select cast(s1 as decimal(7, 2)) from t1;
+ tpDecimal := types.NewFieldType(mysql.TypeNewDecimal)


XuHuaiyu · 2017-07-31T03:43:38Z

parser/parser.y

@@ -5909,6 +5909,7 @@ IntegerType:
 OptInteger:
 {}
 | "INTEGER"
+| "INT"


add a test case in parser_test

XuHuaiyu · 2017-07-31T03:43:59Z

sessionctx/variable/session.go

@@ -422,6 +423,19 @@ func (sc *StatementContext) HandleTruncate(err error) error {
 return err
 }

+// HandleOverflow treats ErrOverflow as warnings or returns the error bases on the StmtCtx.OverflowAsWarning state.


s/ bases/ based

…/tidb into winkyao/cast_compability

…/cast_compability

winkyao · 2017-07-31T16:40:43Z

@XuHuaiyu @jackysp PTAL

jackysp · 2017-08-01T02:43:35Z

LGTM

XuHuaiyu

LGTM

…/cast_compability

…/tidb into winkyao/cast_compability

winkyao and others added 30 commits July 17, 2017 11:13

expression: handle int builtinUnaryOpSig overflow

7762b8b

expression: fix #3762, signed integer overflow handle in minus unary …

f87e0af

…scalar function

expression: fix #3762, add builtin_op_test.go, some builtin test cases

465dd1b

*: tiny cleanup

17f1e5f

expression: fix #3691, builtin CAST func compatibilities, partially s…

5589c9e

…olved

Squashed commit of the following:

c61fea1

Merge branch winkyao/fix_issue_3762 into winkyao/cast_compability

Merge branch 'master' of https://github.com/pingcap/tidb into winkyao…

c8f218f

…/cast_compability

expression: set the correct unary minus function ScalarFunction.RetType

1d15330

Merge branch 'winkyao/fix_issue_3762' into winkyao/cast_compability

4319ade

expression: rewrite unary minus builtin function to handle overflow.

a803f9d

plan: fix popRowArg bug: when get expression.Constatn and getRowLen i…

d6e6c25

…s 2, should return datum, not row

*:git stash

ae342fc

Merge branch 'master' of https://github.com/pingcap/tidb into winkyao…

869e1f6

…/fix_issue_3762

expression: base #3868 pr, and rewrite builtin function unary minus t…

b305a4f

…o handle overflow gracely

expression: refactor builtin unary minus function

bedd270

expression: tiny refactor

0e5de09

expression: tiny cleanup

4045075

expression: cleanup

636170d

expresion: cleanup

33bf09c

expression: tiny cleanup

eb58c48

Merge branch 'master' of https://github.com/pingcap/tidb into winkyao…

b199772

…/fix_issue_3762

Merge branch 'master' into winkyao/fix_issue_3762

6005a18

expression: add some comment

e2aa622

Merge branch 'winkyao/fix_issue_3762' of https://github.com/pingcap/tidb

d0395bf

into winkyao/fix_issue_3762

Merge branch 'master' into winkyao/fix_issue_3762

ffc0df7

Merge branch 'master' of https://github.com/pingcap/tidb into winkyao…

27202d6

…/fix_issue_3762

Merge branch 'winkyao/fix_issue_3762' of https://github.com/pingcap/tidb

03dc0e5

into winkyao/fix_issue_3762

expression: adjust commet

7f01b42

Merge branch 'winkyao/fix_issue_3762' into winkyao/cast_compability

14e9711

Merge branch 'master' of https://github.com/pingcap/tidb into winkyao…

19b2fd9

…/cast_compability

winkyao added 3 commits July 27, 2017 20:35

expression: tiny cleanup

684c314

Merge branch 'winkyao/cast_compability' of https://github.com/pingcap…

4edca6b

…/tidb into winkyao/cast_compability

executor: add test case

028658d

XuHuaiyu changed the title ~~expression, parser: fix issue #3691, cast compabilities~~ expression, parser: fix issue #3691, cast compatibility Jul 28, 2017

XuHuaiyu reviewed Jul 28, 2017

View reviewed changes

jackysp reviewed Jul 28, 2017

View reviewed changes

*: cleanup

07c31ac

Merge branch 'master' into winkyao/cast_compability

bbe1260

XuHuaiyu reviewed Jul 31, 2017

View reviewed changes

winkyao added 3 commits August 1, 2017 00:38

*: refactor, add parser test case

b51e4f1

Merge branch 'winkyao/cast_compability' of https://github.com/pingcap…

8d98e4d

…/tidb into winkyao/cast_compability

Merge branch 'master' of https://github.com/pingcap/tidb into winkyao…

68f0a1d

…/cast_compability

winkyao added the status/LGT1 Indicates that a PR has LGTM 1. label Aug 1, 2017

Merge branch 'master' into winkyao/cast_compability

266971b

XuHuaiyu previously approved these changes Aug 1, 2017

View reviewed changes

XuHuaiyu added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Aug 1, 2017

winkyao added 3 commits August 1, 2017 16:13

Merge branch 'master' of https://github.com/pingcap/tidb into winkyao…

1699498

…/cast_compability

expression: cleanup

55d25a1

Merge branch 'winkyao/cast_compability' of https://github.com/pingcap…

8ac311c

…/tidb into winkyao/cast_compability

winkyao dismissed XuHuaiyu’s stale review via 8ac311c August 1, 2017 08:14

Merge branch 'master' into winkyao/cast_compability

4ff23cd

XuHuaiyu approved these changes Aug 1, 2017

View reviewed changes

jackysp merged commit d0dcb5b into master Aug 1, 2017

winkyao deleted the winkyao/cast_compability branch August 1, 2017 09:15

lysu mentioned this pull request Jul 24, 2018

*: fix union result when mix signed/unsigned columns #7112

Merged

YangKeao mentioned this pull request Dec 4, 2023

Truncate error shouldn't be allowed when handling default values #49141

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

expression, parser: fix issue #3691, cast compatibility #3894

expression, parser: fix issue #3691, cast compatibility #3894

winkyao commented Jul 26, 2017 •

edited

Loading

XuHuaiyu Jul 28, 2017

XuHuaiyu Jul 28, 2017

XuHuaiyu Jul 28, 2017

jackysp Jul 28, 2017

XuHuaiyu Jul 28, 2017

XuHuaiyu Jul 28, 2017

XuHuaiyu Jul 28, 2017

XuHuaiyu Jul 28, 2017

winkyao Jul 29, 2017

XuHuaiyu Jul 28, 2017

XuHuaiyu Jul 28, 2017

jackysp Jul 28, 2017

jackysp Jul 28, 2017

jackysp Jul 28, 2017

winkyao commented Jul 29, 2017

XuHuaiyu Jul 31, 2017

XuHuaiyu Jul 31, 2017

XuHuaiyu Jul 31, 2017

XuHuaiyu Jul 31, 2017

XuHuaiyu Jul 31, 2017

XuHuaiyu Jul 31, 2017

winkyao commented Jul 31, 2017

jackysp commented Aug 1, 2017

XuHuaiyu left a comment

expression, parser: fix issue #3691, cast compatibility #3894

expression, parser: fix issue #3691, cast compatibility #3894

Conversation

winkyao commented Jul 26, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

winkyao commented Jul 29, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

winkyao commented Jul 31, 2017

jackysp commented Aug 1, 2017

XuHuaiyu left a comment

Choose a reason for hiding this comment

winkyao commented Jul 26, 2017 •

edited

Loading