From a13e521bc51d3ad9bb6e8e6481a0c8dea3b648a7 Mon Sep 17 00:00:00 2001 From: Daniel Black Date: Mon, 18 Mar 2024 17:37:13 +1100 Subject: [PATCH 01/10] MDEV-33636: RPM caps is on mariadbd exe Postfix on 51e3f1daf54309d14fe8db438024d88aa110e86a that mariadbd should be the executable name rather than capabilities on a symlink. --- cmake/cpack_rpm.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmake/cpack_rpm.cmake b/cmake/cpack_rpm.cmake index be94a3387f1..9383ae8c13b 100644 --- a/cmake/cpack_rpm.cmake +++ b/cmake/cpack_rpm.cmake @@ -183,7 +183,7 @@ ENDMACRO(SETA) IF (CMAKE_VERSION VERSION_GREATER 3.10.0) # cmake bug #14362 SET(CPACK_RPM_server_USER_FILELIST ${CPACK_RPM_server_USER_FILELIST} - "%caps(cap_ipc_lock=pe) %{_sbindir}/mysqld" + "%caps(cap_ipc_lock=pe) %{_sbindir}/mariadbd" ) ENDIF() From 7d36919f4b73a2507555ed101ccd02cb266dcc52 Mon Sep 17 00:00:00 2001 From: Vladislav Vaintroub Date: Thu, 22 Feb 2024 20:57:00 +0100 Subject: [PATCH 02/10] MDEV-33723 Mroonga ignored WITHOUT_DYNAMIC_PLUGINS Make WITHOUT_DYNAMIC_PLUGINS ignore mrooonga also in its own DIY version of MYSQL_ADD_PLUGIN --- storage/mroonga/CMakeLists.txt | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/storage/mroonga/CMakeLists.txt b/storage/mroonga/CMakeLists.txt index c048b214658..fffcd80c971 100644 --- a/storage/mroonga/CMakeLists.txt +++ b/storage/mroonga/CMakeLists.txt @@ -57,6 +57,11 @@ if(MRN_BUNDLED) "${PLUGIN_MROONGA}" STREQUAL "NO") return() endif() + if(WITHOUT_DYNAMIC_PLUGINS) + if(NOT (PLUGIN_MROONGA STREQUAL STATIC)) + return() + endif() + endif() endif() set(MRN_BUNDLED_GROONGA_RELATIVE_DIR "vendor/groonga") From f0590db5c5760c8d0de41c10fb76e3c7f44358f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Fri, 22 Mar 2024 14:57:00 +0200 Subject: [PATCH 03/10] MDEV-33591 MONITOR_INC_VALUE_CUMULATIVE is executed regardless of "if" condition MONITOR_INC_VALUE_CUMULATIVE is a multiline macro, so the second statement will be executed always, regardless of "if" condition. These problems first started with commit b1ab211dee599eabd9a5b886fafa3adea29ae041 (MDEV-15053). Thanks to Yury Chaikou from ServiceNow for the report. --- storage/innobase/buf/buf0flu.cc | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/storage/innobase/buf/buf0flu.cc b/storage/innobase/buf/buf0flu.cc index e181d311a7c..d74ccd75521 100644 --- a/storage/innobase/buf/buf0flu.cc +++ b/storage/innobase/buf/buf0flu.cc @@ -1389,10 +1389,12 @@ reacquire_mutex: mysql_mutex_assert_owner(&buf_pool.mutex); if (scanned) + { MONITOR_INC_VALUE_CUMULATIVE(MONITOR_LRU_BATCH_SCANNED, MONITOR_LRU_BATCH_SCANNED_NUM_CALL, MONITOR_LRU_BATCH_SCANNED_PER_CALL, scanned); + } } /** Flush and move pages from LRU or unzip_LRU list to the free list. @@ -1543,15 +1545,19 @@ static ulint buf_do_flush_list_batch(ulint max_n, lsn_t lsn) space->release(); if (scanned) + { MONITOR_INC_VALUE_CUMULATIVE(MONITOR_FLUSH_BATCH_SCANNED, MONITOR_FLUSH_BATCH_SCANNED_NUM_CALL, MONITOR_FLUSH_BATCH_SCANNED_PER_CALL, scanned); + } if (count) + { MONITOR_INC_VALUE_CUMULATIVE(MONITOR_FLUSH_BATCH_TOTAL_PAGE, MONITOR_FLUSH_BATCH_COUNT, MONITOR_FLUSH_BATCH_PAGES, count); + } mysql_mutex_assert_owner(&buf_pool.mutex); return count; } From 70b907724e1c5185be8c5722c417dcbc051520f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Fri, 22 Mar 2024 15:07:31 +0200 Subject: [PATCH 04/10] MDEV-32364 fixup: crash in ut_dontdump() --- storage/innobase/include/ut0new.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/storage/innobase/include/ut0new.h b/storage/innobase/include/ut0new.h index 01bf34dc856..f359610bfbd 100644 --- a/storage/innobase/include/ut0new.h +++ b/storage/innobase/include/ut0new.h @@ -1078,9 +1078,8 @@ static inline void *ut_malloc_dontdump(size_t n_bytes, ...) { void *ptr = my_large_malloc(&n_bytes, MYF(0)); - ut_dontdump(ptr, n_bytes, true); - if (ptr) { + ut_dontdump(ptr, n_bytes, true); os_total_large_mem_allocated += n_bytes; } return ptr; From e9d334434df20bf2da2c94c78dd0dbe7939e5ee6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Lindstr=C3=B6m?= Date: Thu, 15 Feb 2024 09:40:50 +0200 Subject: [PATCH 05/10] MDEV-32787 : Assertion `!wsrep_has_changes(thd) || (thd->lex->sql_command == SQLCOM_CREATE_TABLE && !thd->is_current_stmt_binlog_format_row()) || thd->wsrep_cs().transaction().state() == wsrep::transaction::s_aborted' failed in void wsrep_commit_empty(THD*, bool) When we commit empty transaction we should allow wsrep transaction to be on s_must_replay state for DDL that was killed during certification. Fix is tested with RQG because deterministic mtr-testcase was not found. Signed-off-by: Julius Goryavsky --- sql/wsrep_mysqld.cc | 58 +++++++++++++++++++++++++++++---------------- 1 file changed, 37 insertions(+), 21 deletions(-) diff --git a/sql/wsrep_mysqld.cc b/sql/wsrep_mysqld.cc index 6d7f555d479..d7066e65332 100644 --- a/sql/wsrep_mysqld.cc +++ b/sql/wsrep_mysqld.cc @@ -3468,21 +3468,11 @@ void wsrep_ready_set(bool ready_value) step is performed to leave the wsrep transaction in the state as it never existed. - This should not be an inline functions as it requires a lot of stack space - because of WSREP_DBUG() usage. It's also not a function that is - frequently called. */ void wsrep_commit_empty(THD* thd, bool all) { DBUG_ENTER("wsrep_commit_empty"); - WSREP_DEBUG("wsrep_commit_empty for %llu client_state %s client_mode" - " %s trans_state %s sql %s", - thd_get_thread_id(thd), - wsrep::to_c_string(thd->wsrep_cs().state()), - wsrep::to_c_string(thd->wsrep_cs().mode()), - wsrep::to_c_string(thd->wsrep_cs().transaction().state()), - wsrep_thd_query(thd)); if (wsrep_is_real(thd, all) && wsrep_thd_is_local(thd) && @@ -3490,14 +3480,40 @@ void wsrep_commit_empty(THD* thd, bool all) !thd->internal_transaction() && thd->wsrep_trx().state() != wsrep::transaction::s_committed) { - /* Here transaction is either empty (i.e. no changes) or - it was CREATE TABLE with no row binlog format or - we have already aborted transaction e.g. because max writeset size - has been reached. */ - DBUG_ASSERT(!wsrep_has_changes(thd) || - (thd->lex->sql_command == SQLCOM_CREATE_TABLE && - !thd->is_current_stmt_binlog_format_row()) || - thd->wsrep_cs().transaction().state() == wsrep::transaction::s_aborted); +#ifndef DBUG_OFF + const bool empty= !wsrep_has_changes(thd); + const bool create= thd->lex->sql_command == SQLCOM_CREATE_TABLE && + !thd->is_current_stmt_binlog_format_row(); + const bool aborted= thd->wsrep_cs().transaction().state() == wsrep::transaction::s_aborted; + const bool ddl_replay= ((sql_command_flags[thd->lex->sql_command] & + (CF_SCHEMA_CHANGE | CF_ADMIN_COMMAND)) && + thd->wsrep_cs().transaction().state() == wsrep::transaction::s_must_replay); + /* Here transaction is either + (1) empty (i.e. no changes) or + (2) it was CREATE TABLE with no row binlog format or + (3) we have already aborted transaction e.g. because max writeset size + has been reached or + (4) it was DDL and got BF aborted and must replay. + */ + if(!(empty || create || aborted || ddl_replay)) + { + WSREP_DEBUG("wsrep_commit_empty: thread: %llu client_state: %s client_mode:" + " %s trans_state: %s error: %s empty: %d create: %d aborted:" + " %d ddl_replay: %d sql: %s", + thd_get_thread_id(thd), + wsrep::to_c_string(thd->wsrep_cs().state()), + wsrep::to_c_string(thd->wsrep_cs().mode()), + wsrep::to_c_string(thd->wsrep_cs().transaction().state()), + wsrep::to_c_string(thd->wsrep_cs().current_error()), + empty, create, aborted, ddl_replay, + wsrep_thd_query(thd)); + + DBUG_ASSERT(empty || // 1 + create || // 2 + aborted || // 3 + ddl_replay); // 4 + } +#endif /* DBUG_OFF */ bool have_error= wsrep_current_error(thd); int ret= wsrep_before_rollback(thd, all) || wsrep_after_rollback(thd, all) || @@ -3511,10 +3527,10 @@ void wsrep_commit_empty(THD* thd, bool all) DBUG_ASSERT(wsrep_current_error(thd) == wsrep::e_deadlock_error); thd->wsrep_cs().reset_error(); } + if (ret) - { - WSREP_DEBUG("wsrep_commit_empty failed: %d", wsrep_current_error(thd)); - } + WSREP_DEBUG("wsrep_commit_empty failed: %s", + wsrep::to_c_string(thd->wsrep_cs().current_error())); } DBUG_VOID_RETURN; } From 987a266d77a894c2816c10176d102b740bbc18f9 Mon Sep 17 00:00:00 2001 From: Julius Goryavsky Date: Mon, 25 Mar 2024 12:13:57 +0100 Subject: [PATCH 06/10] galera: wsrep-lib submodule update --- wsrep-lib | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wsrep-lib b/wsrep-lib index a5d95f0175f..7d108eb8706 160000 --- a/wsrep-lib +++ b/wsrep-lib @@ -1 +1 @@ -Subproject commit a5d95f0175f10b6127ea039c542725f6c4aa5cb9 +Subproject commit 7d108eb8706962abc74705bedfc60cfc3f296ea6 From ee2ed1a036891239c102eab108e0b9d8d641ede0 Mon Sep 17 00:00:00 2001 From: Daniel Black Date: Wed, 20 Mar 2024 16:30:11 +1100 Subject: [PATCH 07/10] Revert "MDEV-33636: RPM caps is on mariadbd exe" This was the orginal implementation that reverted with a bunch of commits. This reverts commit a13e521bc51d3ad9bb6e8e6481a0c8dea3b648a7. Revert "cmake: append to the array correctly" This reverts commit 51e3f1daf54309d14fe8db438024d88aa110e86a. Revert "build failure with cmake < 3.10" This reverts commit 49cf702ee54040cefcab67f6758c233a6370f5d0. Revert "MDEV-33301 memlock with systemd still not working" This reverts commit 8a1904d7825f9897cd237fc6a1d8a57a9f2108de. --- cmake/cpack_rpm.cmake | 8 ------ debian/mariadb-server-core-10.5.postinst | 26 ------------------- support-files/policy/apparmor/usr.sbin.mysqld | 1 - .../policy/selinux/mariadb-server.te | 4 +-- 4 files changed, 1 insertion(+), 38 deletions(-) delete mode 100644 debian/mariadb-server-core-10.5.postinst diff --git a/cmake/cpack_rpm.cmake b/cmake/cpack_rpm.cmake index 9383ae8c13b..65a739dc03e 100644 --- a/cmake/cpack_rpm.cmake +++ b/cmake/cpack_rpm.cmake @@ -164,7 +164,6 @@ SET(CPACK_RPM_server_USER_FILELIST "%config(noreplace) ${INSTALL_SYSCONF2DIR}/*" "%config(noreplace) ${INSTALL_SYSCONFDIR}/logrotate.d/mysql" ) - SET(CPACK_RPM_common_USER_FILELIST ${ignored} "%config(noreplace) ${INSTALL_SYSCONFDIR}/my.cnf") SET(CPACK_RPM_shared_USER_FILELIST ${ignored} "%config(noreplace) ${INSTALL_SYSCONF2DIR}/*") SET(CPACK_RPM_client_USER_FILELIST ${ignored} "%config(noreplace) ${INSTALL_SYSCONF2DIR}/*") @@ -180,13 +179,6 @@ MACRO(SETA var) ENDFOREACH() ENDMACRO(SETA) -IF (CMAKE_VERSION VERSION_GREATER 3.10.0) - # cmake bug #14362 - SET(CPACK_RPM_server_USER_FILELIST ${CPACK_RPM_server_USER_FILELIST} - "%caps(cap_ipc_lock=pe) %{_sbindir}/mariadbd" - ) -ENDIF() - SETA(CPACK_RPM_client_PACKAGE_OBSOLETES "mysql-client" "MySQL-client" diff --git a/debian/mariadb-server-core-10.5.postinst b/debian/mariadb-server-core-10.5.postinst deleted file mode 100644 index 5f79bed2402..00000000000 --- a/debian/mariadb-server-core-10.5.postinst +++ /dev/null @@ -1,26 +0,0 @@ -#!/bin/sh - -set -e - -# inspired by iputils-ping -# -# cap_ipc_lock is required if a user wants to use --memlock -# and has insufficient RLIMIT_MEMLOCK (MDEV-33301) - -PROGRAM=$(dpkg-divert --truename /usr/sbin/mysqld) - -if [ "$1" = configure ]; then - # If we have setcap installed, try setting - # which allows us to install our binaries without the setuid - # bit. - if command -v setcap > /dev/null; then - if ! setcap cap_ipc_lock+ep "$PROGRAM"; then - echo "Setcap failed on $PROGRAM, required with --memlock if insufficent RLIMIT_MEMLOCK" >&2 - fi - fi -fi - - -#DEBHELPER# - -exit 0 diff --git a/support-files/policy/apparmor/usr.sbin.mysqld b/support-files/policy/apparmor/usr.sbin.mysqld index 732f4b3a97a..c60ecd28531 100644 --- a/support-files/policy/apparmor/usr.sbin.mysqld +++ b/support-files/policy/apparmor/usr.sbin.mysqld @@ -14,7 +14,6 @@ capability chown, capability dac_override, - capability ipc_lock, capability setgid, capability setuid, capability sys_rawio, diff --git a/support-files/policy/selinux/mariadb-server.te b/support-files/policy/selinux/mariadb-server.te index ba53c97d4a8..89846063506 100644 --- a/support-files/policy/selinux/mariadb-server.te +++ b/support-files/policy/selinux/mariadb-server.te @@ -25,7 +25,7 @@ require { class lnk_file read; class process { getattr signull }; class unix_stream_socket connectto; - class capability { ipc_lock sys_resource sys_nice }; + class capability { sys_resource sys_nice }; class tcp_socket { name_bind name_connect }; class file { execute setattr read create getattr execute_no_trans write ioctl open append unlink }; class sock_file { create unlink getattr }; @@ -87,8 +87,6 @@ allow mysqld_t bin_t:file { getattr read execute open execute_no_trans ioctl }; # MariaDB additions allow mysqld_t self:process setpgid; -allow mysqld_t self:capability { ipc_lock }; - # This rule allows port tcp/4444 allow mysqld_t kerberos_port_t:tcp_socket { name_bind name_connect }; # This rule allows port tcp/4567 (tram_port_t may not be available on From 76a27155b4cd8174e900577dd01df2db1327b120 Mon Sep 17 00:00:00 2001 From: Daniel Black Date: Wed, 20 Mar 2024 18:25:21 +1100 Subject: [PATCH 08/10] MDEV-33301 memlock with systemd still not working .. even with MDEV-9095 fix CapabilityBounding sets require filesystem setcap attributes for the executable to gain privileges during execution. A side effect of this however is the getauxvec(AT_SECURE) gets set, and the secure_getenv from OpenSSL internals on OPENSSL_CONF environment variable will get ignored (openssl gh issue 21770). According to capabilities(7), Ambient capabilities don't trigger ld.so triggering the secure execution mode. Include SELinux and Apparmor capabilities for ipc_lock --- support-files/mariadb.service.in | 2 +- support-files/mariadb@.service.in | 2 +- support-files/policy/apparmor/usr.sbin.mysqld | 1 + support-files/policy/selinux/mariadb-server.te | 4 +++- 4 files changed, 6 insertions(+), 3 deletions(-) diff --git a/support-files/mariadb.service.in b/support-files/mariadb.service.in index ee5b5ddc427..dce845a9f72 100644 --- a/support-files/mariadb.service.in +++ b/support-files/mariadb.service.in @@ -51,7 +51,7 @@ Group=mysql # CAP_DAC_OVERRIDE To allow auth_pam_tool (which is SUID root) to read /etc/shadow when it's chmod 0 # does nothing for non-root, not needed if /etc/shadow is u+r # CAP_AUDIT_WRITE auth_pam_tool needs it on Debian for whatever reason -CapabilityBoundingSet=CAP_IPC_LOCK CAP_DAC_OVERRIDE CAP_AUDIT_WRITE +AmbientCapabilities=CAP_IPC_LOCK CAP_DAC_OVERRIDE CAP_AUDIT_WRITE # PrivateDevices=true implies NoNewPrivileges=true and # SUID auth_pam_tool suddenly doesn't do setuid anymore diff --git a/support-files/mariadb@.service.in b/support-files/mariadb@.service.in index a2a73b41e3e..b53711ef7e0 100644 --- a/support-files/mariadb@.service.in +++ b/support-files/mariadb@.service.in @@ -181,7 +181,7 @@ PrivateNetwork=false # CAP_DAC_OVERRIDE To allow auth_pam_tool (which is SUID root) to read /etc/shadow when it's chmod 0 # does nothing for non-root, not needed if /etc/shadow is u+r # CAP_AUDIT_WRITE auth_pam_tool needs it on Debian for whatever reason -CapabilityBoundingSet=CAP_IPC_LOCK CAP_DAC_OVERRIDE CAP_AUDIT_WRITE +AmbientCapabilities=CAP_IPC_LOCK CAP_DAC_OVERRIDE CAP_AUDIT_WRITE # PrivateDevices=true implies NoNewPrivileges=true and # SUID auth_pam_tool suddenly doesn't do setuid anymore diff --git a/support-files/policy/apparmor/usr.sbin.mysqld b/support-files/policy/apparmor/usr.sbin.mysqld index c60ecd28531..732f4b3a97a 100644 --- a/support-files/policy/apparmor/usr.sbin.mysqld +++ b/support-files/policy/apparmor/usr.sbin.mysqld @@ -14,6 +14,7 @@ capability chown, capability dac_override, + capability ipc_lock, capability setgid, capability setuid, capability sys_rawio, diff --git a/support-files/policy/selinux/mariadb-server.te b/support-files/policy/selinux/mariadb-server.te index 89846063506..ba53c97d4a8 100644 --- a/support-files/policy/selinux/mariadb-server.te +++ b/support-files/policy/selinux/mariadb-server.te @@ -25,7 +25,7 @@ require { class lnk_file read; class process { getattr signull }; class unix_stream_socket connectto; - class capability { sys_resource sys_nice }; + class capability { ipc_lock sys_resource sys_nice }; class tcp_socket { name_bind name_connect }; class file { execute setattr read create getattr execute_no_trans write ioctl open append unlink }; class sock_file { create unlink getattr }; @@ -87,6 +87,8 @@ allow mysqld_t bin_t:file { getattr read execute open execute_no_trans ioctl }; # MariaDB additions allow mysqld_t self:process setpgid; +allow mysqld_t self:capability { ipc_lock }; + # This rule allows port tcp/4444 allow mysqld_t kerberos_port_t:tcp_socket { name_bind name_connect }; # This rule allows port tcp/4567 (tram_port_t may not be available on From 58df20974b56c676333fdc36c39a38ac130c9eb4 Mon Sep 17 00:00:00 2001 From: Dave Gosselin Date: Wed, 13 Mar 2024 16:56:37 -0400 Subject: [PATCH 09/10] MDEV-33460 select '123' 'x'; unexpected result Queries that select concatenated constant strings now have colname and value that match. For example, SELECT '123' 'x'; will return a result where the column name and value both are '123x'. Review: Daniel Black --- mysql-test/main/empty_string_literal.result | 25 ++++++++++++++++--- mysql-test/main/empty_string_literal.test | 12 +++++++++ mysql-test/main/func_str.result | 2 +- mysql-test/main/join.result | 2 +- .../oracle/r/empty_string_literal.result | 6 ++--- sql/item.cc | 1 + 6 files changed, 40 insertions(+), 8 deletions(-) diff --git a/mysql-test/main/empty_string_literal.result b/mysql-test/main/empty_string_literal.result index 49153d68fa5..569b9d5e9ca 100644 --- a/mysql-test/main/empty_string_literal.result +++ b/mysql-test/main/empty_string_literal.result @@ -64,7 +64,7 @@ SET sql_mode=@mode; # Test litteral concat # SELECT 'a' 'b'; -a +ab ab SELECT 'a' ''; a @@ -76,13 +76,13 @@ SELECT '' ''; NULL NULL SELECT '' 'b' 'c'; -b +bc bc SELECT '' '' 'c'; c c SELECT 'a' '' 'c'; -a +ac ac SELECT 'a' '' ''; a @@ -208,3 +208,22 @@ t1 CREATE TABLE `t1` ( KEY `a` (`a`,`b`) ) ENGINE=MyISAM DEFAULT CHARSET=latin1 COLLATE=latin1_swedish_ci drop table t1; +# +# MDEV-33460 select '123' 'x'; unexpected result +# +SELECT ''; +NULL +NULL +SELECT '' 'b' 'c'; +bc +bc +SELECT '' '' 'c'; +c +c +SELECT 'a' '' 'c'; +ac +ac +SELECT 'a' '' ''; +a +a +# End of 10.5 test diff --git a/mysql-test/main/empty_string_literal.test b/mysql-test/main/empty_string_literal.test index 9174a7714a2..3320841fb42 100644 --- a/mysql-test/main/empty_string_literal.test +++ b/mysql-test/main/empty_string_literal.test @@ -25,3 +25,15 @@ flush tables; update t1 set a = 2; show create table t1; drop table t1; + +--echo # +--echo # MDEV-33460 select '123' 'x'; unexpected result +--echo # + +SELECT ''; +SELECT '' 'b' 'c'; +SELECT '' '' 'c'; +SELECT 'a' '' 'c'; +SELECT 'a' '' ''; + +--echo # End of 10.5 test diff --git a/mysql-test/main/func_str.result b/mysql-test/main/func_str.result index a47c149fb0d..c071b92318a 100644 --- a/mysql-test/main/func_str.result +++ b/mysql-test/main/func_str.result @@ -8,7 +8,7 @@ select 'hello',"'hello'",'""hello""','''h''e''l''l''o''',"hel""lo",'hel\'lo'; hello 'hello' ""hello"" 'h'e'l'l'o' hel"lo hel'lo hello 'hello' ""hello"" 'h'e'l'l'o' hel"lo hel'lo select 'hello' 'monty'; -hello +hellomonty hellomonty select length('\n\t\r\b\0\_\%\\'); length('\n\t\r\b\0\_\%\\') diff --git a/mysql-test/main/join.result b/mysql-test/main/join.result index 07579a065f9..ecc64b5f343 100644 --- a/mysql-test/main/join.result +++ b/mysql-test/main/join.result @@ -894,7 +894,7 @@ show status like '%cost%'; Variable_name Value Last_query_cost 4.016090 select 'The cost of accessing t1 (dont care if it changes' '^'; -The cost of accessing t1 (dont care if it changes +The cost of accessing t1 (dont care if it changes^ The cost of accessing t1 (dont care if it changes^ select 'vv: Following query must use ALL(t1), eq_ref(A), eq_ref(B): vv' Z; Z diff --git a/mysql-test/suite/compat/oracle/r/empty_string_literal.result b/mysql-test/suite/compat/oracle/r/empty_string_literal.result index 4fac736367b..a84cc81b38c 100644 --- a/mysql-test/suite/compat/oracle/r/empty_string_literal.result +++ b/mysql-test/suite/compat/oracle/r/empty_string_literal.result @@ -64,7 +64,7 @@ SET sql_mode=@mode; # Test litteral concat # SELECT 'a' 'b'; -a +ab ab SELECT 'a' ''; a @@ -76,13 +76,13 @@ SELECT '' ''; NULL NULL SELECT '' 'b' 'c'; -b +bc bc SELECT '' '' 'c'; c c SELECT 'a' '' 'c'; -a +ac ac SELECT 'a' '' ''; a diff --git a/sql/item.cc b/sql/item.cc index 0e5ed30cb91..99df4e65ac1 100644 --- a/sql/item.cc +++ b/sql/item.cc @@ -6943,6 +6943,7 @@ Item_basic_constant * Item_string::make_string_literal_concat(THD *thd, const LEX_CSTRING *str) { append(str->str, (uint32) str->length); + set_name(thd, &str_value); if (!(collation.repertoire & MY_REPERTOIRE_EXTENDED)) { // If the string has been pure ASCII so far, check the new part. From 0fc123c595cd0b61ec852800012b837858e2784f Mon Sep 17 00:00:00 2001 From: Alexander Barkov Date: Wed, 27 Mar 2024 15:22:58 +0400 Subject: [PATCH 10/10] MDEV-33772 Bad SEPARATOR value in GROUP_CONCAT on character set conversion Item_func_group_concat::print() did not take into account that Item_func_group_concat::separator can be of a different character set than the "String *str" (when the printing is being done to). Therefore, printing did not work correctly for: - non-ASCII separators when GROUP_CONCAT is done on 8bit data or multi-byte data with mbminlen==1. - all separators (even including simple ones like comma) when GROUP_CONCAT is done on ucs2/utf16/utf32 data (mbminlen>1). Because of this problem, VIEW definitions did not print correctly to their FRM files. This later led to a wrong SELECT and SHOW CREATE output. Fix: - Adding new String methods: bool append_for_single_quote_using_mb_wc(const char *str, size_t length, CHARSET_INFO *cs); bool append_for_single_quote_opt_convert(const char *str, size_t length, CHARSET_INFO *cs) which perform both escaping and character set conversion at the same time. - Adding a new String method escaped_wc_for_single_quote(), to reuse the code between the old and the new methods. - Fixing Item_func_group_concat::print() to use the new method append_for_single_quote_opt_convert(). --- mysql-test/main/ctype_ucs.result | 20 ++++++++++++ mysql-test/main/ctype_ucs.test | 17 +++++++++++ mysql-test/main/func_gconcat.result | 20 ++++++++++++ mysql-test/main/func_gconcat.test | 16 ++++++++++ sql/item_sum.cc | 2 +- sql/sql_string.cc | 47 ++++++++++++++++++++--------- sql/sql_string.h | 36 ++++++++++++++++++++++ 7 files changed, 143 insertions(+), 15 deletions(-) diff --git a/mysql-test/main/ctype_ucs.result b/mysql-test/main/ctype_ucs.result index eb1220f9a56..8f747dfd51e 100644 --- a/mysql-test/main/ctype_ucs.result +++ b/mysql-test/main/ctype_ucs.result @@ -6520,5 +6520,25 @@ SELECT 1 COLLATE latin1_swedish_ci; ERROR 42000: COLLATION 'latin1_swedish_ci' is not valid for CHARACTER SET 'ucs2' SET NAMES utf8; # +# MDEV-33772 Bad SEPARATOR value in GROUP_CONCAT on character set conversion +# +SET NAMES utf8mb3, @@collation_connection=ucs2_general_ci; +CREATE TABLE t1 (c VARCHAR(10)) CHARACTER SET ucs2; +INSERT INTO t1 VALUES ('a'),('A'); +CREATE OR REPLACE VIEW v1 AS +SELECT COUNT(*) AS cnt, GROUP_CONCAT(c) AS c1 FROM t1 GROUP BY c; +SELECT * FROM v1; +cnt c1 +2 a,A +SELECT HEX(c1) FROM v1; +HEX(c1) +0061002C0041 +SHOW CREATE VIEW v1; +View Create View character_set_client collation_connection +v1 CREATE ALGORITHM=UNDEFINED DEFINER=`root`@`localhost` SQL SECURITY DEFINER VIEW `v1` AS select count(0) AS `cnt`,group_concat(`t1`.`c` separator ',') AS `c1` from `t1` group by `t1`.`c` utf8 ucs2_general_ci +DROP VIEW v1; +DROP TABLE t1; +SET NAMES utf8mb3; +# # End of 10.5 tests # diff --git a/mysql-test/main/ctype_ucs.test b/mysql-test/main/ctype_ucs.test index 3df155c0f9d..f2fcee1dbb4 100644 --- a/mysql-test/main/ctype_ucs.test +++ b/mysql-test/main/ctype_ucs.test @@ -1189,6 +1189,23 @@ SELECT HEX(1 COLLATE ucs2_bin); SELECT 1 COLLATE latin1_swedish_ci; SET NAMES utf8; +--echo # +--echo # MDEV-33772 Bad SEPARATOR value in GROUP_CONCAT on character set conversion +--echo # + +SET NAMES utf8mb3, @@collation_connection=ucs2_general_ci; +CREATE TABLE t1 (c VARCHAR(10)) CHARACTER SET ucs2; +INSERT INTO t1 VALUES ('a'),('A'); +CREATE OR REPLACE VIEW v1 AS + SELECT COUNT(*) AS cnt, GROUP_CONCAT(c) AS c1 FROM t1 GROUP BY c; +SELECT * FROM v1; +SELECT HEX(c1) FROM v1; +SHOW CREATE VIEW v1; +DROP VIEW v1; +DROP TABLE t1; +SET NAMES utf8mb3; + + --echo # --echo # End of 10.5 tests --echo # diff --git a/mysql-test/main/func_gconcat.result b/mysql-test/main/func_gconcat.result index 7bc6441661b..e922134a9da 100644 --- a/mysql-test/main/func_gconcat.result +++ b/mysql-test/main/func_gconcat.result @@ -1517,4 +1517,24 @@ deallocate prepare stmt; set join_cache_level=default; set group_concat_max_len=default; drop table t1,t2; +# +# MDEV-33772 Bad SEPARATOR value in GROUP_CONCAT on character set conversion +# +SET NAMES utf8, @@collation_connection=latin1_swedish_ci; +CREATE TABLE t1 (c VARCHAR(10)) CHARACTER SET latin1; +INSERT INTO t1 VALUES ('a'),('A'); +CREATE OR REPLACE VIEW v1 AS +SELECT GROUP_CONCAT(c SEPARATOR 'ß') AS c1 FROM t1 GROUP BY c; +SELECT * FROM v1; +c1 +aßA +SELECT HEX(c1) FROM v1; +HEX(c1) +61DF41 +SHOW CREATE VIEW v1; +View Create View character_set_client collation_connection +v1 CREATE ALGORITHM=UNDEFINED DEFINER=`root`@`localhost` SQL SECURITY DEFINER VIEW `v1` AS select group_concat(`t1`.`c` separator 'ß') AS `c1` from `t1` group by `t1`.`c` utf8 latin1_swedish_ci +DROP VIEW v1; +DROP TABLE t1; +SET NAMES latin1; # End of 10.5 tests diff --git a/mysql-test/main/func_gconcat.test b/mysql-test/main/func_gconcat.test index 2e09bddbb8b..c9787ce4471 100644 --- a/mysql-test/main/func_gconcat.test +++ b/mysql-test/main/func_gconcat.test @@ -1105,4 +1105,20 @@ set group_concat_max_len=default; drop table t1,t2; +--echo # +--echo # MDEV-33772 Bad SEPARATOR value in GROUP_CONCAT on character set conversion +--echo # + +SET NAMES utf8, @@collation_connection=latin1_swedish_ci; +CREATE TABLE t1 (c VARCHAR(10)) CHARACTER SET latin1; +INSERT INTO t1 VALUES ('a'),('A'); +CREATE OR REPLACE VIEW v1 AS + SELECT GROUP_CONCAT(c SEPARATOR 'ß') AS c1 FROM t1 GROUP BY c; +SELECT * FROM v1; +SELECT HEX(c1) FROM v1; +SHOW CREATE VIEW v1; +DROP VIEW v1; +DROP TABLE t1; +SET NAMES latin1; + --echo # End of 10.5 tests diff --git a/sql/item_sum.cc b/sql/item_sum.cc index 7f39efe6c9a..4cf403c1618 100644 --- a/sql/item_sum.cc +++ b/sql/item_sum.cc @@ -4587,7 +4587,7 @@ void Item_func_group_concat::print(String *str, enum_query_type query_type) if (sum_func() == GROUP_CONCAT_FUNC) { str->append(STRING_WITH_LEN(" separator \'")); - str->append_for_single_quote(separator->ptr(), separator->length()); + str->append_for_single_quote_opt_convert(*separator); str->append(STRING_WITH_LEN("\'")); } diff --git a/sql/sql_string.cc b/sql/sql_string.cc index b723ac5db66..cf1ed210ca3 100644 --- a/sql/sql_string.cc +++ b/sql/sql_string.cc @@ -1126,26 +1126,45 @@ bool String::append_for_single_quote(const char *st, size_t len) int chlen; for (; st < end; st++) { - switch (*st) + char ch2= (char) (uchar) escaped_wc_for_single_quote((uchar) *st); + if (ch2) { - case '\\': APPEND(STRING_WITH_LEN("\\\\")); break; - case '\0': APPEND(STRING_WITH_LEN("\\0")); break; - case '\'': APPEND(STRING_WITH_LEN("\\'")); break; - case '\n': APPEND(STRING_WITH_LEN("\\n")); break; - case '\r': APPEND(STRING_WITH_LEN("\\r")); break; - case '\032': APPEND(STRING_WITH_LEN("\\Z")); break; - default: if ((chlen=charset()->charlen(st, end)) > 0) - { - APPEND(st, chlen); - st+= chlen-1; - } - else - APPEND(*st); + if (append('\\') || append(ch2)) + return true; + continue; } + if ((chlen= charset()->charlen(st, end)) > 0) + { + APPEND(st, chlen); + st+= chlen-1; + } + else + APPEND(*st); } return 0; } + +bool String::append_for_single_quote_using_mb_wc(const char *src, + size_t length, + CHARSET_INFO *cs) +{ + DBUG_ASSERT(&my_charset_bin != charset()); + DBUG_ASSERT(&my_charset_bin != cs); + const uchar *str= (const uchar *) src; + const uchar *end= (const uchar *) src + length; + int chlen; + my_wc_t wc; + for ( ; (chlen= cs->cset->mb_wc(cs, &wc, str, end)) > 0; str+= chlen) + { + my_wc_t wc2= escaped_wc_for_single_quote(wc); + if (wc2 ? (append_wc('\\') || append_wc(wc2)) : append_wc(wc)) + return true; + } + return false; +} + + void String::print(String *str) const { str->append_for_single_quote(Ptr, str_length); diff --git a/sql/sql_string.h b/sql/sql_string.h index 13820329750..3dbeb7b83cf 100644 --- a/sql/sql_string.h +++ b/sql/sql_string.h @@ -1134,6 +1134,42 @@ public: print_with_conversion(to, cs); } + static my_wc_t escaped_wc_for_single_quote(my_wc_t ch) + { + switch (ch) + { + case '\\': return '\\'; + case '\0': return '0'; + case '\'': return '\''; + case '\n': return 'n'; + case '\r': return 'r'; + case '\032': return 'Z'; + } + return 0; + } + + // Append for single quote using mb_wc/wc_mb Unicode conversion + bool append_for_single_quote_using_mb_wc(const char *str, size_t length, + CHARSET_INFO *cs); + + // Append for single quote with optional mb_wc/wc_mb conversion + bool append_for_single_quote_opt_convert(const char *str, + size_t length, + CHARSET_INFO *cs) + { + return charset() == &my_charset_bin || cs == &my_charset_bin || + my_charset_same(charset(), cs) ? + append_for_single_quote(str, length) : + append_for_single_quote_using_mb_wc(str, length, cs); + } + + bool append_for_single_quote_opt_convert(const String &str) + { + return append_for_single_quote_opt_convert(str.ptr(), + str.length(), + str.charset()); + } + bool append_for_single_quote(const char *st, size_t len); bool append_for_single_quote(const String *s) {