Skip to content

Conversation

yu34po
Copy link
Contributor

@yu34po yu34po commented Oct 26, 2018

What problem does this PR solve?

fix #7591
fix #7615

What is changed and how it works?

1.add an builtinTruncateUintSig Function to support unsigned int truncate
2.change builtinTruncateIntSig truncate function to avoid get incorrect value

Check List

Tests

  • Unit test
  • Integration test

Code changes

  • Has exported function/method change

Side effects

Related changes

@sre-bot
Copy link
Contributor

sre-bot commented Oct 26, 2018

Hi contributor, thanks for your PR.

This patch needs to be approved by someone of admins. They should reply with "/ok-to-test" to accept this PR for running test automatically.

@winoros
Copy link
Member

winoros commented Oct 29, 2018

@yu34po Please fill the pr description.

@XuHuaiyu
Copy link
Contributor

@yu34po
We need do some modification, since the input argument type if types.Row:
func (b *builtinTruncateIntSig) evalInt(row types.Row) (int64, bool, error) {...}

@lysu
Copy link
Contributor

lysu commented Oct 30, 2018

/run-all-tests tidb-test=release-2.0 tikv=release-2.0 pd=release-2.0

1 similar comment
@WangXiangUSTC
Copy link
Contributor

/run-all-tests tidb-test=release-2.0 tikv=release-2.0 pd=release-2.0

@zz-jason
Copy link
Member

LGTM
@XuHuaiyu PTAL

@zz-jason zz-jason added status/LGT1 Indicates that a PR has LGTM 1. contribution This PR is from a community contributor. labels Oct 31, 2018
Copy link
Contributor

@XuHuaiyu XuHuaiyu left a comment

Choose a reason for hiding this comment

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

LGTM

@XuHuaiyu XuHuaiyu added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Oct 31, 2018
@zz-jason zz-jason merged commit 0a37bd5 into pingcap:release-2.0 Oct 31, 2018
@yu34po yu34po deleted the release-2.0 branch October 31, 2018 13:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/expression contribution This PR is from a community contributor. status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants