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, executor, plan: rewrite builtin function hex #3794

Merged
merged 3 commits into from Jul 19, 2017

Conversation

Projects
None yet
4 participants
@SteveZhangBit
Contributor

SteveZhangBit commented Jul 18, 2017

No description provided.

@shenli

This comment has been minimized.

Show comment
Hide comment
Member

shenli commented Jul 18, 2017

LGTM
@XuHuaiyu @zz-jason PTAL

if err != nil {
return nil, errors.Trace(err)
}
// Use UTF-8 as default

This comment has been minimized.

@XuHuaiyu

XuHuaiyu Jul 19, 2017

Contributor

what does this comment mean?
why set flen = arg0.Flen*3*2 ?

@XuHuaiyu

XuHuaiyu Jul 19, 2017

Contributor

what does this comment mean?
why set flen = arg0.Flen*3*2 ?

This comment has been minimized.

@SteveZhangBit

SteveZhangBit Jul 19, 2017

Contributor

hex(str) will return the hex string of the original string byte by byte, so for utf-8 string, it should be *3 for each rune and *2 for each byte.

@SteveZhangBit

SteveZhangBit Jul 19, 2017

Contributor

hex(str) will return the hex string of the original string byte by byte, so for utf-8 string, it should be *3 for each rune and *2 for each byte.

@XuHuaiyu

This comment has been minimized.

Show comment
Hide comment
@XuHuaiyu

XuHuaiyu Jul 19, 2017

Contributor

LGTM

Contributor

XuHuaiyu commented Jul 19, 2017

LGTM

@XuHuaiyu XuHuaiyu added status/LGT2 and removed status/LGT1 labels Jul 19, 2017

@shenli

shenli approved these changes Jul 19, 2017

@shenli shenli merged commit e7f9752 into pingcap:master Jul 19, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla Contributor License Agreement is signed.
Details

dbjoa added a commit to cloud-pi/tidb that referenced this pull request Jul 20, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment