-
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: support JSON for builtin function COALESCE #9087
Conversation
Signed-off-by: Jian Zhang <zjsariel@gmail.com>
/run-all-tests |
@zz-jason CI failed, 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.
Please fix ci.
@@ -202,6 +202,9 @@ func (c *coalesceFunctionClass) getFunction(ctx sessionctx.Context, args []Expre | |||
} | |||
sig = &builtinCoalesceDurationSig{bf} | |||
sig.setPbCode(tipb.ScalarFuncSig_CoalesceDuration) | |||
case types.ETJson: | |||
sig = &builtinCoalesceJsonSig{bf} | |||
sig.setPbCode(tipb.ScalarFuncSig_CoalesceJson) |
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.
We need to update getSignatureByPB
as well.
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.
getSignatureByPB
is only used in mocktikv when this function is pushed to the coprocessor. I think we can do it later when we push it?
/run-all-tests |
Codecov Report
@@ Coverage Diff @@
## master #9087 +/- ##
==========================================
- Coverage 67.21% 67.19% -0.02%
==========================================
Files 371 371
Lines 76945 76958 +13
==========================================
- Hits 51721 51715 -6
- Misses 20606 20622 +16
- Partials 4618 4621 +3
Continue to review full report at Codecov.
|
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
Signed-off-by: Jian Zhang zjsariel@gmail.com
What problem does this PR solve?
Support JSON type for builtin function COALESCE, to avoid panic once we returned a nil pointer.
What is changed and how it works?
As the above said.
Check List
Tests
Related changes