- 25 Jul, 2023 1 commit
-
-
Yuchen Pei authored
We introduce simple plugin dependency. A plugin init function may return HA_ERR_RETRY_INIT. If this happens during server startup when the server is trying to initialise all plugins, the failed plugins will be retried, until no more plugins succeed in initialisation or want to be retried. This will fix spider init bugs which is caused in part by its dependency on Aria for initialisation. The reason we need a new return code, instead of treating every failure as a request for retry, is that it may be impossible to clean up after a failed plugin initialisation. Take InnoDB for example, it has a global variable `buf_page_cleaner_is_active`, which may not satisfy an assertion during a second initialisation try, probably because InnoDB does not expect the initialisation to be called twice.
-
- 24 Jul, 2023 2 commits
-
-
Oleksandr Byelkin authored
-
-
- 23 Jul, 2023 2 commits
-
-
Georg Richter authored
Since TLS server certificate verification is a client only option, this flag is removed in both client (C/C) and MariaDB server capability flags. This patch reverts commit 89d759b9 (MySQL Bug #21543) and stores the server certificate validation option in mysql->options.extensions.
-
Georg Richter authored
Since TLS server certificate verification is a client only option, this flag is removed in both client (C/C) and MariaDB server capability flags. This patch reverts commit 89d759b9 (MySQL Bug #21543) and stores the server certificate validation option in mysql->options.extensions.
-
- 21 Jul, 2023 1 commit
-
-
Daniel Black authored
noinline attribute was being ignored by clang-16 and reporting 32 stack size on Gentoo, 16 locally on Fedora 38. Based on https://stackoverflow.com/questions/54481855/clang-ignoring-attribute-noinline appended noopt in addition to the gcc recognised attributes. After that the -pcre_exec(NULL, NULL, NULL, -999, -999, 0, NULL, 0) returned 1056, simlar to gcc. From https://bugs.gentoo.org/910188. Thanks Zhixu Liu for the great bug report.
-
- 20 Jul, 2023 14 commits
-
-
Aleksey Midenkov authored
Restrict vcol_cleanup_expr() in close_thread_tables() to only simple locked tables mode. Prelocked is cleaned up like normal statement: in close_thread_table().
-
Aleksey Midenkov authored
First UPDATE under START TRANSACTION does nothing (nstate= nstate), but anyway generates history. Since update vector is empty we get into (!uvect->n_fields) branch which only adds history row, but does not do update. After that we get current row with wrong (old) row_start value and because of that second UPDATE tries to insert history row again because it sees trx->id != row_start which is the guard to avoid inserting multiple trx_id-based history rows under same transaction (because we have same trx_id and we get duplicate error and this bug demostrates that). But this try anyway fails because PK is based on row_end which is constant under same transaction, so PK didn't change. The fix moves vers_make_update() to an earlier stage of calc_row_difference(). Therefore it prepares update vector before (!uvect->n_fields) check and never gets into that branch, hence no need to handle versioning inside that condition anymore. Now trx->id and row_start are equal after first UPDATE and we don't try to insert second history row. == Cleanups and improvements == ha_innobase::update_row(): vers_set_fields and vers_ins_row are cleaned up into direct condition check. SQLCOM_ALTER_TABLE check now is not used as this is dead code, assertion is done instead. upd_node->is_delete is set in calc_row_difference() just to keep versioning code as much in one place as possible. vers_make_delete() is still located in row_update_for_mysql() as this is required for ha_innodbase::delete_row() as well. row_ins_duplicate_error_in_clust(): Restrict DB_FOREIGN_DUPLICATE_KEY to the better conditions. VERSIONED_DELETE is used specifically to help lower stack to understand what caused current insert. Related to MDEV-29813.
-
Aleksey Midenkov authored
On create table tmp as select ... we exited Item_func::fix_fields() with error. fix_fields_if_needed('foo' or 'bar') failed and we returned true, but already changed const_item_cache. So the item is in inconsistent state: fixed == false and const_item_cache == false. Now we cleanup the item before the return if Item_func::fix_fields() fails to process.
-
Aleksey Midenkov authored
Use vers_check_update() to avoid inserting history row for ODKU if now versioned fields specified in update_fields.
-
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
-