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: deduce result type for multi-argument functions like `IF` wrongly in some cases #11605

Merged
merged 3 commits into from Aug 5, 2019

Conversation

@qw4990
Copy link
Contributor

commented Aug 5, 2019

What problem does this PR solve?

Fix #11594;

When we deduce the result type for a multi-argument function like IF, we don't take signed/unsigned flag into account now;

What is changed and how it works?

If anyone of these input arguments is not unsigned, we set the result type to signed.

Check List

Tests

  • Unit test

@qw4990 qw4990 force-pushed the qw4990:fix-11594 branch from e309aeb to 924e45b Aug 5, 2019

@codecov

This comment has been minimized.

Copy link

commented Aug 5, 2019

Codecov Report

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

@@             Coverage Diff             @@
##             master     #11605   +/-   ##
===========================================
  Coverage   81.2608%   81.2608%           
===========================================
  Files           426        426           
  Lines         92176      92176           
===========================================
  Hits          74903      74903           
  Misses        11897      11897           
  Partials       5376       5376

@zz-jason zz-jason requested a review from XuHuaiyu Aug 5, 2019

qw4990 added some commits Aug 5, 2019

fixup
fixup

fixup

fixup

fixup?

@qw4990 qw4990 force-pushed the qw4990:fix-11594 branch from 924e45b to f55ecff Aug 5, 2019

@XuHuaiyu
Copy link
Contributor

left a comment

LGTM

@XuHuaiyu XuHuaiyu added the status/LGT1 label Aug 5, 2019

@zz-jason
Copy link
Member

left a comment

LGTM

@qw4990

This comment has been minimized.

Copy link
Contributor Author

commented Aug 5, 2019

/run-all-tests

@sre-bot

This comment has been minimized.

Copy link

commented Aug 5, 2019

/run-all-tests

@sre-bot sre-bot merged commit cb4b778 into pingcap:master Aug 5, 2019

14 checks passed

ci/circleci Your tests passed on CircleCI!
Details
idc-jenkins-ci-tidb/build Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/build_check_race Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/check_dev Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/check_dev_2 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
@sre-bot

This comment has been minimized.

Copy link

commented Aug 5, 2019

cherry pick to release-2.1 failed

@sre-bot

This comment has been minimized.

Copy link

commented Aug 5, 2019

cherry pick to release-3.0 in PR #11621

qw4990 added a commit to qw4990/tidb that referenced this pull request Aug 8, 2019

qw4990 added a commit to qw4990/tidb that referenced this pull request Aug 8, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.