-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[Fix](Row store) fix row store with invalid json string in variant type #39394
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
Conversation
Previous we allow invalid text as variant in PR apache#37794 and store as string type.But in encoding rowstore we CHECK the json is valid and store as jsonb binary field.In this PR we support the invalid json encoding as row store
|
Thank you for your contribution to Apache Doris. Since 2024-03-18, the Document has been moved to doris-website. |
|
run buildall |
TPC-H: Total hot run time: 37983 ms |
TPC-H: Total hot run time: 37434 ms |
TPC-DS: Total hot run time: 189051 ms |
ClickBench: Total hot run time: 30.39 s |
amorynan
left a comment
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
|
PR approved by anyone and no changes requested. |
xiaokang
left a comment
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
|
PR approved by at least one committer and no changes requested. |
| bool succ = json_parser.parse(value_str.data(), value_str.size()); | ||
| // maybe more graceful, it is ok to check here since data could be parsed | ||
| CHECK(succ); | ||
| if (!succ) { |
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.
if (succ) {
// write binary
} else {
// write string
}
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.
done
2f48c66 to
323277a
Compare
|
clang-tidy review says "All clean, LGTM! 👍" |
1 similar comment
|
clang-tidy review says "All clean, LGTM! 👍" |
|
run buildall |
TPC-H: Total hot run time: 38409 ms |
xiaokang
left a comment
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
|
PR approved by at least one committer and no changes requested. |
TPC-DS: Total hot run time: 196570 ms |
ClickBench: Total hot run time: 31.58 s |
…pe (apache#39394) Previous we allow invalid text as variant in PR apache#37794 and store as string type.But in encoding rowstore we CHECK the json is valid and store as jsonb binary field.In this PR we support the invalid json encoding as row store
…pe (apache#39394) Previous we allow invalid text as variant in PR apache#37794 and store as string type.But in encoding rowstore we CHECK the json is valid and store as jsonb binary field.In this PR we support the invalid json encoding as row store
Previous we allow invalid text as variant in PR #37794 and store as string type.But in encoding rowstore we CHECK the json is valid and store as jsonb binary field.In this PR we support the invalid json encoding as row store
Proposed changes
Issue Number: close #xxx