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: COALESCE #4157

Merged
merged 31 commits into from Aug 22, 2017

Conversation

Projects
None yet
7 participants
@bailaohe
Contributor

bailaohe commented Aug 12, 2017

This pr rewrite builtin coalesce.

@CLAassistant

This comment has been minimized.

Show comment
Hide comment
@CLAassistant

CLAassistant Aug 12, 2017

CLA assistant check
All committers have signed the CLA.

CLAassistant commented Aug 12, 2017

CLA assistant check
All committers have signed the CLA.

@bailaohe bailaohe changed the title from rewrite coalesce builtin with new expression framework to expression: rewrite coalesce builtin with new expression framework Aug 12, 2017

@zz-jason zz-jason changed the title from expression: rewrite coalesce builtin with new expression framework to expression: rewrite builtin function: COALESCE Aug 12, 2017

@zz-jason

This comment has been minimized.

Show comment
Hide comment
@zz-jason
Member

zz-jason commented Aug 12, 2017

to #4080

Show outdated Hide outdated executor/executor_test.go Outdated
Show outdated Hide outdated expression/builtin_compare.go Outdated
@zz-jason

This comment has been minimized.

Show comment
Hide comment
@zz-jason

zz-jason Aug 12, 2017

Member

add test in plan/typeinfer_test.go

Member

zz-jason commented Aug 12, 2017

add test in plan/typeinfer_test.go

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

Closed

rewrite builtin functions #4080

150 of 150 tasks complete

bailaohe added some commits Aug 13, 2017

@bailaohe

This comment has been minimized.

Show comment
Hide comment
@bailaohe

bailaohe Aug 14, 2017

Contributor

@zz-jason PTAL

Contributor

bailaohe commented Aug 14, 2017

@zz-jason PTAL

Show outdated Hide outdated expression/builtin_compare.go Outdated
@@ -91,7 +91,8 @@ func AggTypeClass(tps []*FieldType, flag *uint) TypeClass {
continue
}
argTypeClass := argFieldType.ToClass()
if argTypeClass == ClassString && mysql.HasBinaryFlag(argFieldType.Flag) {
if (IsTypeBlob(argFieldType.Tp) || IsTypeVarchar(argFieldType.Tp) || IsTypeChar(argFieldType.Tp)) &&

This comment has been minimized.

@zz-jason

zz-jason Aug 14, 2017

Member

why modify this ?

@zz-jason

zz-jason Aug 14, 2017

Member

why modify this ?

This comment has been minimized.

@bailaohe

bailaohe Aug 14, 2017

Contributor

I found "coalesce(c_int, c_datetime)" has binary flag, which is inconsistent with mysql output

@bailaohe

bailaohe Aug 14, 2017

Contributor

I found "coalesce(c_int, c_datetime)" has binary flag, which is inconsistent with mysql output

This comment has been minimized.

@bailaohe

bailaohe Aug 14, 2017

Contributor

borrowed from aa13a8f

in pr #4158

After it's merged I'll fix the conflicts

@bailaohe

bailaohe Aug 14, 2017

Contributor

borrowed from aa13a8f

in pr #4158

After it's merged I'll fix the conflicts

This comment has been minimized.

@XuHuaiyu

XuHuaiyu Aug 22, 2017

Contributor

#4158 which has been merged seems to not modify this?
so should we keep this unchanged?
@bailaohe

@XuHuaiyu

XuHuaiyu Aug 22, 2017

Contributor

#4158 which has been merged seems to not modify this?
so should we keep this unchanged?
@bailaohe

@bailaohe

This comment has been minimized.

Show comment
Hide comment
@bailaohe

bailaohe Aug 14, 2017

Contributor

@zz-jason PTAL

Contributor

bailaohe commented Aug 14, 2017

@zz-jason PTAL

@jackysp

This comment has been minimized.

Show comment
Hide comment
@jackysp

jackysp Aug 15, 2017

Member

Please resolve the conflicts.

Member

jackysp commented Aug 15, 2017

Please resolve the conflicts.

@jackysp

This comment has been minimized.

Show comment
Hide comment
@jackysp

jackysp Aug 15, 2017

Member

Need to add json type? @hicqu

Member

jackysp commented Aug 15, 2017

Need to add json type? @hicqu

Show outdated Hide outdated expression/integration_test.go Outdated
Show outdated Hide outdated plan/typeinfer_test.go Outdated
Show outdated Hide outdated expression/builtin_compare.go Outdated
Show outdated Hide outdated expression/builtin_compare_test.go Outdated
Show outdated Hide outdated expression/builtin_compare.go Outdated
@bailaohe

This comment has been minimized.

Show comment
Hide comment
@bailaohe

bailaohe Aug 15, 2017

Contributor

@XuHuaiyu PTAL

Contributor

bailaohe commented Aug 15, 2017

@XuHuaiyu PTAL

bailaohe and others added some commits Aug 15, 2017

@bailaohe

This comment has been minimized.

Show comment
Hide comment
@bailaohe

bailaohe Aug 16, 2017

Contributor

@XuHuaiyu PTAL again

Contributor

bailaohe commented Aug 16, 2017

@XuHuaiyu PTAL again

Show outdated Hide outdated expression/builtin_compare.go Outdated
@shenli

This comment has been minimized.

Show comment
Hide comment
@shenli

shenli Aug 16, 2017

Member

@bailaohe Please resolve the conflicts. @XuHuaiyu PTAL

Member

shenli commented Aug 16, 2017

@bailaohe Please resolve the conflicts. @XuHuaiyu PTAL

@XuHuaiyu

This comment has been minimized.

Show comment
Hide comment
@XuHuaiyu

XuHuaiyu Aug 17, 2017

Contributor

ci failed, ptal @bailaohe

Contributor

XuHuaiyu commented Aug 17, 2017

ci failed, ptal @bailaohe

@bailaohe

This comment has been minimized.

Show comment
Hide comment
@bailaohe

bailaohe Aug 18, 2017

Contributor

fixed. PTAL @XuHuaiyu

Contributor

bailaohe commented Aug 18, 2017

fixed. PTAL @XuHuaiyu

bailaohe added some commits Aug 19, 2017

@XuHuaiyu

This comment has been minimized.

Show comment
Hide comment
@XuHuaiyu

XuHuaiyu Aug 22, 2017

Contributor

rest LGTM
PTAL @zz-jason @jackysp

Contributor

XuHuaiyu commented Aug 22, 2017

rest LGTM
PTAL @zz-jason @jackysp

}
// For integer, field length = maxIntLen + (1/0 for sign bit)
// For decimal, field lenght = maxIntLen + maxDecimal + (1/0 for sign bit)

This comment has been minimized.

@jackysp

jackysp Aug 22, 2017

Member

typo lenght

@jackysp

jackysp Aug 22, 2017

Member

typo lenght

@hanfei1991 hanfei1991 merged commit d0beee7 into pingcap:master Aug 22, 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

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

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