-
Notifications
You must be signed in to change notification settings - Fork 3.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
[fix](short circurt) fix return default value issue #34186
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! 👍" |
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 test column to verify this feature
run buildall |
clang-tidy review says "All clean, LGTM! 👍" |
TeamCity be ut coverage result: |
TPC-H: Total hot run time: 41108 ms
|
TPC-DS: Total hot run time: 186957 ms
|
run buildall |
clang-tidy review says "All clean, LGTM! 👍" |
TPC-H: Total hot run time: 40682 ms
|
TeamCity be ut coverage result: |
TPC-DS: Total hot run time: 187298 ms
|
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. |
PR approved by anyone 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
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
be/src/vec/jsonb/serialize.cpp
Outdated
@@ -107,7 +107,7 @@ void JsonbSerializeUtil::jsonb_to_block(const DataTypeSerDeSPtrs& serdes, const | |||
MutableColumnPtr col = column_type_name.column->assume_mutable(); | |||
if (col->size() < num_rows + 1) { | |||
DCHECK(col->size() == num_rows); | |||
if (default_values[i].empty()) { | |||
if (default_values[i].empty() || col->empty()) { |
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 it's wrong, the col always empty for the start, so when default_values[i]
is not emtpy, it may result incorrect value
796ccaa
clang-tidy review says "All clean, LGTM! 👍" |
run buildall |
clang-tidy review says "All clean, LGTM! 👍" |
TPC-H: Total hot run time: 41644 ms
|
TPC-DS: Total hot run time: 169192 ms
|
ClickBench: Total hot run time: 30.22 s
|
TeamCity be ut coverage result: |
5f58566
to
8c54f19
Compare
clang-tidy review says "All clean, LGTM! 👍" |
TPC-H: Total hot run time: 40640 ms
|
run buildall |
clang-tidy review says "All clean, LGTM! 👍" |
TPC-H: Total hot run time: 39911 ms
|
TeamCity be ut coverage result: |
TPC-DS: Total hot run time: 172933 ms
|
ClickBench: Total hot run time: 31.07 s
|
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. |
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
Proposed changes
Issue Number: close #34056
problem
When using short circurt for point-checking, if a field in the table constructor clause has a default value and a NULL value is written later, the query returns the default value instead of the NULL value
recurrence
The query result is 1970-01-01 00:00:00.111
3、after restoration
Query returns NULL after repair
Further comments
If this is a relatively large or complex change, kick off the discussion at dev@doris.apache.org by explaining why you chose the solution you did and what alternatives you considered, etc...