Skip to content
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

MySQL 8.0.36 is released #1019

Merged
merged 1 commit into from
Jan 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 78 additions & 0 deletions patches/mysql/8.0.36/skip-install-pdb.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
diff --git a/cmake/install_macros.cmake b/cmake/install_macros.cmake
index 859715a45c4..3d5c8064531 100644
--- a/cmake/install_macros.cmake
+++ b/cmake/install_macros.cmake
@@ -22,33 +22,8 @@

# For windows: install .pdb file for each target.
MACRO(INSTALL_DEBUG_SYMBOLS target)
- IF(MSVC)
- GET_TARGET_PROPERTY(type ${target} TYPE)
- IF(NOT INSTALL_LOCATION)
- IF(type MATCHES "STATIC_LIBRARY"
- OR type MATCHES "MODULE_LIBRARY"
- OR type MATCHES "SHARED_LIBRARY")
- SET(INSTALL_LOCATION "lib")
- ELSEIF(type MATCHES "EXECUTABLE")
- SET(INSTALL_LOCATION "bin")
- ELSE()
- MESSAGE(FATAL_ERROR
- "cannot determine type of ${target}. Don't now where to install")
- ENDIF()
- ENDIF()
-
- IF(target STREQUAL "mysqld" OR target STREQUAL "mysqlbackup")
- SET(comp Server)
- ELSE()
- SET(comp Debuginfo)
- ENDIF()
-
- # No .pdb file for static libraries.
- IF(NOT type MATCHES "STATIC_LIBRARY")
- INSTALL(FILES $<TARGET_PDB_FILE:${target}>
- DESTINATION ${INSTALL_LOCATION} COMPONENT ${comp})
- ENDIF()
- ENDIF()
+# .pdb files are too large to use in GitHub Actions.
+# so skip installing
ENDMACRO()
Comment on lines +8 to +38
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The modification to the INSTALL_DEBUG_SYMBOLS macro effectively disables the installation of .pdb files by commenting out the original logic and replacing it with a comment explaining the rationale. While this achieves the goal of not installing .pdb files, it removes the flexibility of installing debug symbols when needed. Consider adding a conditional check or a configuration option to allow users to opt-in for .pdb file installation if required for debugging purposes.

+# .pdb files are too large to use in GitHub Actions.
+# so skip installing
+IF(NOT DISABLE_PDB_INSTALLATION)
 ENDMACRO()
+ENDIF()

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
MACRO(INSTALL_DEBUG_SYMBOLS target)
- IF(MSVC)
- GET_TARGET_PROPERTY(type ${target} TYPE)
- IF(NOT INSTALL_LOCATION)
- IF(type MATCHES "STATIC_LIBRARY"
- OR type MATCHES "MODULE_LIBRARY"
- OR type MATCHES "SHARED_LIBRARY")
- SET(INSTALL_LOCATION "lib")
- ELSEIF(type MATCHES "EXECUTABLE")
- SET(INSTALL_LOCATION "bin")
- ELSE()
- MESSAGE(FATAL_ERROR
- "cannot determine type of ${target}. Don't now where to install")
- ENDIF()
- ENDIF()
-
- IF(target STREQUAL "mysqld" OR target STREQUAL "mysqlbackup")
- SET(comp Server)
- ELSE()
- SET(comp Debuginfo)
- ENDIF()
-
- # No .pdb file for static libraries.
- IF(NOT type MATCHES "STATIC_LIBRARY")
- INSTALL(FILES $<TARGET_PDB_FILE:${target}>
- DESTINATION ${INSTALL_LOCATION} COMPONENT ${comp})
- ENDIF()
- ENDIF()
+# .pdb files are too large to use in GitHub Actions.
+# so skip installing
ENDMACRO()
MACRO(INSTALL_DEBUG_SYMBOLS target)
+# .pdb files are too large to use in GitHub Actions.
+# so skip installing
+IF(NOT DISABLE_PDB_INSTALLATION)
ENDMACRO()
+ENDIF()



@@ -274,23 +249,6 @@ FUNCTION(INSTALL_DEBUG_TARGET target)
ENDIF()
ENDIF()

- # For windows, install .pdb files for .exe and .dll files.
- IF(MSVC AND NOT target_type STREQUAL "STATIC_LIBRARY")
- GET_FILENAME_COMPONENT(ext ${debug_target_location} EXT)
- STRING(REPLACE "${ext}" ".pdb"
- debug_pdb_target_location "${debug_target_location}" )
- IF (RENAME_PARAM)
- STRING(REPLACE "${ext}" ".pdb" pdb_rename "${ARG_RENAME}")
- SET(PDB_RENAME_PARAM RENAME "${pdb_rename}")
- ENDIF()
-
- INSTALL(FILES ${debug_pdb_target_location}
- DESTINATION ${ARG_DESTINATION}
- ${PDB_RENAME_PARAM}
- CONFIGURATIONS Release RelWithDebInfo
- COMPONENT ${ARG_COMPONENT}
- OPTIONAL)
- ENDIF()
ENDFUNCTION()
Comment on lines +45 to +62
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes in the INSTALL_DEBUG_TARGET function also remove the installation of .pdb files for .exe and .dll files. Similar to the previous comment, this change lacks flexibility for scenarios where debug symbols might be necessary. It's advisable to introduce a conditional check or configuration option here as well, to maintain the option of installing .pdb files when needed.

+# For windows, install .pdb files for .exe and .dll files.
+IF(NOT DISABLE_PDB_INSTALLATION)
 IF(MSVC AND NOT target_type STREQUAL "STATIC_LIBRARY")
 ...
 ENDIF()
+ENDIF()

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
- # For windows, install .pdb files for .exe and .dll files.
- IF(MSVC AND NOT target_type STREQUAL "STATIC_LIBRARY")
- GET_FILENAME_COMPONENT(ext ${debug_target_location} EXT)
- STRING(REPLACE "${ext}" ".pdb"
- debug_pdb_target_location "${debug_target_location}" )
- IF (RENAME_PARAM)
- STRING(REPLACE "${ext}" ".pdb" pdb_rename "${ARG_RENAME}")
- SET(PDB_RENAME_PARAM RENAME "${pdb_rename}")
- ENDIF()
-
- INSTALL(FILES ${debug_pdb_target_location}
- DESTINATION ${ARG_DESTINATION}
- ${PDB_RENAME_PARAM}
- CONFIGURATIONS Release RelWithDebInfo
- COMPONENT ${ARG_COMPONENT}
- OPTIONAL)
- ENDIF()
ENDFUNCTION()
ENDFUNCTION()



diff --git a/router/cmake/Plugin.cmake b/router/cmake/Plugin.cmake
index 30ba9fa7eaf..4b49751a124 100644
--- a/router/cmake/Plugin.cmake
+++ b/router/cmake/Plugin.cmake
@@ -150,9 +150,6 @@ FUNCTION(add_harness_plugin NAME)
INSTALL(TARGETS ${NAME}
RUNTIME DESTINATION ${_option_DESTINATION}
COMPONENT Router)
- INSTALL(FILES $<TARGET_PDB_FILE:${NAME}>
- DESTINATION ${_option_DESTINATION}
- COMPONENT Router)
Comment on lines +73 to +75
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The removal of .pdb file installation for the Router component in the add_harness_plugin function follows the same pattern as the previous modifications. While it aligns with the objective of reducing the installation package size, consider providing a mechanism to optionally include these files for advanced debugging scenarios.

+IF(NOT DISABLE_PDB_INSTALLATION)
       INSTALL(FILES $<TARGET_PDB_FILE:${NAME}>
         DESTINATION ${_option_DESTINATION}
         COMPONENT Router)
+ENDIF()

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
- INSTALL(FILES $<TARGET_PDB_FILE:${NAME}>
- DESTINATION ${_option_DESTINATION}
- COMPONENT Router)
IF(NOT DISABLE_PDB_INSTALLATION)
INSTALL(FILES $<TARGET_PDB_FILE:${NAME}>
DESTINATION ${_option_DESTINATION}
COMPONENT Router)
ENDIF()

ELSE()
INSTALL(TARGETS ${NAME}
LIBRARY DESTINATION ${_option_DESTINATION}
Original file line number Diff line number Diff line change
@@ -0,0 +1,265 @@
commit ee1e935b8a4606a943285376a08c815efbaeeb31
Author: ICHINOSE Shogo <shogo82148@gmail.com>
Date: Wed Oct 4 13:23:49 2023 +0900

Revert "Bug#35115810 stmt-execute with new-params-bound = 0 [2/2]"

This reverts commit cce8da20cb5fc12e831a66e725fdc9c0c43ad7c6.

diff --git a/router/src/mysql_protocol/include/mysqlrouter/classic_protocol_codec_base.h b/router/src/mysql_protocol/include/mysqlrouter/classic_protocol_codec_base.h
index 1065e59927e..e536b4cf70e 100644
--- a/router/src/mysql_protocol/include/mysqlrouter/classic_protocol_codec_base.h
+++ b/router/src/mysql_protocol/include/mysqlrouter/classic_protocol_codec_base.h
@@ -110,7 +110,6 @@ stdx::expected<size_t, std::error_code> encode(const T &v,
*
* @param buffer buffer to read from
* @param caps protocol capabilities
- * @tparam T the message class
* @returns number of bytes read from 'buffers' and a T on success, or
* std::error_code on error
*/
@@ -120,28 +119,6 @@ stdx::expected<std::pair<size_t, T>, std::error_code> decode(
return Codec<T>::decode(buffer, caps);
}

-/**
- * decode a message from a buffer.
- *
- * @param buffer buffer to read from
- * @param caps protocol capabilities
- * @param args arguments that shall be forwarded to T's decode()
- * @tparam T the message class
- * @tparam Args Types of the extra arguments to be forwarded to T's decode()
- * function.
- * @returns number of bytes read from 'buffers' and a T on success, or
- * std::error_code on error
- */
-template <class T, class... Args>
-stdx::expected<std::pair<size_t, T>, std::error_code> decode(
- const net::const_buffer &buffer, capabilities::value_type caps,
- // clang-format off
- Args &&... args
- // clang-format on
-) {
- return Codec<T>::decode(buffer, caps, std::forward<Args>(args)...);
-}
-
namespace impl {

/**
diff --git a/router/src/mysql_protocol/include/mysqlrouter/classic_protocol_codec_message.h b/router/src/mysql_protocol/include/mysqlrouter/classic_protocol_codec_message.h
index feabf47dd56..1d5b6b19566 100644
--- a/router/src/mysql_protocol/include/mysqlrouter/classic_protocol_codec_message.h
+++ b/router/src/mysql_protocol/include/mysqlrouter/classic_protocol_codec_message.h
@@ -2381,35 +2381,23 @@ class Codec<borrowable::message::client::StmtExecute<Borrowed>>
auto new_params_bound_res = accu.template step<bw::FixedInt<1>>();
if (!accu.result()) return stdx::make_unexpected(accu.result().error());

- std::vector<typename value_type::ParamDef> types;
-
auto new_params_bound = new_params_bound_res->value();
- if (new_params_bound == 0) {
- // no new params, use the last known params.
- types = *metadata_res;
- } else if (new_params_bound == 1) {
- // check that there is at least enough data for the types (a FixedInt<2>)
- // before reserving memory.
- if (param_count >= buffer.size() / 2) {
- return stdx::make_unexpected(
- make_error_code(codec_errc::invalid_input));
- }
+ if (new_params_bound != 1) {
+ // new-params-bound is required as long as the decoder doesn't know the
+ // old param-defs
+ return stdx::make_unexpected(make_error_code(codec_errc::invalid_input));
+ }

+ std::vector<typename value_type::ParamDef> types;
+
+ if (new_params_bound == 1) {
types.reserve(param_count);

for (size_t n{}; n < param_count; ++n) {
auto type_res = accu.template step<bw::FixedInt<2>>();
if (!accu.result()) return stdx::make_unexpected(accu.result().error());

- if (supports_query_attributes) {
- auto name_res = accu.template step<bw::VarString<Borrowed>>();
- if (!accu.result()) {
- return stdx::make_unexpected(accu.result().error());
- }
- types.emplace_back(type_res->value(), name_res->value());
- } else {
- types.emplace_back(type_res->value());
- }
+ types.push_back(type_res->value());
}
} else {
return stdx::make_unexpected(make_error_code(codec_errc::invalid_input));
diff --git a/router/src/routing/src/classic_frame.h b/router/src/routing/src/classic_frame.h
index 29adcf6c503..c693c5d9bd5 100644
--- a/router/src/routing/src/classic_frame.h
+++ b/router/src/routing/src/classic_frame.h
@@ -108,57 +108,4 @@ class ClassicFrame {
}
};

-/**
- * receive a StmtExecute message from a channel.
- *
- * specialization of recv_msg<> as StmtExecute needs a the data from the
- * StmtPrepareOk.
- */
-template <>
-inline stdx::expected<classic_protocol::borrowed::message::client::StmtExecute,
- std::error_code>
-ClassicFrame::recv_msg<
- classic_protocol::borrowed::message::client::StmtExecute>(
- Channel *src_channel, ClassicProtocolState *src_protocol,
- classic_protocol::capabilities::value_type caps) {
- using msg_type = classic_protocol::borrowed::message::client::StmtExecute;
-
- auto read_res = ClassicFrame::recv_frame_sequence(src_channel, src_protocol);
- if (!read_res) return stdx::make_unexpected(read_res.error());
-
- const auto &recv_buf = src_channel->recv_plain_view();
-
- auto frame_decode_res = classic_protocol::decode<
- classic_protocol::frame::Frame<classic_protocol::borrowed::wire::String>>(
- net::buffer(recv_buf), caps);
- if (!frame_decode_res) {
- return stdx::make_unexpected(frame_decode_res.error());
- }
-
- src_protocol->seq_id(frame_decode_res->second.seq_id());
-
- auto decode_res = classic_protocol::decode<msg_type>(
- net::buffer(frame_decode_res->second.payload().value()), caps,
- [src_protocol](auto stmt_id)
- -> stdx::expected<std::vector<msg_type::ParamDef>, std::error_code> {
- const auto it = src_protocol->prepared_statements().find(stmt_id);
- if (it == src_protocol->prepared_statements().end()) {
- return stdx::make_unexpected(make_error_code(
- classic_protocol::codec_errc::statement_id_not_found));
- }
-
- std::vector<msg_type::ParamDef> params;
- params.reserve(it->second.parameters.size());
-
- for (const auto &param : it->second.parameters) {
- params.emplace_back(param.type_and_flags);
- }
-
- return params;
- });
- if (!decode_res) return stdx::make_unexpected(decode_res.error());
-
- return decode_res->second;
-}
-
#endif
diff --git a/router/src/routing/src/classic_stmt_execute_forwarder.cc b/router/src/routing/src/classic_stmt_execute_forwarder.cc
index a66066a1e52..8833ea760df 100644
--- a/router/src/routing/src/classic_stmt_execute_forwarder.cc
+++ b/router/src/routing/src/classic_stmt_execute_forwarder.cc
@@ -26,7 +26,6 @@

#include "classic_connection_base.h"
#include "classic_frame.h"
-#include "hexify.h"
#include "mysql/harness/stdx/expected.h"
#include "mysql/harness/tls_error.h"
#include "mysqld_error.h" // mysql-server error-codes
@@ -63,51 +62,7 @@ StmtExecuteForwarder::process() {
stdx::expected<Processor::Result, std::error_code>
StmtExecuteForwarder::command() {
if (auto &tr = tracer()) {
- auto *socket_splicer = connection()->socket_splicer();
- auto *src_channel = socket_splicer->client_channel();
- auto *src_protocol = connection()->client_protocol();
-
- auto msg_res = ClassicFrame::recv_msg<
- classic_protocol::borrowed::message::client::StmtExecute>(src_channel,
- src_protocol);
- if (!msg_res) {
- auto ec = msg_res.error();
-
- // parse errors are invalid input.
- if (ec.category() ==
- make_error_code(classic_protocol::codec_errc::invalid_input)
- .category()) {
- auto send_res =
- ClassicFrame::send_msg<classic_protocol::message::server::Error>(
- src_channel, src_protocol,
- {ER_MALFORMED_PACKET, "Malformed packet", "HY000"});
- if (!send_res) return send_client_failed(send_res.error());
-
- const auto &recv_buf = src_channel->recv_plain_view();
-
- tr.trace(Tracer::Event().stage("stmt_execute::command:\n" +
- mysql_harness::hexify(recv_buf)));
-
- discard_current_msg(src_channel, src_protocol);
-
- stage(Stage::Done);
- return Result::SendToClient;
- }
-
- return recv_client_failed(msg_res.error());
- }
-
- const auto &recv_buf = src_channel->recv_plain_view();
-
- tr.trace(Tracer::Event().stage(
- "stmt_execute::command:\nstmt-id: " + //
- std::to_string(msg_res->statement_id()) + "\n" + //
- "flags: " + msg_res->flags().to_string() + "\n" + //
- "new-params-bound: " + std::to_string(msg_res->new_params_bound()) +
- "\n" + //
- "types::size(): " + std::to_string(msg_res->types().size()) + "\n" +
- "values::size(): " + std::to_string(msg_res->values().size()) + "\n" +
- mysql_harness::hexify(recv_buf)));
+ tr.trace(Tracer::Event().stage("stmt_execute::command"));
}
Comment on lines +165 to +223
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In classic_stmt_execute_forwarder.cc, the detailed logging and error handling for StmtExecute command processing have been removed. This simplification could affect the ability to diagnose issues during statement execution forwarding. Consider whether additional logging or error handling might be necessary to maintain visibility and robustness in the forwarding process.

+IF(ENABLE_DETAILED_LOGGING)
 tr.trace(Tracer::Event().stage("stmt_execute::command"));
+ENDIF()

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
@@ -26,7 +26,6 @@
#include "classic_connection_base.h"
#include "classic_frame.h"
-#include "hexify.h"
#include "mysql/harness/stdx/expected.h"
#include "mysql/harness/tls_error.h"
#include "mysqld_error.h" // mysql-server error-codes
@@ -63,51 +62,7 @@ StmtExecuteForwarder::process() {
stdx::expected<Processor::Result, std::error_code>
StmtExecuteForwarder::command() {
if (auto &tr = tracer()) {
- auto *socket_splicer = connection()->socket_splicer();
- auto *src_channel = socket_splicer->client_channel();
- auto *src_protocol = connection()->client_protocol();
-
- auto msg_res = ClassicFrame::recv_msg<
- classic_protocol::borrowed::message::client::StmtExecute>(src_channel,
- src_protocol);
- if (!msg_res) {
- auto ec = msg_res.error();
-
- // parse errors are invalid input.
- if (ec.category() ==
- make_error_code(classic_protocol::codec_errc::invalid_input)
- .category()) {
- auto send_res =
- ClassicFrame::send_msg<classic_protocol::message::server::Error>(
- src_channel, src_protocol,
- {ER_MALFORMED_PACKET, "Malformed packet", "HY000"});
- if (!send_res) return send_client_failed(send_res.error());
-
- const auto &recv_buf = src_channel->recv_plain_view();
-
- tr.trace(Tracer::Event().stage("stmt_execute::command:\n" +
- mysql_harness::hexify(recv_buf)));
-
- discard_current_msg(src_channel, src_protocol);
-
- stage(Stage::Done);
- return Result::SendToClient;
- }
-
- return recv_client_failed(msg_res.error());
- }
-
- const auto &recv_buf = src_channel->recv_plain_view();
-
- tr.trace(Tracer::Event().stage(
- "stmt_execute::command:\nstmt-id: " + //
- std::to_string(msg_res->statement_id()) + "\n" + //
- "flags: " + msg_res->flags().to_string() + "\n" + //
- "new-params-bound: " + std::to_string(msg_res->new_params_bound()) +
- "\n" + //
- "types::size(): " + std::to_string(msg_res->types().size()) + "\n" +
- "values::size(): " + std::to_string(msg_res->values().size()) + "\n" +
- mysql_harness::hexify(recv_buf)));
+ tr.trace(Tracer::Event().stage("stmt_execute::command"));
}
@@ -26,7 +26,6 @@
#include "classic_connection_base.h"
#include "classic_frame.h"
-#include "hexify.h"
#include "mysql/harness/stdx/expected.h"
#include "mysql/harness/tls_error.h"
#include "mysqld_error.h" // mysql-server error-codes
@@ -63,51 +62,7 @@ StmtExecuteForwarder::process() {
stdx::expected<Processor::Result, std::error_code>
StmtExecuteForwarder::command() {
if (auto &tr = tracer()) {
+IF(ENABLE_DETAILED_LOGGING)
tr.trace(Tracer::Event().stage("stmt_execute::command"));
+ENDIF()
}


auto &server_conn = connection()->socket_splicer()->server_conn();
diff --git a/router/tests/integration/test_routing_direct.cc b/router/tests/integration/test_routing_direct.cc
index 25a1bfa5a64..b21bec4fa39 100644
--- a/router/tests/integration/test_routing_direct.cc
+++ b/router/tests/integration/test_routing_direct.cc
@@ -2181,17 +2181,6 @@ TEST_P(ConnectionTest, classic_protocol_prepare_execute) {
};
ASSERT_NO_ERROR(stmt.bind_params(params));

- // execute again to trigger a StmtExecute with new-params-bound = 1.
- {
- auto exec_res = stmt.execute();
- ASSERT_NO_ERROR(exec_res);
-
- for ([[maybe_unused]] auto res : *exec_res) {
- // drain the resultsets.
- }
- }
-
- // execute again to trigger a StmtExecute with new-params-bound = 0.
{
auto exec_res = stmt.execute();
ASSERT_NO_ERROR(exec_res);
@@ -2205,7 +2194,7 @@ TEST_P(ConnectionTest, classic_protocol_prepare_execute) {
auto events_res = changed_event_counters(cli);
ASSERT_NO_ERROR(events_res);

- EXPECT_THAT(*events_res, ElementsAre(Pair("statement/com/Execute", 2),
+ EXPECT_THAT(*events_res, ElementsAre(Pair("statement/com/Execute", 1),
Pair("statement/com/Prepare", 1)));
}

@@ -2217,7 +2206,7 @@ TEST_P(ConnectionTest, classic_protocol_prepare_execute) {
ASSERT_NO_ERROR(events_res);

EXPECT_THAT(*events_res,
- ElementsAre(Pair("statement/com/Execute", 2),
+ ElementsAre(Pair("statement/com/Execute", 1),
Pair("statement/com/Prepare", 1),
// explicit
Pair("statement/com/Reset Connection", 1),
2 changes: 1 addition & 1 deletion versions/mysql.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"8.3.0",
"8.2.0",
"8.1.0",
"8.0.35",
"8.0.36",
"5.7.44",
"5.6.51"
]
Loading