-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
[improve](json)improve json support empty keys #36762
[improve](json)improve json support empty keys #36762
Conversation
Thank you for your contribution to Apache Doris. Since 2024-03-18, the Document has been moved to doris-website. |
clang-tidy review says "All clean, LGTM! 👍" |
run buildall |
TPC-H: Total hot run time: 40254 ms
|
TPC-DS: Total hot run time: 172016 ms
|
ClickBench: Total hot run time: 30.99 s
|
be/src/util/jsonb_utils.h
Outdated
@@ -191,7 +191,11 @@ class JsonbToJson { | |||
if (iter->klen()) { | |||
string_to_json(iter->getKeyStr(), iter->klen()); | |||
} else { | |||
os_.write(iter->getKeyId()); | |||
if (iter->getKeyId() == 0) { |
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.
is key == 0 special ? if special add comment to explain
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.
Yes, we do not use keyid before , just use keystr for jsonkey, and se I make keyid == 0 is special for empty key
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.
maybe use sMaxKeyId for safe and readable, 0 is not too special
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.
OK I will try
be/src/util/jsonb_writer.h
Outdated
os_->write(key, len); | ||
size += len; | ||
if (len == 0) { | ||
JsonbKeyValue::keyid_type idx = 0; |
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.
use sMaxKeyId
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 comment
clang-tidy review says "All clean, LGTM! 👍" |
run buildall |
TPC-H: Total hot run time: 40231 ms
|
TPC-DS: Total hot run time: 174148 ms
|
ClickBench: Total hot run time: 31.33 s
|
clang-tidy review says "All clean, LGTM! 👍" |
1 similar comment
clang-tidy review says "All clean, LGTM! 👍" |
run buildall |
clang-tidy review says "All clean, LGTM! 👍" |
TPC-H: Total hot run time: 40117 ms
|
TPC-DS: Total hot run time: 173351 ms
|
ClickBench: Total hot run time: 30.72 s
|
run buildall |
clang-tidy review says "All clean, LGTM! 👍" |
run buildall |
clang-tidy review says "All clean, LGTM! 👍" |
TPC-H: Total hot run time: 39762 ms
|
TPC-DS: Total hot run time: 174348 ms
|
ClickBench: Total hot run time: 30.66 s
|
PR approved by at least one committer and no changes requested. |
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
support empty key for json behavior reference MSYQL
Proposed changes
support empty key for json
behavior reference MSYQL
Issue Number: close #xxx