expression: open CAST push down switcher default #12652

lonng · 2019-10-12T08:19:06Z

Signed-off-by: Lonng [email protected]

What problem does this PR solve?

Open the CAST push-down switcher and fix some execution plans.

What is changed and how it works?

We have fixed all CAST functions related inconsistent between TiDB and TiKV, it's time to open the switcher default.

Check List

Tests

Unit test
Integration test

Release note

Open the CAST function push-down switcher by default. (reverted in Revert "expression: open CAST push down switcher default" #13264)

Signed-off-by: Lonng <[email protected]>

Signed-off-by: H-ZeX <[email protected]>

…sh-down-2

Signed-off-by: H-ZeX <[email protected]>

codecov · 2019-10-16T06:48:29Z

Codecov Report

Merging #12652 into master will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##            master    #12652   +/-   ##
=========================================
  Coverage   80.181%   80.181%           
=========================================
  Files          469       469           
  Lines       111555    111555           
=========================================
  Hits         89446     89446           
  Misses       15200     15200           
  Partials      6909      6909

Signed-off-by: Lonng <[email protected]>

H-ZeX · 2019-10-16T16:23:13Z

/run-all-tests tidb-test=pr/918

Signed-off-by: Lonng <[email protected]>

Deardrops · 2019-11-07T02:08:33Z

LGTM

lonng · 2019-11-07T02:26:30Z

Coprocessor integration test: tikv/copr-test#4

djshow832 · 2019-11-07T03:09:48Z

expression/aggregation/util.go

+			data = types.Datum{}
+			data.SetMysqlDecimal(to)
+			if err != nil {
+				return data, err


Why put return after creating data? If err != nil, to is not even integral.

Yes, we should return the data even though overflow occurred (the caller should handle it in a proper way if the err is overflow or truncated).

djshow832 · 2019-11-07T03:16:21Z

expression/aggregation/util.go

@@ -54,7 +55,7 @@ func (d *distinctChecker) Check(values []types.Datum) (bool, error) {
 }

 // calculateSum adds v to sum.
-func calculateSum(sc *stmtctx.StatementContext, sum, v types.Datum) (data types.Datum, err error) {
+func calculateSum(sc *stmtctx.StatementContext, sum, v types.Datum, retType *types.FieldType) (data types.Datum, err error) {


What's the relationship between pushdown CAST and calculateSum? Are there any test cases for sum?

There are many test cases that will cover the sum (query with sum/avg aggregation functions). The is no direct relationship with CAST and sum and I modify it because of the previous implementation does not round appropriately (this BUG is detected by this PR).

djshow832 · 2019-11-07T05:45:24Z

LGTM

Deardrops

LGTM

lonng · 2019-11-07T05:51:48Z

/run-all-tests tidb-test=pr/940

lonng · 2019-11-07T06:38:28Z

/run-integration-common-test tidb-test=pr/940

XuHuaiyu · 2019-11-07T06:42:04Z

expression/expr_to_pb_test.go

 		js, err := json.Marshal(pbExpr)
 		c.Assert(err, IsNil)
-		c.Assert(string(js), Equals, "null")
+		c.Assert(string(js), Equals, expects[i])


Why does this result change?

lonng · 2019-11-07T11:54:33Z

/run-integration-common-test tidb-test=pr/940

lonng · 2019-11-07T11:57:17Z

/run-all-tests tidb-test=pr/940

This reverts commit a785493.

…13264) This reverts commit a785493.

sre-bot · 2019-11-08T08:58:35Z

Benchmark Report

Run Sysbench Performance Test on VMs

@@                               Benchmark Diff                               @@
================================================================================
--- tidb: 79756747e706c2c706ecb00e8f1459796295656c
+++ tidb: a04215ca624cf90b2228882fbb49b4abbcbfd5cd
tikv: 1999eaf8a6da5d4d7cc8f89a0d1127c516614aef
pd: d6d08d9b58fa42a29c3b3f517b9a64a562475588
================================================================================
oltp_update_non_index:
    * QPS: 4712.26 ± 0.35% (std=10.55) delta: 0.01% (p=0.949)
    * Latency p50: 27.16 ± 0.36% (std=0.06) delta: 0.00%
    * Latency p99: 42.43 ± 4.09% (std=1.14) delta: 2.00%
            
oltp_insert:
    * QPS: 3883.00 ± 0.82% (std=20.38) delta: -0.79% (p=0.151)
    * Latency p50: 32.96 ± 0.81% (std=0.17) delta: 0.80%
    * Latency p99: 75.82 ± 0.00% (std=0.00) delta: 0.90%
            
oltp_read_write:
    * QPS: 15614.20 ± 0.14% (std=19.90) delta: 0.29% (p=0.118)
    * Latency p50: 164.30 ± 0.14% (std=0.21) delta: -0.26%
    * Latency p99: 323.08 ± 0.90% (std=2.91) delta: -0.30%
            
oltp_update_index:
    * QPS: 4257.40 ± 0.02% (std=0.62) delta: 0.09% (p=0.300)
    * Latency p50: 30.06 ± 0.07% (std=0.01) delta: -0.10%
    * Latency p99: 54.83 ± 0.00% (std=0.00) delta: 3.66%
            
oltp_point_select:
    * QPS: 50142.42 ± 0.46% (std=183.88) delta: 0.23% (p=0.631)
    * Latency p50: 2.55 ± 0.39% (std=0.01) delta: -0.29%
    * Latency p99: 8.90 ± 0.00% (std=0.00) delta: 0.45%

Signed-off-by: Lonng <[email protected]>

…)" (pingcap#13264) This reverts commit a785493.

lonng added the status/DNM label Oct 12, 2019

lonng requested review from qw4990, Reminiscent, SunRunAway and XuHuaiyu as code owners October 12, 2019 08:19

expression: open CAST push down switcher default

8e27d6b

Signed-off-by: Lonng <[email protected]>

SunRunAway removed their request for review October 12, 2019 08:50

zz-jason removed request for Reminiscent, qw4990 and XuHuaiyu October 15, 2019 02:35

H-ZeX added 2 commits October 16, 2019 13:02

fix plan explain diff after cop cast push down

c275ef1

Signed-off-by: H-ZeX <[email protected]>

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

c6ebe83

…sh-down-2

lonng requested a review from a team as a code owner October 16, 2019 05:12

ghost requested review from XuHuaiyu and removed request for a team October 16, 2019 05:12

H-ZeX added 3 commits October 16, 2019 13:27

fix ci failed

3d31c27

Signed-off-by: H-ZeX <[email protected]>

fix ci failed

c33b5fa

Signed-off-by: H-ZeX <[email protected]>

fix ci failed

5f09713

Signed-off-by: H-ZeX <[email protected]>

lonng added 5 commits October 16, 2019 21:00

go mod tidy

62639e8

Signed-off-by: Lonng <[email protected]>

Merge remote-tracking branch 'origin/master' into cast-push-down

3285154

Merge remote-tracking branch 'origin/master' into cast-push-down

d98dfc4

tiny tweak logic

fec2b71

Signed-off-by: Lonng <[email protected]>

Merge branch 'master' into cast-push-down

4432ba2

H-ZeX and others added 5 commits October 17, 2019 00:28

Merge branch 'master' into cast-push-down

5ab6317

fix decimal evaluation precesion issue

3746056

Signed-off-by: Lonng <[email protected]>

Merge branch 'master' into cast-push-down

8755851

fix the CI failure

799bd92

Signed-off-by: Lonng <[email protected]>

Merge remote-tracking branch 'origin/master' into cast-push-down

7c2d019

H-ZeX added release-note Denotes a PR that will be considered when it comes time to generate release notes. type/new-feature labels Nov 6, 2019

Merge branch 'master' into cast-push-down

e46be5c

XuHuaiyu requested review from eurekaka, qw4990, SunRunAway and XuHuaiyu November 7, 2019 01:51

djshow832 reviewed Nov 7, 2019

View reviewed changes

Deardrops approved these changes Nov 7, 2019

View reviewed changes

Merge branch 'master' into cast-push-down

daf7a02

XuHuaiyu reviewed Nov 7, 2019

View reviewed changes

lonng added 2 commits November 7, 2019 14:55

Merge branch 'master' into cast-push-down

9fc8390

Merge branch 'master' into cast-push-down

48d4832

Merge branch 'master' into cast-push-down

a04215c

lonng merged commit a785493 into pingcap:master Nov 7, 2019

lonng deleted the cast-push-down branch November 7, 2019 12:07

breezewish added a commit that referenced this pull request Nov 7, 2019

Revert "expression: open CAST push down switcher default (#12652)"

057207a

This reverts commit a785493.

breezewish mentioned this pull request Nov 7, 2019

Revert "expression: open CAST push down switcher default" #13264

Merged

SunRunAway removed the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Nov 8, 2019

breezewish added a commit that referenced this pull request Nov 8, 2019

Revert "expression: open CAST push down switcher default (#12652)" (#…

be128c6

…13264) This reverts commit a785493.

XiaTianliang pushed a commit to XiaTianliang/tidb that referenced this pull request Dec 21, 2019

expression: open CAST push down switcher default (pingcap#12652)

1ed7cc7

Signed-off-by: Lonng <[email protected]>

XiaTianliang pushed a commit to XiaTianliang/tidb that referenced this pull request Dec 21, 2019

Revert "expression: open CAST push down switcher default (pingcap#12652…

feba482

…)" (pingcap#13264) This reverts commit a785493.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

expression: open CAST push down switcher default #12652

expression: open CAST push down switcher default #12652

lonng commented Oct 12, 2019 •

edited by SunRunAway

Loading

codecov bot commented Oct 16, 2019 •

edited

Loading

H-ZeX commented Oct 16, 2019

Deardrops commented Nov 7, 2019

lonng commented Nov 7, 2019

djshow832 Nov 7, 2019

lonng Nov 7, 2019

djshow832 Nov 7, 2019

lonng Nov 7, 2019

djshow832 commented Nov 7, 2019

Deardrops left a comment

lonng commented Nov 7, 2019

lonng commented Nov 7, 2019

XuHuaiyu Nov 7, 2019

lonng commented Nov 7, 2019

lonng commented Nov 7, 2019

sre-bot commented Nov 8, 2019

expression: open CAST push down switcher default #12652

expression: open CAST push down switcher default #12652

Conversation

lonng commented Oct 12, 2019 • edited by SunRunAway Loading

What problem does this PR solve?

What is changed and how it works?

Check List

codecov bot commented Oct 16, 2019 • edited Loading

Codecov Report

H-ZeX commented Oct 16, 2019

Deardrops commented Nov 7, 2019

lonng commented Nov 7, 2019

djshow832 Nov 7, 2019

Choose a reason for hiding this comment

lonng Nov 7, 2019

Choose a reason for hiding this comment

djshow832 Nov 7, 2019

Choose a reason for hiding this comment

lonng Nov 7, 2019

Choose a reason for hiding this comment

djshow832 commented Nov 7, 2019

Deardrops left a comment

Choose a reason for hiding this comment

lonng commented Nov 7, 2019

lonng commented Nov 7, 2019

XuHuaiyu Nov 7, 2019

Choose a reason for hiding this comment

lonng commented Nov 7, 2019

lonng commented Nov 7, 2019

sre-bot commented Nov 8, 2019

Benchmark Report

lonng commented Oct 12, 2019 •

edited by SunRunAway

Loading

codecov bot commented Oct 16, 2019 •

edited

Loading