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: add json into builtin if function. #4203

Merged
merged 19 commits into from Aug 29, 2017

Conversation

Projects
None yet
4 participants
@hicqu
Contributor

hicqu commented Aug 16, 2017

No description provided.

@hicqu hicqu requested review from XuHuaiyu and zz-jason Aug 16, 2017

@shenli

This comment has been minimized.

Show comment
Hide comment
@shenli

shenli Aug 16, 2017

Member

LGTM

Member

shenli commented Aug 16, 2017

LGTM

@zz-jason

plz add more test in plan/typeinfer_test.go and expression/integration_test.go

@zz-jason

This comment has been minimized.

Show comment
Hide comment
@zz-jason

zz-jason Aug 16, 2017

Member

should builtin function NULLIF consider json input as well ?

Member

zz-jason commented Aug 16, 2017

should builtin function NULLIF consider json input as well ?

@XuHuaiyu

This comment has been minimized.

Show comment
Hide comment
@XuHuaiyu

XuHuaiyu Aug 16, 2017

Contributor

Nullif has been rewritten to IF. @zz-jason

Contributor

XuHuaiyu commented Aug 16, 2017

Nullif has been rewritten to IF. @zz-jason

@zz-jason

This comment has been minimized.

Show comment
Hide comment
@zz-jason
Member

zz-jason commented Aug 16, 2017

@XuHuaiyu

address comment
rest LGTM

@hicqu

This comment has been minimized.

Show comment
Hide comment
@hicqu

hicqu Aug 17, 2017

Contributor

@zz-jason PTAL, thanks!

Contributor

hicqu commented Aug 17, 2017

@zz-jason PTAL, thanks!

@zz-jason

This comment has been minimized.

Show comment
Hide comment
@zz-jason

zz-jason Aug 26, 2017

Member

@hicqu plz resolve conflicts, @XuHuaiyu PTAL

Member

zz-jason commented Aug 26, 2017

@hicqu plz resolve conflicts, @XuHuaiyu PTAL

@zz-jason

This comment has been minimized.

Show comment
Hide comment
@zz-jason
Member

zz-jason commented Aug 26, 2017

to #4080

@XuHuaiyu

This comment has been minimized.

Show comment
Hide comment
@XuHuaiyu

XuHuaiyu Aug 28, 2017

Contributor

please resolve the conflicts.

Contributor

XuHuaiyu commented Aug 28, 2017

please resolve the conflicts.

@hicqu

This comment has been minimized.

Show comment
Hide comment
@hicqu

hicqu Aug 28, 2017

Contributor

@XuHuaiyu PTAL, thanks.

Contributor

hicqu commented Aug 28, 2017

@XuHuaiyu PTAL, thanks.

@XuHuaiyu

This comment has been minimized.

Show comment
Hide comment
@XuHuaiyu

XuHuaiyu Aug 29, 2017

Contributor

LGTM
please resolve the conflicts.

Contributor

XuHuaiyu commented Aug 29, 2017

LGTM
please resolve the conflicts.

@zz-jason zz-jason removed the status/LGT1 label Aug 29, 2017

@zz-jason

This comment has been minimized.

Show comment
Hide comment
@zz-jason

zz-jason Aug 29, 2017

Member

@hicqu ci failed due to:

expression/builtin_control.go:420: undefined: tipb.ScalarFuncSig_IfJson
expression/builtin_control.go:600: undefined: tipb.ScalarFuncSig_IfNullJson
Member

zz-jason commented Aug 29, 2017

@hicqu ci failed due to:

expression/builtin_control.go:420: undefined: tipb.ScalarFuncSig_IfJson
expression/builtin_control.go:600: undefined: tipb.ScalarFuncSig_IfNullJson
@hicqu

This comment has been minimized.

Show comment
Hide comment
@hicqu

hicqu Aug 29, 2017

Contributor

Got it, thanks. I will raise a PR in tipb.

Contributor

hicqu commented Aug 29, 2017

Got it, thanks. I will raise a PR in tipb.

@hicqu hicqu closed this Aug 29, 2017

@zz-jason

This comment has been minimized.

Show comment
Hide comment
@zz-jason

zz-jason Aug 29, 2017

Member

you can remain this pr and merge it after pingcap/tipb#55 being merged.

Member

zz-jason commented Aug 29, 2017

you can remain this pr and merge it after pingcap/tipb#55 being merged.

@zz-jason zz-jason reopened this Aug 29, 2017

hicqu and others added some commits Aug 29, 2017

@XuHuaiyu

LGTM

@XuHuaiyu XuHuaiyu merged commit c9b62fb into master Aug 29, 2017

3 checks passed

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

@hicqu hicqu deleted the qupeng/json-if-case branch Aug 29, 2017

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