-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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: cast json_extract as bool #18948
Conversation
Codecov Report
@@ Coverage Diff @@
## master #18948 +/- ##
===========================================
Coverage 79.4013% 79.4013%
===========================================
Files 546 546
Lines 148762 148762
===========================================
Hits 118119 118119
Misses 21173 21173
Partials 9470 9470 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should add some test cases into code.
@SunRunAway PTAL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you like to add these following cases?
insert into testjson set j='""';
insert into testjson set j='null';
insert into testjson set j='0';
select * from testjson where j;
I've added some test cases. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
/merge |
/run-all-tests |
What problem does this PR solve?
Issue Number: close #12233
Problem Summary:
What is changed and how it works?
Proposal: xxx
What's Changed:
How it Works:
Related changes
pingcap/docs
/pingcap/docs-cn
:Check List
Tests
Side effects
Release note