[opt](mysql serde) Avoid core dump when converting invalid block to mysql result #28069
Conversation
|
clang-tidy review says "All clean, LGTM! 👍" |
|
run buildall |
|
PR approved by at least one committer and no changes requested. |
github-actions
bot
added
approved
|
PR approved by anyone and no changes requested. |
| for (int i = 0; i < _output_vexpr_ctxs.size(); ++i) { | ||
| RETURN_IF_ERROR(arguments[i].serde->write_column_to_mysql( | ||
| *(arguments[i].column), row_buffer, row_idx, arguments[i].is_const)); | ||
| for (size_t row_idx = 0; row_idx < num_rows; ++row_idx) { |
There was a problem hiding this comment.
My fault.
Check loop is divided from for loop and placed in the front.
|
(From new machine)TeamCity pipeline, clickbench performance test result: |
github-actions
bot
removed
the
approved
|
run buildall |
|
clang-tidy review says "All clean, LGTM! 👍" |
1 similar comment
|
clang-tidy review says "All clean, LGTM! 👍" |
|
(From new machine)TeamCity pipeline, clickbench performance test result: |
TPC-H test result on machine: 'aliyun_ecs.c7a.8xlarge_32C64G' |
TPC-H test result on machine: 'aliyun_ecs.c7a.8xlarge_32C64G' |
zhiqiang-hhhh
force-pushed
the
opt-ser
branch
from
8b9153c
to
06c78ae
Compare
|
run buildall |
|
clang-tidy review says "All clean, LGTM! 👍" |
|
(From new machine)TeamCity pipeline, clickbench performance test result: |
TPC-H test result on machine: 'aliyun_ecs.c7a.8xlarge_32C64G' |
|
PR approved by at least one committer and no changes requested. |
github-actions
bot
added
the
approved
zhiqiang-hhhh
force-pushed
the
opt-ser
branch
from
06c78ae
to
9bb3606
Compare
github-actions
bot
removed
the
approved
|
run buildall |
|
clang-tidy review says "All clean, LGTM! 👍" |
TPC-H test result on machine: 'aliyun_ecs.c7a.8xlarge_32C64G' |
| const auto col_index = index_check_const(row_idx, col_const); | ||
| const auto row_idx_of_col_arr = index_check_const(row_idx_of_mysql, col_const); | ||
|
|
||
| if (column_array.size() <= row_idx_of_col_arr) { |
There was a problem hiding this comment.
I do not think here should add this check, if here add , string serde should add same check too.
There was a problem hiding this comment.
Yes, we should add check for all serde if we can.
There was a problem hiding this comment.
if u add array , do same with map too.
There was a problem hiding this comment.
check is removed since it is checked per row.
this pr just fixed mysql result writer, and we have other writer types, maybe they are subjected to the same problem
|
(From new machine)TeamCity pipeline, clickbench performance test result: |
|
run buildall |
|
clang-tidy review says "All clean, LGTM! 👍" |
|
(From new machine)TeamCity pipeline, clickbench performance test result: |
TPC-H test result on machine: 'aliyun_ecs.c7a.8xlarge_32C64G' |
github-actions
bot
added
the
approved
|
PR approved by at least one committer and no changes requested. |
…ysql result (apache#28069) BE will core dump if result block is invalid when we doing result serialization. An existing bug case is described in apache#28030, so we add check branch to avoid BE core dump due to out of range related problem.
…ysql result (apache#28069) BE will core dump if result block is invalid when we doing result serialization. An existing bug case is described in apache#28030, so we add check branch to avoid BE core dump due to out of range related problem.
BE will core dump if result block is invalid when we doing result serialization.
An existing bug case is described in #28030, so we add check branch to avoid BE core dump due to out of range related problem.