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 #7443

Merged
merged 12 commits into from
Sep 7, 2018

Conversation

mccxj
Copy link
Contributor

@mccxj mccxj commented Aug 20, 2018

What problem does this PR solve?

#7299

What is changed and how it works?

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

Check List

Tests

  • Unit test
  • Integration test

Code changes

  • Has exported function/method change
    add exported method PathExpression#ContainsAnyAsterisk()

Related changes

waiting for pingcap/tipb#99

@jackysp
Copy link
Member

jackysp commented Aug 21, 2018

Thanks for your contribution, @mccxj !

@jackysp jackysp added contribution This PR is from a community contributor. component/document store labels Aug 21, 2018
@jackysp
Copy link
Member

jackysp commented Aug 22, 2018

pingcap/tipb#99 is merged, you can update the vendor now. @mccxj

@mccxj
Copy link
Contributor Author

mccxj commented Aug 25, 2018

@jackysp PTAL

@jackysp
Copy link
Member

jackysp commented Aug 30, 2018

/run-all-tests

Copy link
Member

@jackysp jackysp left a comment

Choose a reason for hiding this comment

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

LGTM

@jackysp
Copy link
Member

jackysp commented Sep 3, 2018

PTAL @hicqu @coocood

@jackysp jackysp added the status/LGT1 Indicates that a PR has LGTM 1. label Sep 3, 2018
@jackysp
Copy link
Member

jackysp commented Sep 4, 2018

PTAL @hicqu @coocood

@coocood
Copy link
Member

coocood commented Sep 5, 2018

LGTM

@coocood coocood added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Sep 5, 2018
@coocood
Copy link
Member

coocood commented Sep 5, 2018

/run-unit-test

return res, isNull, errors.Trace(err)
}
var pathExpr json.PathExpression
if len(b.args) == 3 {
Copy link
Member

Choose a reason for hiding this comment

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

It better to write another function signature to handle the scenario that the third argument is provided.

@zz-jason zz-jason merged commit 3f5bd3b into pingcap:master Sep 7, 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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants