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: add missing setPbCode() for some arithmetic function #12864

Merged
merged 3 commits into from
Oct 22, 2019
Merged

expression: add missing setPbCode() for some arithmetic function #12864

merged 3 commits into from
Oct 22, 2019

Conversation

lonng
Copy link
Contributor

@lonng lonng commented Oct 22, 2019

Signed-off-by: Lonng [email protected]

What problem does this PR solve?

The MOD function does not call the setPbCode while pushing down to coprocessor, which will cause an unexpected error: Error 1105: other error: [components/tidb_query/src/expr/scalar_function.rs:516]: unexpected arguments: sig CastIntAsInt with 2 args.

What is changed and how it works?

Add the missing call.

Check List

Tests

  • Unit test
  • Integration test

Release note

  • Add the missing setPbCode call for the MOD function to avoid unexpected error while pushing the MOD to the coprocessor.

@lonng lonng requested a review from a team as a code owner October 22, 2019 04:57
@ghost ghost requested review from wshwsh12 and removed request for a team October 22, 2019 04:57
@lonng
Copy link
Contributor Author

lonng commented Oct 22, 2019

/run-all-tests

@lonng
Copy link
Contributor Author

lonng commented Oct 22, 2019

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

@codecov
Copy link

codecov bot commented Oct 22, 2019

Codecov Report

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

@@             Coverage Diff             @@
##             master     #12864   +/-   ##
===========================================
  Coverage   80.1368%   80.1368%           
===========================================
  Files           465        465           
  Lines        107858     107858           
===========================================
  Hits          86434      86434           
  Misses        14983      14983           
  Partials       6441       6441

Copy link
Contributor

@qw4990 qw4990 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@H-ZeX H-ZeX added the status/can-merge Indicates a PR has been approved by a committer. label Oct 22, 2019
@H-ZeX H-ZeX removed the status/can-merge Indicates a PR has been approved by a committer. label Oct 22, 2019
@sre-bot
Copy link
Contributor

sre-bot commented Oct 22, 2019

/run-all-tests

@H-ZeX H-ZeX changed the title expression: add missing setPbCode() for MOD function expression: add missing setPbCode() for some arithmetic function Oct 22, 2019
@H-ZeX H-ZeX added the status/can-merge Indicates a PR has been approved by a committer. label Oct 22, 2019
@sre-bot
Copy link
Contributor

sre-bot commented Oct 22, 2019

/run-all-tests

@sre-bot sre-bot merged commit 5ea9834 into pingcap:master Oct 22, 2019
@lonng lonng deleted the fix-pbcode branch October 22, 2019 23:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/expression status/can-merge Indicates a PR has been approved by a committer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants