-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
mysql/json:support json_type #1937
Conversation
|
||
use super::Json; | ||
|
||
const JSON_TYPE_BOOLEAN: &[u8] = b"BOOLEAN"; |
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.
&'static [u8]
LGTM |
#[cfg(test)] | ||
mod test { | ||
use super::*; | ||
#[test] |
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.
Add a blank line before attribute.
use super::Json; | ||
|
||
const JSON_TYPE_BOOLEAN: &[u8] = b"BOOLEAN"; | ||
const JSON_TYPE_NONE: &[u8] = b"NULL"; |
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.
why not use str here?
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.
Since we only need binary
in tikv
. And if we use str
here, it would still be converted to binary
after json_type
@siddontang
const JSON_TYPE_ARRAY: &[u8] = b"ARRAY"; | ||
|
||
impl Json { | ||
pub fn json_type(&self) -> &[u8] { |
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.
IMO, the type should be an enum or integer, but here it is a &[u8]
, do we use this for comparison?
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.
no, it's an attribute function for json
in sql: https://dev.mysql.com/doc/refman/5.7/en/json-attribute-functions.html#function_json-type
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
Hi,
This PR implement
json_type
forjson
. Sincetidb
haven't implementjson_type
in evaluator, so we would implement this part later. The related source file intidb
is (https://github.com/pingcap/tidb/blob/master/util/types/json/functions.go#L25)@andelf @hhkbp2 @hicqu @lishihai9017 PTAL