| Bug #115186 | correct the return for start_result_metadata | ||
|---|---|---|---|
| Submitted: | 31 May 14:04 | Modified: | 3 Jun 9:43 |
| Reporter: | alex xing (OCA) | Email Updates: | |
| Status: | Verified | Impact on me: | |
| Category: | MySQL Server: Connection Handling | Severity: | S5 (Performance) |
| Version: | 8.0 | OS: | Any |
| Assigned to: | CPU Architecture: | Any | |
[31 May 14:04]
alex xing
[31 May 14:04]
alex xing
a simple patch to describe the optimization (*) I confirm the code being submitted is offered under the terms of the OCA, and that I am authorized to contribute it.
Contribution: optimize_net_work.patch (text/plain), 1.03 KiB.
[31 May 14:06]
MySQL Verification Team
Hi Mr . xing, Thank you for your bug report. However, we do not see a test case here at all. We do accept bug reports based on a very thorough code analysis, but we also do not see any in-depth code analysis. Can't repeat.
[1 Jun 3:34]
alex xing
in mysqld code, if my_net_write return true means something wrong in net-work and report errors to client.
Therefore, in the mysqld code, the basic logic will get the return value of my_net_write. if it is true, it will abandon the subsequent execution logic and directly report an error to the user.
But in the Protocol_classic::start_result_metadata function, however, this is not done. Instead, the protocol ignores my_net_write and returns false.
I personally find this to be unfriendly because Protocol_classic::start_result_metadata has an upper-level judgment about this return value, and if it returns false by default, this upper-level judgment is meaningless.
when user execute sql: select * from t;
Code execution stack:
my_net_write(NET * net, const uchar * packet, size_t len) (/mysql80/sql-common/net_serv.cc:444)
Protocol_classic::start_result_metadata(Protocol_classic * const this, uint num_cols_arg, uint flags, const CHARSET_INFO * cs) (/mysql80/sql/protocol_classic.cc:3310)
THD::send_result_metadata(THD * const this, const mem_root_deque<Item*> & list, uint flags) (/mysql80/sql/sql_class.cc:3186)
Query_result_send::send_result_set_metadata(Query_result_send * const this, THD * thd, const mem_root_deque<Item*> & list, uint flags) (/mysql80/sql/query_result.cc:71)
Query_expression::ExecuteIteratorQuery(Query_expression * const this, THD * thd) (/mysql80/sql/sql_union.cc:1691)
Query_expression::execute(Query_expression * const this, THD * thd) (/mysql80/sql/sql_union.cc:1824)
Sql_cmd_dml::execute_inner(Sql_cmd_dml * const this, THD * thd) (/mysql80/sql/sql_select.cc:785)
Sql_cmd_dml::execute(Sql_cmd_dml * const this, THD * thd) (/mysql80/sql/sql_select.cc:585)
mysql_execute_command(THD * thd, bool first_level) (/mysql80/sql/sql_parse.cc:5103)
dispatch_sql_command(THD * thd, Parser_state * parser_state, bool update_userstat) (/mysql80/sql/sql_parse.cc:5752)
dispatch_command(THD * thd, const COM_DATA * com_data, enum_server_command command) (/mysql80/sql/sql_parse.cc:2214)
do_command(THD * thd) (/mysql80/sql/sql_parse.cc:1560)
handle_connection(void * arg) (/mysql80/sql/conn_handler/connection_handler_per_thread.cc:399)
pfs_spawn_thread(void * arg) (/mysql80/storage/perfschema/pfs.cc:2987)
libpthread.so.0!start_thread(void * arg) (/build/glibc-77giwP/glibc-2.24/nptl/pthread_create.c:456)
libc.so.6!clone() (/build/glibc-77giwP/glibc-2.24/sysdeps/unix/sysv/linux/x86_64/clone.S:97)
Pseudo-code for Protocol_classic::start_result_metadata：
bool Protocol_classic::start_result_metadata(uint num_cols_arg, uint flags,
const CHARSET_INFO *cs) {
my_net_write(&m_thd->net, (uchar *)&tmp, (size_t)(pos - (uchar *)&tmp));
return false;
}
Pseudo-code for Protocol_classic::start_result_metadata：
bool Protocol_classic::start_result_metadata(uint num_cols_arg, uint flags,
const CHARSET_INFO *cs) {
my_net_write(&m_thd->net, (uchar *)&tmp, (size_t)(pos - (uchar *)&tmp));
return false;
}
Pseudo-code for THD::send_result_metadata：
THD::send_result_metadata(){
//important: Because of the above description, it will never goto the goto err
if (m_protocol->start_result_metadata(CountVisibleFields(list), flags,
variables.character_set_results))
goto err;
}
[3 Jun 9:43]
MySQL Verification Team
Hi Mr. xing, We have taken time and have analysed deeply you analysis. We fully agree with it. This is now a verified bug for the versions 8.0 and higher. Thank you ........
