Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

expression, planner: support builtin function benchmark #9252

Merged
merged 6 commits into from Feb 13, 2019

Conversation

@wuudjac
Copy link
Contributor

commented Feb 4, 2019

What problem does this PR solve?

Implements builtin function BENCHMARK(), and introduces a counter in expression_rewriter to disable constant folding. (#6774)

What is changed and how it works?

  1. Implements builtin BENCHMARK() function.
  2. To disable constant folding inside BENCHMARK() scope, expressionRewriter.disableFoldCounter is introduced.

Check List

Tests

  • Unit test
  • Integration test
  • Manual test
select benchmark(1000000000, 3);
select benchmark(1000000000, length("abc"));
select benchmark(1000000000, sin(123) + sin(456));

Execurtion time of these 3 commands should differ.

Code changes

  • builtinBenchmarkSig in expression/builtin_info.go
  • expressionRewriter.disableFoldCounter in planner/core/expression_rewriter.go
  • expressionRewriter.newFunction() in planner/core/expression_rewriter.go to control new function behavior by the counter.

Side effects

  • Everytime the expression_rewriter wants to get a new builtin function expression, it has to use expressionRewriter.newFunction() instead of expression.NewFunction(), or this builtin function may not be available to be measured by BENCHMARK() because of constant folding.

Related changes

  • Nothing.
expression, planner: support builtin function benchmark
implements builtin function `BENCHMARK()`, and introduces a counter
in expression_rewriter to disable constant folding. (#6774)
@CLAassistant

This comment has been minimized.

Copy link

commented Feb 4, 2019

CLA assistant check
All committers have signed the CLA.

@codecov-io

This comment has been minimized.

Copy link

commented Feb 4, 2019

Codecov Report

Merging #9252 into master will increase coverage by <.01%.
The diff coverage is 72.22%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #9252      +/-   ##
==========================================
+ Coverage   67.15%   67.15%   +<.01%     
==========================================
  Files         371      371              
  Lines       77427    77491      +64     
==========================================
+ Hits        51995    52041      +46     
- Misses      20785    20797      +12     
- Partials     4647     4653       +6
Impacted Files Coverage Δ
expression/builtin_info.go 61.94% <64.91%> (+0.77%) ⬆️
planner/core/expression_rewriter.go 73.93% <84.84%> (+0.31%) ⬆️
store/tikv/scan.go 73.94% <0%> (-3.37%) ⬇️
infoschema/infoschema.go 76.31% <0%> (-1.32%) ⬇️
expression/schema.go 93.75% <0%> (-0.79%) ⬇️
executor/join.go 77.86% <0%> (-0.53%) ⬇️
expression/builtin_string.go 76% <0%> (+0.1%) ⬆️
expression/util.go 72.57% <0%> (+0.57%) ⬆️
expression/scalar_function.go 92.64% <0%> (+2.2%) ⬆️
util/systimemon/systime_mon.go 100% <0%> (+20%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 357f9d7...555686b. Read the comment docs.

@shenli

This comment has been minimized.

Copy link
Member

commented Feb 5, 2019

@wuudjac Thanks for your contribution!

Show resolved Hide resolved expression/builtin_info.go Outdated
Show resolved Hide resolved expression/builtin_info.go Outdated
Show resolved Hide resolved expression/builtin_info.go Outdated
Show resolved Hide resolved expression/builtin_info.go Outdated
Show resolved Hide resolved expression/builtin_info.go Outdated

@zz-jason zz-jason requested review from eurekaka and qw4990 Feb 12, 2019

wuudjac added some commits Feb 12, 2019

@zz-jason
Copy link
Member

left a comment

LGTM

@lamxTyler
Copy link
Member

left a comment

LGTM

@lamxTyler lamxTyler added status/LGT2 and removed status/LGT1 labels Feb 13, 2019

@lamxTyler

This comment has been minimized.

Copy link
Member

commented Feb 13, 2019

/run-all-tests

@zz-jason zz-jason merged commit 0081e17 into pingcap:master Feb 13, 2019

13 checks passed

ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
idc-jenkins-ci-tidb/build Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/check_dev Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/common-test job succeeded
Details
idc-jenkins-ci-tidb/integration-common-test Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/integration-compatibility-test Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/integration-ddl-test Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/mybatis-test job succeeded
Details
idc-jenkins-ci-tidb/sqllogic-test-1 Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/sqllogic-test-2 Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/unit-test Jenkins job succeeded.
Details
license/cla Contributor License Agreement is signed.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants
You can’t perform that action at this time.