- 20 Jul, 2023 10 commits
-
-
Aleksey Midenkov authored
Constraints processing row_ins_check_foreign_constraint() was not called because row_upd_check_references_constraints() didn't see update as delete: node->is_delete was false. Since MDEV-30378 we check for TRG_EVENT_DELETE to detect versioned delete in ha_innobase::update_row(). Now we can use TRG_EVENT_DELETE to set upd_node->is_delete, so constraints processing is triggered correctly.
-
Aleksey Midenkov authored
1. Exclude merging history rows into fts index. The check !history_fts && (index->type & DICT_FTS) was just incorrect attempt to avoid history in fts index. 2. Don't check for duplicates for history rows.
-
Daniel Lenski authored
The `safe_strcpy()` function was added in https://github.com/mariadb/server/commit/567b68129943#diff-23f88d0b52735bf79b7eb76e2ddbbebc96f3b1ca16e784a347525a9c43134d77 Unfortunately, its current implementation triggers many GCC 8+ string truncation and array bounds warnings, particularly due to the potential for a false positive `-Warray-bounds`. For example, the line `safe_strcpy(delimiter, sizeof(delimiter), ";")` in `client/mysqldump.c` causes the following warning: [1669/1914] Building C object client/CMakeFiles/mariadb-dump.dir/mysqldump.c.o In file included from /PATH/include/my_sys.h:20, from /PATH/mysqldump.c:51: In function ?safe_strcpy?, inlined from ?dump_events_for_db.isra? at /PATH/client/mysqldump.c:2595:3: /PATH/include/m_string.h:258:39: warning: array subscript 1535 is outside array bounds of ?const char[2]? [-Warray-bounds=] 258 | if (dst[dst_size - 2] != '\0' && src[dst_size - 1] != '\0') | ~~~^~~~~~~~~~~~~~ GCC is reporting that the `safe_strcpy` function *could* cause an out-of-bounds read from the constant *source* string `";"`, however this warning is unhelpful and confusing because it can only happen if the size of the *destination* buffer is incorrectly specified, which is not the case here. In https://github.com/MariaDB/server/pull/2640, Andrew Hutchings proposed fixing this by disabling the `-Warray-bounds` check in this function (specifically in https://github.com/MariaDB/server/pull/2640/commits/be382d01d08739d081f6cf40f350f7414f29b49d#diff-23f88d0b52735bf79b7eb76e2ddbbebc96f3b1ca16e784a347525a9c43134d77R255-R262). However, this was rejected because it also disables the *helpful* `-Warray-bounds` check on the destination buffer. Cherry-picking the commit https://github.com/MariaDB/server/commit/a7adfd4c52307876d68ad3386cefd3757ee66e92 from 11.2 by Monty Widenius solves the first two problems: 1. It reimplements `safe_strcpy` a bit more efficiently, skipping the `memset(dst, 0, dst_size)`. This is unnecessary since `strncpy` already pads `dst` with 0 bytes. 2. It will not trigger the `-Warray-bounds` warning, because `src` is not read based on an offset determined from `dst_size`. There is a third problem, however. Using `strncpy` triggers the `-Wstringop-truncation` warning (https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wstringop-truncation), so we need to disable that. However, that is a much less broadly and generally-useful warning so there is no loss of static analysis value caused by disabling it. All new code of the whole pull request, including one or several files that are either new files or modified ones, are contributed under the BSD-new license. I am contributing on behalf of my employer Amazon Web Services, Inc.
-
Monty authored
Note: We should replace most case of safe_strcpy() with strmake() to avoid the not needed zerofill.
-
Oleksandr Byelkin authored
Do not get value of expensive constants.
-
Daniel Black authored
Add the end of file marker x1A to the DBF file and handle it correctly to preserve interoperability with Libreoffice, and others that have followed the DBF spec. The file open mode of "a+" was problematic because Linux and the OSX, the previous main development mode are inconsistent (see man fopen). The main problem per the bug report was the inability to fseek back to the beginning to update the records in the header. As such the "a+" mode is remove and "w+b" is used inserting to a new file and "r+b" is used for appending to the file. In DBFFAM::CloseTableFile move PlugCloseFile down to close the file in all modes. The year unlike the comments is always since 1900. Use the YYYY-MM-DD as an unabigious form during tracing. Thanks for Mr. Zoltan Duna for the descriptive bug report.
-
Oleksandr Byelkin authored
Fix of typo in checking variable list corectness. Fix of error handling in case of variable list parse error
-
Alexander Barkov authored
MDEV-28384 UBSAN: null pointer passed as argument 1, which is declared to never be null in my_strnncoll_binary on SELECT ... COUNT or GROUP_CONCAT Also fixes: MDEV-30982 UBSAN: runtime error: null pointer passed as argument 2, which is declared to never be null in my_strnncoll_binary on DELETE Calling memcmp() with a NULL pointer is undefined behaviour according to the C standard, even if the length argument is 0. Adding tests for length==0 before calling memcmp() into: - my_strnncoll_binary() - my_strnncoll_8bit_bin
-
Alexander Barkov authored
MDEV-22856 Assertion `!str || str != Ptr' and Assertion `!str || str != Ptr || !is_alloced()' failed in String::copy The problem was earlier fixed by MDEV-26953. Adding MTR tests only.
-
Daniel Black authored
Concatenation error leads to typo in the warning message
-
- 19 Jul, 2023 3 commits
-
-
Alexander Barkov authored
Problem: Item_func_conv::val_str() copied the ASCII string with the numeric base conversion result directly to the function result string. In case of a tricky character set (e.g. utf32) it produced an illformed string. Fix: Copy the base conversion result to the function result as is only if the function character set is ASCII compatible, go through a character set conversion otherwise.
-
Alexander Barkov authored
Change history in the affected code: - Since 10.4.8 (MDEV-20397 and MDEV-23311), functions ROUND(), CEILING(), FLOOR() return a TIME value for a TIME input. - Since 10.4.14 (MDEV-23525), MIN() and MAX() calculate a result for a TIME input using val_native() rather than val_str(). Problem: The patch for MDEV-23525 did not take into account combinations like MIN(ROUND(time)), MAX(FLOOR(time)), etc. MIN() and MAX() with ROUND(time), CEILING(time), FLOOR(time) as an argument call the method val_native() of the undelying classes Item_func_round and Item_func_int_val. However these classes implemented the method val_native() as DBUG_ASSERT(0). Fix: This patch adds a TIME-specific code inside: - Item_func_round::val_native() - Item_func_int_val::val_native() still with DBUG_ASSERT(0) for all other data types, as other data types do not call val_native() of these classes. We'll need a more generic solition eventualy, e.g. turn Item_func_round and Item_func_int_val into Item_handled_func. However, this change would be too risky for 10.4 at this point.
-
Daniel Black authored
The pointer was used deep in the call path. Resolve this by setting the pointer to NULL at the end of the function. Tested with gcc-13.3.1 (fc38) The warning disable 38fe266e can be reverted in 10.6+ on merge.
-
- 14 Jul, 2023 2 commits
-
-
Daniel Black authored
Add @ to the allowed characters in a username.
-
Oleksandr Byelkin authored
MDEV-28017 Illegal mix of collations (cp1251_general_ci,IMPLICIT), (latin1_swedish_ci,COERCIBLE), (latin1_swedish_ci,COERCIBLE) for operation 'between' Correct test fix is to disable service connection to create views in connection with correct charset settings.
-
- 13 Jul, 2023 1 commit
-
-
Yuchen Pei authored
The existing (incorrect) overriding mechanism is: Non-minus-one var value overrides table param overrides default value. Before MDEV-27169, unspecified var value is -1. So if the user sets both the var to be a value other than -1 and the table param, the var value will prevail, which is incorrect. After MDEV-27169, unspecified var value is default value. So if the user does not set the var but sets the table param, the default value will prevail, which is even more incorrect. This patch fixes it so that table param, if specified, always overrides var value, and the latter if not specified or set to -1, falls back to the default value We achieve this by replacing all such overriding in spd_param.cc with macros that override in the correct way, and removing all the "overriding -1" lines involving table params in spider_set_connect_info_default() except for those table params not defined as sysvar/thdvar in spd_params.cc We also introduced macros for non-overriding sysvar and thdvar, so that the code is cleaner and less error-prone In server versions where MDEV-27169 has not been applied, we also backport the patch, that is, replacing -1 default values with real default values In server versions where MDEV-28006 has not been applied, we do the same for udf params
-
- 12 Jul, 2023 8 commits
-
-
Andrei authored
MDEV-31503 ALTER SEQUENCE ends up in optimistic parallel slave binlog out-of-order The OOO error still was possible even after MDEV-31077. This time it occured through open_table() when the sequence table was not in the table cache *and* the table was created before the last server restart. In such context a internal (read-only) transaction is committed and it was not blocked from doing a wakeup() call to subsequent transactions. Fixed with extending suspend_subsequent_commits() effect for the entirety of Sql_cmd_alter_sequence::execute(). An elaborated MDEV-31077 test proves the fixes of both failure scenarios. Also the bug condition suggests a workaround to pre-SELECT sequence tables before START SLAVE. Reviewed-by: Brandon Nesterenko <brandon.nesterenko@mariadb.com>
-
Kristian Nielsen authored
The case is statement format and mixed InnoDB/MyISAM without binlog_direct_non_trans_update. Fix due to Brandon Nesterenko. Reviewed-by: Andrei Elkin <andrei.elkin@mariadb.com> Signed-off-by: Kristian Nielsen <knielsen@knielsen-hq.org>
-
Kristian Nielsen authored
The largest_started_sub_id needs to be set under LOCK_parallel_entry together with testing stop_sub_id. However, in-between was the logic for do_ftwrl_wait(), which temporarily releases the mutex. This could lead to inconsistent stopping amongst worker threads and lost data. Fix by moving all the stop-related logic out from unrelated do_gco_wait() and do_ftwrl_wait() and into its own function do_stop_handling(). Reviewed-by: Andrei Elkin <andrei.elkin@mariadb.com> Signed-off-by: Kristian Nielsen <knielsen@knielsen-hq.org>
-
Kristian Nielsen authored
Various test cases for the bugs around MDEV-31448. Test cases due to Brandon Nesterenko, thanks! Reviewed-by: Andrei Elkin <andrei.elkin@mariadb.com> Signed-off-by: Kristian Nielsen <knielsen@knielsen-hq.org>
-
Kristian Nielsen authored
The problem is that when a worker thread is (user) killed in wait_for_prior_commit, the event group may complete out-of-order since the wait for prior commit was aborted by the kill. This fix ensures that event groups will always complete in-order, even in the error case. This is done in finish_event_group() by doing an extra wait_for_prior_commit(), if necessary, that ignores kills. This fix supersedes the fix for MDEV-30780, so the earlier fix for that is reverted in this patch. Also fix that an error from wait_for_prior_commit() inside finish_event_group() would not signal the error to wakeup_subsequent_commits(). Based on earlier work by Brandon Nesterenko and Andrei Elkin, with some changes to simplify the semantics of wait_for_prior_commit() and make the code more robust to future changes. Reviewed-by: Andrei Elkin <andrei.elkin@mariadb.com> Signed-off-by: Kristian Nielsen <knielsen@knielsen-hq.org>
-
Kristian Nielsen authored
The problem was an incorrect unmark_start_commit() in signal_error_to_sql_driver_thread(). If an event group gets an error, this unmark could run after the following GCO started, and the subsequent re-marking could access de-allocated GCO. The offending unmark_start_commit() looks obviously incorrect, and the fix is to just remove it. It was introduced in the MDEV-8302 patch, the commit message of which suggests it was added there solely to satisfy an assertion in ha_rollback_trans(). So update this assertion instead to not trigger for event groups that experienced an error (rgi->worker_error). When an error occurs in an event group, all following event groups are skipped anyway, so the unmark should never be needed in this case. Reviewed-by: Andrei Elkin <andrei.elkin@mariadb.com> Signed-off-by: Kristian Nielsen <knielsen@knielsen-hq.org>
-
Kristian Nielsen authored
At STOP SLAVE, worker threads will continue applying event groups until the end of the current GCO before stopping. This is a left-over from when only conservative mode was available. In optimistic and aggressive mode, often _all_ queued event will be in the same GCO, and slave stop will be needlessly delayed. This patch instead records at STOP SLAVE time the latest (highest sub_id) event group that has started. Then worker threads will continue to apply event groups up to that event group, but skip any following. The result is that each worker thread will complete its currently running event group, and then the slave will stop. If the slave is caught up, and STOP SLAVE is run in the middle of an event group that is already executing in a worker thread, then that event group will be rolled back and the slave stop immediately, as normal. Reviewed-by: Andrei Elkin <andrei.elkin@mariadb.com> Signed-off-by: Kristian Nielsen <knielsen@knielsen-hq.org>
-
Kristian Nielsen authored
Signed-off-by: Kristian Nielsen <knielsen@knielsen-hq.org>
-
- 11 Jul, 2023 1 commit
-
-
Daniel Black authored
Docker when mounting a configuration file into a Windows exposes the file with permission 0777. These world writable files are ignored by by MariaDB. Add the access check such that filesystem RO or immutable file is counted as sufficient protection on the file. Test: $ mkdir /tmp/src $ vi /tmp/src/my.cnf $ chmod 666 /tmp/src/my.cnf $ mkdir /tmp/dst $ sudo mount --bind /tmp/src /tmp/dst -o ro $ ls -la /tmp/dst total 4 drwxr-xr-x. 2 dan dan 60 Jun 15 15:12 . drwxrwxrwt. 25 root root 660 Jun 15 15:13 .. -rw-rw-rw-. 1 dan dan 10 Jun 15 15:12 my.cnf $ mount | grep dst tmpfs on /tmp/dst type tmpfs (ro,seclabel,nr_inodes=1048576,inode64) strace client/mariadb --defaults-file=/tmp/dst/my.cnf newfstatat(AT_FDCWD, "/tmp/dst/my.cnf", {st_mode=S_IFREG|0666, st_size=10, ...}, 0) = 0 access("/tmp/dst/my.cnf", W_OK) = -1 EROFS (Read-only file system) openat(AT_FDCWD, "/tmp/dst/my.cnf", O_RDONLY|O_CLOEXEC) = 3 The one failing test, but this isn't a regression, just not a total fix: $ chmod u-w /tmp/src/my.cnf $ ls -la /tmp/src/my.cnf -r--rw-rw-. 1 dan dan 18 Jun 16 10:22 /tmp/src/my.cnf $ strace -fe trace=access client/mariadb --defaults-file=/tmp/dst/my.cnf access("/etc/ld.so.preload", R_OK) = -1 ENOENT (No such file or directory) access("/etc/system-fips", F_OK) = -1 ENOENT (No such file or directory) access("/tmp/dst/my.cnf", W_OK) = -1 EACCES (Permission denied) Warning: World-writable config file '/tmp/dst/my.cnf' is ignored Windows test (Docker Desktop ~4.21) which was the important one to fix: dan@LAPTOP-5B5P7RCK:~$ docker run --rm -v /mnt/c/Users/danie/Desktop/conf:/etc/mysql/conf.d/:ro -e MARIADB_ROOT_PASSWORD=bob quay.io/m ariadb-foundation/mariadb-devel:10.4-MDEV-27038-ro-mounts-pkgtest ls -la /etc/mysql/conf.d total 4 drwxrwxrwx 1 root root 512 Jun 15 13:57 . drwxr-xr-x 4 root root 4096 Jun 15 07:32 .. -rwxrwxrwx 1 root root 43 Jun 15 13:56 myapp.cnf root@a59b38b45af1:/# strace -fe trace=access mariadb access("/etc/ld.so.preload", R_OK) = -1 ENOENT (No such file or directory) access("/etc/mysql/conf.d/myapp.cnf", W_OK) = -1 EROFS (Read-only file system)
-
- 10 Jul, 2023 2 commits
-
-
Monty authored
The problematic query outlined a bug in window functions sorting optimization. When multiple window functions are present in a query, we sort the sorting key (as defined by PARTITION BY and ORDER BY) from generic to specific. SELECT RANK() OVER (ORDER BY const_col) as r1, RANK() OVER (ORDER BY const_col, a) as r2, RANK() OVER (PARTITION BY c) as r3, RANK() OVER (PARTITION BY c ORDER BY b) as r4 FROM table; For these functions, the sorting we need to do for window function computations are: [(const_col), (const_col, a)] and [(c), (c, b)]. Instead of doing 4 different sort order, the sorts grouped within [] are compatible and we can use the most *specific* sort to cover both window functions. The bug was caused by an incorrect flagging of which sort is most specific for a compatible group of functions. In our specific test case, instead of picking (const_col, a) as the most specific sort, it would only sort by (const_col), which lead to wrong results for rank function. By ensuring that we pick the last sort key before an "incompatible sort" flag is met in our "ordered array of sorting specifications", we guarantee correct results.
-
Marko Mäkelä authored
print_summary(): Skip index_ids for which index.pages is 0. Tablespaces may contain some freed pages that used to refer to indexes or tables that were dropped.
-
- 07 Jul, 2023 3 commits
-
-
Lawrin Novitsky authored
If procedure is changed in one connection, and other procedure has already called the initial version of the procedure, the query to INFORMATION_SCHEMA.PARAMETERS would use obsolete information from sp cache for that connection. That happens because cache invalidating method only increments cache version, and does not flush (all) the cache(s), and changing of a procedure only invalidates cache, and removes the procedure's cache entry from local thread cache only. The fix adds the check if sp info obtained from the cache for forming of results for the query to I_S, is not obsoleted, and does not use it, if it is. The test has been added to main.information_schema. It changes the SP in one connection, and ensures, that the change is seen in the query to the I_S.PARAMETERS in other connection, that already has called the procedure before the change.
-
Yury Chaikou authored
MDEV-24712 get_partition_set is never executed in ha_partition::multi_range_key_create_key due to bitwise & with 0 constant use == to compare enum (despite the name it is not a bit flag)
-
Oleg Smirnov authored
There was no actual execution of the SQL of a pushed derived table, which caused "r_rows" to be always displayed as 0 and "r_total_time_ms" to show inaccurate numbers. This commit makes a derived table SQL to be executed by the storage engine, so the server is able to calculate the number of rows returned and measure the execution time more accurately
-
- 06 Jul, 2023 2 commits
-
-
Vlad Lesin authored
PROBLEM: A deadlock was possible when a transaction tried to "upgrade" an already held Record Lock to Next Key Lock. SOLUTION: This patch is based on observations that: (1) a Next Key Lock is equivalent to Record Lock combined with Gap Lock (2) a GAP Lock never has to wait for any other lock In case we request a Next Key Lock, we check if we already own a Record Lock of equal or stronger mode, and if so, then we change the requested lock type to GAP Lock, which we either already have, or can be granted immediately, as GAP locks don't conflict with any other lock types. (We don't consider Insert Intention Locks a Gap Lock in above statements). The reason of why we don't upgrage Record Lock to Next Key Lock is the following. Imagine a transaction which does something like this: for each row { request lock in LOCK_X|LOCK_REC_NOT_GAP mode request lock in LOCK_S mode } If we upgraded lock from Record Lock to Next Key lock, there would be created only two lock_t structs for each page, one for LOCK_X|LOCK_REC_NOT_GAP mode and one for LOCK_S mode, and then used their bitmaps to mark all records from the same page. The situation would look like this: request lock in LOCK_X|LOCK_REC_NOT_GAP mode on row 1: // -> creates new lock_t for LOCK_X|LOCK_REC_NOT_GAP mode and sets bit for // 1 request lock in LOCK_S mode on row 1: // -> notices that we already have LOCK_X|LOCK_REC_NOT_GAP on the row 1, // so it upgrades it to X request lock in LOCK_X|LOCK_REC_NOT_GAP mode on row 2: // -> creates a new lock_t for LOCK_X|LOCK_REC_NOT_GAP mode (because we // don't have any after we've upgraded!) and sets bit for 2 request lock in LOCK_S mode on row 2: // -> notices that we already have LOCK_X|LOCK_REC_NOT_GAP on the row 2, // so it upgrades it to X ...etc...etc.. Each iteration of the loop creates a new lock_t struct, and in the end we have a lot (one for each record!) of LOCK_X locks, each with single bit set in the bitmap. Soon we run out of space for lock_t structs. If we create LOCK_GAP instead of lock upgrading, the above scenario works like the following: // -> creates new lock_t for LOCK_X|LOCK_REC_NOT_GAP mode and sets bit for // 1 request lock in LOCK_S mode on row 1: // -> notices that we already have LOCK_X|LOCK_REC_NOT_GAP on the row 1, // so it creates LOCK_S|LOCK_GAP only and sets bit for 1 request lock in LOCK_X|LOCK_REC_NOT_GAP mode on row 2: // -> reuses the lock_t for LOCK_X|LOCK_REC_NOT_GAP by setting bit for 2 request lock in LOCK_S mode on row 2: // -> notices that we already have LOCK_X|LOCK_REC_NOT_GAP on the row 2, // so it reuses LOCK_S|LOCK_GAP setting bit for 2 In the end we have just two locks per page, one for each mode: LOCK_X|LOCK_REC_NOT_GAP and LOCK_S|LOCK_GAP. Another benefit of this solution is that it avoids not-entirely const-correct, (and otherwise looking risky) "upgrading". The fix was ported from mysql/mysql-server@bfba840dfa7794b988c59c94658920dbe556075d mysql/mysql-server@75cefdb1f73b8f8ac8e22b10dfb5073adbdfdfb0 Reviewed by: Marko Mäkelä
-
Alexander Barkov authored
"mtr --view-protocol func_math" failed because of a too long column names imlicitly generated for the underlying expressions. With --view-protocol they were replaced to "Name_exp_1". Adding column aliases for these expressions.
-
- 05 Jul, 2023 1 commit
-
-
Rucha Deodhar authored
MDEV-23187 misses resetting collation connection causing test failures for 10.5+ bugs.
-
- 04 Jul, 2023 2 commits
-
-
Kristian Nielsen authored
Fix that rpl_slave_state::load() was calling rpl_slave_state::update() without holding LOCK_slave_state. Reviewed-by: Monty <monty@mariadb.org> Signed-off-by: Kristian Nielsen <knielsen@knielsen-hq.org>
-
Yuchen Pei authored
This fixes mdev_26541.test, and the new clean_up_spider.inc will be useful for other tests where part of deinit_spider does not apply, e.g. those testing spider initialisation only.
-
- 03 Jul, 2023 5 commits
-
-
Vicentiu Ciorbaru authored
The original code generated a warning in gcc 13.1
-
Sergei Golubchik authored
-
Sergei Golubchik authored
make it "query reached <some limit> result may be incomplete"
-
Sergei Golubchik authored
also make sure all unused error messages are "You should never see it" and have no translations
-
Marko Mäkelä authored
btr_node_ptr_max_size(): Handle BINARY(0) and VARBINARY(0) as special cases, similar to CHAR(0) and VARCHAR(0).
-