update, expression: fix update float panic #8045

crazycs520 · 2018-10-24T14:23:13Z

What problem does this PR solve?

sql:

drop table if exists t1;
CREATE TABLE t1 (c1 float);
INSERT INTO t1 SET c1 = 1;
UPDATE t1 SET c1 = 1.2 WHERE c1=1;

2018/10/25 17:29:56.341 conn.go:427: [error] lastCmd UPDATE t1 SET c1 = 1.2 WHERE c1=1, runtime error: index out of range, goroutine 317 [running]:
github.com/pingcap/tidb/server.(*clientConn).Run.func1(0xc0085ea680, 0xc0088addff)
        /Users/cs/code/goread/src/github.com/pingcap/tidb/server/conn.go:425 +0x10c
panic(0x1dd7020, 0x2e29120)
        /usr/local/go/src/runtime/panic.go:513 +0x1b9
encoding/binary.littleEndian.PutUint64(...)
        /usr/local/go/src/encoding/binary/binary.go:82
github.com/pingcap/tidb/util/chunk.MutRow.SetDatum(0xc009adf860, 0x0, 0x0, 0x4, 0x3ff3333333333333, 0x0, 0x0, 0x0, 0x0, 0x0)
        /Users/cs/code/goread/src/github.com/pingcap/tidb/util/chunk/mutrow.go:283 +0x823
github.com/pingcap/tidb/executor.(*UpdateExec).composeNewRow(0xc009af2000, 0x0, 0xc009af6230, 0x2, 0x2, 0xc009af6230, 0x2, 0x2, 0xa, 0x69db650)
        /Users/cs/code/goread/src/github.com/pingcap/tidb/executor/update.go:198 +0x408
github.com/pingcap/tidb/executor.(*UpdateExec).fetchChunkRows(0xc009af2000, 0x63c1d10, 0xc009ae0900, 0x0, 0x0)
        /Users/cs/code/goread/src/github.com/pingcap/tidb/executor/update.go:159 +0x1dc
github.com/pingcap/tidb/executor.(*UpdateExec).Next(0xc009af2000, 0x63c1d10, 0xc009ae0900, 0xc009adf800, 0x213ed40, 0xc008764b60)
        /Users/cs/code/goread/src/github.com/pingcap/tidb/executor/update.go:118 +0x71
github.com/pingcap/tidb/executor.(*ExecStmt).handleNoDelayExecutor(0xc009aea900, 0x63c1d10, 0xc009ae0900, 0x213ed40, 0xc008764b60, 0x2120f20, 0xc009af2000, 0x0, 0x0, 0x0, ...)
        /Users/cs/code/goread/src/github.com/pingcap/tidb/executor/adapter.go:272 +0x155
github.com/pingcap/tidb/executor.(*ExecStmt).Exec(0xc009aea900, 0x63c1d10, 0xc009ae0900, 0x0, 0x0, 0x0, 0x0)
        /Users/cs/code/goread/src/github.com/pingcap/tidb/executor/adapter.go:236 +0x4a3
github.com/pingcap/tidb/session.runStmt(0x63c1d10, 0xc009ae0900, 0x213ed40, 0xc008764b60, 0x211c200, 0xc009aea900, 0x105d140, 0xc0088ad768, 0xc0d921f42f, 0xc0088ad7a0)
        /Users/cs/code/goread/src/github.com/pingcap/tidb/session/tidb.go:149 +0x7b
github.com/pingcap/tidb/session.(*session).executeStatement(0xc008764b60, 0x63c1d10, 0xc009ae0900, 0x1, 0x211b540, 0xc009aea680, 0x211c200, 0xc009aea900, 0x0, 0x0, ...)
        /Users/cs/code/goread/src/github.com/pingcap/tidb/session/session.go:741 +0x1eb
github.com/pingcap/tidb/session.(*session).execute(0xc008764b60, 0x63c1d10, 0xc009ae0900, 0xc009ae6661, 0x21, 0x203002, 0x203002, 0x105b1b2, 0x104a2d8, 0x105d140)
        /Users/cs/code/goread/src/github.com/pingcap/tidb/session/session.go:812 +0x87e
github.com/pingcap/tidb/session.(*session).Execute(0xc008764b60, 0x63c1d10, 0xc009ae0900, 0xc009ae6661, 0x21, 0x1, 0x0, 0x0, 0xc0088adb58, 0x10d7420)
        /Users/cs/code/goread/src/github.com/pingcap/tidb/session/session.go:762 +0x69
github.com/pingcap/tidb/server.(*TiDBContext).Execute(0xc008772540, 0x63c1d10, 0xc009ae0900, 0xc009ae6661, 0x21, 0xc0088adbe8, 0x1c5c941, 0xc00017dc70, 0x0, 0x2e7d860)
        /Users/cs/code/goread/src/github.com/pingcap/tidb/server/driver_tidb.go:240 +0x7c
github.com/pingcap/tidb/server.(*clientConn).handleQuery(0xc0085ea680, 0x63c1d10, 0xc009ae0900, 0xc009ae6661, 0x21, 0x0, 0x0)
        /Users/cs/code/goread/src/github.com/pingcap/tidb/server/conn.go:874 +0x8e
github.com/pingcap/tidb/server.(*clientConn).dispatch(0xc0085ea680, 0xc009ae6661, 0x22, 0x22, 0x0, 0x0)
        /Users/cs/code/goread/src/github.com/pingcap/tidb/server/conn.go:626 +0x686
github.com/pingcap/tidb/server.(*clientConn).Run(0xc0085ea680)
        /Users/cs/code/goread/src/github.com/pingcap/tidb/server/conn.go:470 +0x1be
github.com/pingcap/tidb/server.(*Server).onConn(0xc007bdf560, 0x2124dc0, 0xc0083a7ac8)
        /Users/cs/code/goread/src/github.com/pingcap/tidb/server/server.go:324 +0x224
created by github.com/pingcap/tidb/server.(*Server).Run
        /Users/cs/code/goread/src/github.com/pingcap/tidb/server/server.go:264 +0x4a9

What is changed and how it works?

Root cause:
UpdateExec.evalBuffer init with type float32, but scalar_function eval return float64 when FoldConstant, then evalBuffer.SetDatum will panic.

Fix this : add a cast when type is Float.

Check List

Tests

Unit test

Code changes

Has exported function/method change

Side effects

Related changes

Need to cherry-pick to the release branch

…pdate

crazycs520 · 2018-10-24T14:32:30Z

/run-all-tests

crazycs520 · 2018-10-25T09:37:23Z

@jackysp @zz-jason PTAL

zz-jason · 2018-10-25T10:39:51Z

UpdateExec.evalBuffer init with type float32, but scalar_function eval return float64 when FoldConstant, then evalBuffer.SetDatum will panic.

@crazycs520 I think we should modify the datum before evalBuffer.SetDatum() in the update/insert/replace operator.

crazycs520 · 2018-10-25T10:44:38Z

@zz-jason the RetType of scalar_function is float32, but eval return float64, Is this appropriate?

zz-jason · 2018-10-25T10:47:07Z

@crazycs520 Yes, we can convert it back to float32.

crazycs520 · 2018-10-25T11:21:51Z

@zz-jason done. PTAL, insert/replace already have a cast before evalBuffer.SetDatum().

jackysp · 2018-10-25T15:18:13Z

executor/update.go

@@ -194,6 +194,12 @@ func (e *UpdateExec) composeNewRow(rowIdx int, oldRow []types.Datum) ([]types.Da
 		if err1 := e.handleErr(assign.Col.ColName, rowIdx, err); err1 != nil {
 			return nil, err1
 		}
+
+		val, err = val.ConvertTo(e.ctx.GetSessionVars().StmtCtx, fields[assign.Col.Index])


It's better to use

tidb/table/column.go

Line 149 in a84cce1

func CastValue(ctx sessionctx.Context, val types.Datum, col *model.ColumnInfo) (casted types.Datum, err error) {

here.

crazycs520 · 2018-10-26T06:34:48Z

/run-all-tests

…pdate

crazycs520 · 2018-10-26T07:11:33Z

/run-all-tests

zz-jason

LGTM

crazycs520 · 2018-10-29T06:53:15Z

@jackysp PTAL

jackysp

LGTM

crazycs520 · 2018-10-29T08:17:57Z

/run-all-tests

zz-jason · 2018-10-29T08:39:25Z

@crazycs520 Should we cherrypick this PR to the release-2.1 and release-2.0 branch?

crazycs520 · 2018-10-29T10:27:25Z

@zz-jason No, there is no evalBuffer in release-2.1 and 2.0, so won't cause this bug.

jackysp · 2018-11-05T07:44:04Z

This pr could cherry-pick to 2.1 now. @crazycs520

update, expression: fix update float panic

d870cdc

crazycs520 force-pushed the float-update branch from f0d3b75 to d870cdc Compare October 24, 2018 14:28

crazycs520 added the component/expression label Oct 24, 2018

Merge branch 'master' of https://github.com/pingcap/tidb into float-u…

6764595

…pdate

crazycs520 added the type/bugfix This PR fixes a bug. label Oct 24, 2018

update: add convert

8b7a55a

jackysp reviewed Oct 25, 2018

View reviewed changes

update: add cast

aaf7981

Merge branch 'master' of https://github.com/pingcap/tidb into float-u…

54d55bd

…pdate

zz-jason reviewed Oct 26, 2018

View reviewed changes

zz-jason added status/LGT1 Indicates that a PR has LGTM 1. sig/execution SIG execution and removed component/expression labels Oct 26, 2018

jackysp approved these changes Oct 29, 2018

View reviewed changes

Merge branch 'master' into float-update

d3ac5f9

zimulala added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Oct 29, 2018

zz-jason merged commit 736c313 into pingcap:master Oct 29, 2018

crazycs520 added a commit to crazycs520/tidb that referenced this pull request Nov 5, 2018

executor: fix update float panic (pingcap#8045)

f27b856

crazycs520 mentioned this pull request Nov 5, 2018

executor: fix update float panic (#8045) #8170

Merged

zz-jason pushed a commit that referenced this pull request Nov 5, 2018

executor: fix update float panic (#8045) (#8170)

de38a50

exialin mentioned this pull request Nov 28, 2018

executor: fix overflow error message of update #8457

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

update, expression: fix update float panic #8045

update, expression: fix update float panic #8045

crazycs520 commented Oct 24, 2018 •

edited

Loading

crazycs520 commented Oct 24, 2018

crazycs520 commented Oct 25, 2018

zz-jason commented Oct 25, 2018

crazycs520 commented Oct 25, 2018

zz-jason commented Oct 25, 2018

crazycs520 commented Oct 25, 2018

jackysp Oct 25, 2018

crazycs520 commented Oct 26, 2018

crazycs520 commented Oct 26, 2018

zz-jason left a comment

crazycs520 commented Oct 29, 2018

jackysp left a comment

crazycs520 commented Oct 29, 2018

zz-jason commented Oct 29, 2018

crazycs520 commented Oct 29, 2018

jackysp commented Nov 5, 2018

update, expression: fix update float panic #8045

update, expression: fix update float panic #8045

Conversation

crazycs520 commented Oct 24, 2018 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

crazycs520 commented Oct 24, 2018

crazycs520 commented Oct 25, 2018

zz-jason commented Oct 25, 2018

crazycs520 commented Oct 25, 2018

zz-jason commented Oct 25, 2018

crazycs520 commented Oct 25, 2018

jackysp Oct 25, 2018

Choose a reason for hiding this comment

crazycs520 commented Oct 26, 2018

crazycs520 commented Oct 26, 2018

zz-jason left a comment

Choose a reason for hiding this comment

crazycs520 commented Oct 29, 2018

jackysp left a comment

Choose a reason for hiding this comment

crazycs520 commented Oct 29, 2018

zz-jason commented Oct 29, 2018

crazycs520 commented Oct 29, 2018

jackysp commented Nov 5, 2018

crazycs520 commented Oct 24, 2018 •

edited

Loading