Skip to content
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: support builtin function json_contains_path #7596

Merged
merged 3 commits into from
Sep 9, 2018

Conversation

xiangyuf
Copy link
Contributor

@xiangyuf xiangyuf commented Sep 3, 2018

What problem does this PR solve?

#7546

add builtin function: JSON_CONTAINS_PATH

What is changed and how it works?

add builtin function json_contains_path according to https://dev.mysql.com/doc/refman/5.7/en/json-search-functions.html#function_json-contains-path

Check List

Tests

  • Unit test
  • Integration test

Code changes

  • Has exported function/method change

@sre-bot
Copy link
Contributor

sre-bot commented Sep 3, 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.

@CLAassistant
Copy link

CLAassistant commented Sep 3, 2018

CLA assistant check
All committers have signed the CLA.

@xiangyuf xiangyuf force-pushed the expression/json_contains_path branch from 8dc2a67 to fe84f88 Compare September 3, 2018 16:59
@shenli shenli added the contribution This PR is from a community contributor. label Sep 4, 2018
@shenli
Copy link
Member

shenli commented Sep 4, 2018

@xiangyuf Thanks!

@shenli
Copy link
Member

shenli commented Sep 4, 2018

@xiangyuf We need to merge the PR in the tipb repo first.

@xiangyuf
Copy link
Contributor Author

xiangyuf commented Sep 4, 2018

@shenli sure.

expression/integration_test.go Outdated Show resolved Hide resolved
expression/builtin_json.go Outdated Show resolved Hide resolved
@zz-jason zz-jason added component/expression type/enhancement The issue or PR belongs to an enhancement. labels Sep 5, 2018
types/json/constants.go Outdated Show resolved Hide resolved
expression/builtin_json.go Outdated Show resolved Hide resolved
expression/builtin_json.go Outdated Show resolved Hide resolved
@xiangyuf xiangyuf force-pushed the expression/json_contains_path branch from fe84f88 to cbc8123 Compare September 6, 2018 17:12
@xiangyuf
Copy link
Contributor Author

xiangyuf commented Sep 6, 2018

PTAL @zz-jason @mccxj

@xiangyuf
Copy link
Contributor Author

xiangyuf commented Sep 6, 2018

Waiting for tipb update: pingcap/tipb#102

@zz-jason
Copy link
Member

zz-jason commented Sep 7, 2018

@xiangyuf please merge master and resolve the conflicts.

expression/builtin_json.go Outdated Show resolved Hide resolved
expression/builtin_json.go Outdated Show resolved Hide resolved
@xiangyuf xiangyuf force-pushed the expression/json_contains_path branch 2 times, most recently from 9a2eeda to e378ef2 Compare September 7, 2018 15:36
@xiangyuf
Copy link
Contributor Author

xiangyuf commented Sep 7, 2018

@zz-jason Done

@xiangyuf xiangyuf force-pushed the expression/json_contains_path branch from e378ef2 to 2129f31 Compare September 7, 2018 16:05
expression/builtin_json.go Outdated Show resolved Hide resolved
@xiangyuf xiangyuf force-pushed the expression/json_contains_path branch from 2129f31 to e407f66 Compare September 7, 2018 16:30
spongedu
spongedu approved these changes Sep 8, 2018
Copy link
Contributor

@spongedu spongedu left a comment

Choose a reason for hiding this comment

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

Thanks a lot! LGTM. @zz-jason PTAL

Copy link
Member

@zz-jason zz-jason left a comment

Choose a reason for hiding this comment

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

LGTM

@zz-jason
Copy link
Member

zz-jason commented Sep 8, 2018

/run-all-tests

@zz-jason zz-jason added the status/LGT2 Indicates that a PR has LGTM 2. label Sep 8, 2018
@ngaut ngaut merged commit 30ae420 into pingcap:master Sep 9, 2018
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. type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants