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: rewrite builtin function: UNIX_TIMESTAMP #4297

Merged
merged 17 commits into from Aug 31, 2017

Conversation

Projects
None yet
3 participants
@breeswish
Member

breeswish commented Aug 23, 2017

To #4080

Some test cases are commented out due to:

Show outdated Hide outdated expression/builtin_time.go
Show outdated Hide outdated expression/builtin_time.go
} else if retTp == tpDecimal {
retFLen = 12 + retDecimal
} else {
panic("Unexpected retTp")

This comment has been minimized.

@zz-jason

zz-jason Aug 23, 2017

Member

we should throw an error instead of panic

@zz-jason

zz-jason Aug 23, 2017

Member

we should throw an error instead of panic

} else if retTp == tpDecimal {
sig = &builtinUnixTimestampDecSig{baseDecimalBuiltinFunc{bf}}
} else {
panic("Unexpected retTp")

This comment has been minimized.

@zz-jason

zz-jason Aug 23, 2017

Member

ditto

@zz-jason
val, isNull, err := b.args[0].EvalTime(row, b.getCtx().GetSessionVars().StmtCtx)
if isNull || err != nil {
// Return 0 for invalid date time.
return 0, isNull, nil

This comment has been minimized.

@zz-jason

zz-jason Aug 23, 2017

Member

should we handle the invalid time error here ?

@zz-jason

zz-jason Aug 23, 2017

Member

should we handle the invalid time error here ?

This comment has been minimized.

@breeswish

breeswish Aug 23, 2017

Member

we should return zero in this function.

@breeswish

breeswish Aug 23, 2017

Member

we should return zero in this function.

val, isNull, err := b.args[0].EvalTime(row, b.getCtx().GetSessionVars().StmtCtx)
if isNull || err != nil {
// Return 0 for invalid date time.
return new(types.MyDecimal), isNull, nil

This comment has been minimized.

@zz-jason

zz-jason Aug 23, 2017

Member

ditto

@zz-jason

@zz-jason zz-jason referenced this pull request Aug 23, 2017

Closed

rewrite builtin functions #4080

150 of 150 tasks complete

breeswish added some commits Aug 28, 2017

@breeswish

This comment has been minimized.

Show comment
Hide comment
@breeswish

breeswish Aug 28, 2017

Member

@zz-jason Done, PTAL

Member

breeswish commented Aug 28, 2017

@zz-jason Done, PTAL

@zz-jason

This comment has been minimized.

Show comment
Hide comment
@zz-jason

zz-jason Aug 28, 2017

Member

@breeswish plz resolve conflicts, @XuHuaiyu PTAL

Member

zz-jason commented Aug 28, 2017

@breeswish plz resolve conflicts, @XuHuaiyu PTAL

@zz-jason

This comment has been minimized.

Show comment
Hide comment
@zz-jason

zz-jason Aug 29, 2017

Member

@breeswish since pr #4347 merged, function newBaseBuiltinFuncWithTp does not return error any more, plz update code and fix ci

Member

zz-jason commented Aug 29, 2017

@breeswish since pr #4347 merged, function newBaseBuiltinFuncWithTp does not return error any more, plz update code and fix ci

@hanfei1991

LGTM

@hanfei1991 hanfei1991 added status/LGT2 and removed status/LGT1 labels Aug 31, 2017

@hanfei1991 hanfei1991 merged commit 861d844 into master Aug 31, 2017

2 of 3 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
ci/circleci Your tests passed on CircleCI!
Details
license/cla Contributor License Agreement is signed.
Details

@hanfei1991 hanfei1991 deleted the wenxuan/exp-rewrite-datetime branch Aug 31, 2017

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