Console View
|
Categories: connectors experimental galera main |
|
| connectors | experimental | galera | main | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
|
|
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Raghunandan Bhat
raghunandan.bhat96@gmail.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-41193: ASAN heap-buffer-overflow in ha_connect::CheckCond after select from Connect table Problem: When CONNECT engine pushes a WHERE clause down to an external table, it writes the filter into the work area, without checking how much space is left. A large string literal in the WHERE clause can overflow the work area allocated by the engine. For ex: if connect_work_size is set to 4MB and the string literal in the WHERE clause is larger than 4MB, it can grow past the allocated work area. Fix: Track the space left in the work area and check it before writing. If the filter doesn't fit, drop it instead of writing past the buffer. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Alexander Barkov
bar@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-41246 "Illegal mix of collations" on the mysql.user view In progress |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Dave Gosselin
dave.gosselin@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-40573: Crash on multi-table DELETE with an impossible WHERE A DELETE containing a single table, an index hint, an impossible WHERE condition, and a window function will take the multi-delete code path but never initialize tables for deletion, leading to a crash. Such a statement would never delete rows from the target table. Record in the multi_delete whether it was ever initialized for execution, and don't attempt to delete anything if it wasn't initialized. The index hint forces the single table DELETE to take the multi-table codepath. Since this case has an impossible WHERE condition, we set subq_exit_fl which later causes JOIN::optimize_stage2 to skip the multi-delete table initialization. It's not safe to attempt initialization when trying to find a "tableless" subquery plan, so defend against this case with the new multi-delete flag added by this commit. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Alexander Barkov
bar@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-41246 "Illegal mix of collations" on the mysql.user view In progress |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Raghunandan Bhat
raghunandan.bhat96@gmail.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-41193: ASAN heap-buffer-overflow in ha_connect::CheckCond after select from Connect table Problem: When CONNECT engine pushes a WHERE clause down to an external table, it writes the filter into the work area, without checking how much space is left. A large string literal in the WHERE clause can overflow the work area allocated by the engine. For ex: if connect_work_size is set to 4MB and the string literal in the WHERE clause is larger than 4MB, it can grow past the allocated work area. Fix: Track the space left in the work area and check it before writing. If the filter doesn't fit, drop it instead of writing past the buffer. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Rucha Deodhar
rucha.deodhar@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-39049: Memory corruption & crash in check_key_in_list upon using JSON_KEYS after modifying character set name/collation Analysis: Buffer overflow crashes and empty key duplication in check_key_in_list. Fix: Checking result buffer validity prevents segmentation faults on empty strings and malformed inputs while preserving correct key matching behavior. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Marko Mäkelä
marko.makela@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fixup! b387a4a6f9f9b3194a29c1a80c39c983d5dc4fd5 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Aleksey Midenkov
midenok@gmail.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-41157 CREATE DATABASE COMMENT overflows db.opt comment buffer Bug 1: put_dbopt() used strmov() to copy schema_comment into a fixed DATABASE_COMMENT_MAXLEN+1 buffer. validate_comment_length() only truncates comment->length in non-strict sql_mode, leaving comment->str NUL-terminated at its original (unbounded) length. strmov() copies until the source NUL, ignoring the truncated length, overflowing the destination buffer for long comments. The fix uses strmake() bounded by comment->length instead, matching the LEX_CSTRING contract (length is authoritative, str need not be NUL-terminated at length). Bug 2: write_db_opt() used strxnmov() to copy the full un-truncated comment until it ran out of buffer space mid-string with no trailing newline. Which made load_db_opt() silently discard the whole unterminated "comment=" line on the next restart, losing the comment entirely instead of just truncating it. The fix bounds the comment copy into db.opt by the already-validated comment->length via strmake(), instead of relying on the source string's own NUL terminator, matching the put_dbopt() fix. Bug 3: validate_comment_length() only runs on a COMMENT clause given in the current statement. ALTER DATABASE without one instead pulls the existing comment off disk via load_db_opt(), which never bounded it. That unvalidated length then reached write_db_opt()'s own comment= copy into its stack buffer, so a legacy or hand-edited db.opt with an overlong comment= line overflowed it on ALTER DATABASE. The fix: load_db_opt() now clamps the parsed comment to DATABASE_COMMENT_MAXLEN right when it reads the "comment=" line, so every consumer (put_dbopt(), write_db_opt()'s ALTER path) always sees an already-bounded value. The clamp itself must truncate by bytes, not characters: Well_formed_prefix()'s LEX_CSTRING overload takes a character count, but DATABASE_COMMENT_MAXLEN sizes the buffers in bytes. Bug 4: write_db_opt() still trusted validate_comment_length() to bound a directly-given COMMENT to DATABASE_COMMENT_MAXLEN bytes, but it only bounds it to DATABASE_COMMENT_MAXLEN *characters* -- so a multi-byte comment could still overflow the same fixed buffers. The fix: write_db_opt() clamps schema_comment to DATABASE_COMMENT_MAXLEN bytes itself, right after validate_comment_length() returns. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Arcadiy Ivanov
arcadiy@ivanov.biz |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-41297 A stored empty blob never equals an all-space value Blob values for empty strings should return a pointer to an empty string and not NULL. `hp_materialize_one_blob()` returned NULL, which its callers `hp_rec_key_cmp()` and `hp_key_cmp()` read as an allocation failure. `hp_test_write_dup-t.c` was extended to test key reads on `TEXT` columns. This could not be done in MTR, as a `MEMORY` table cannot be created with a key on a `TEXT` column. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Aleksey Midenkov
midenok@gmail.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-41157 CREATE DATABASE COMMENT overflows db.opt comment buffer Bug 1: put_dbopt() used strmov() to copy schema_comment into a fixed DATABASE_COMMENT_MAXLEN+1 buffer. validate_comment_length() only truncates comment->length in non-strict sql_mode, leaving comment->str NUL-terminated at its original (unbounded) length. strmov() copies until the source NUL, ignoring the truncated length, overflowing the destination buffer for long comments. The fix uses strmake() bounded by comment->length instead, matching the LEX_CSTRING contract (length is authoritative, str need not be NUL-terminated at length). Bug 2: write_db_opt() used strxnmov() to copy the full un-truncated comment until it ran out of buffer space mid-string with no trailing newline. Which made load_db_opt() silently discard the whole unterminated "comment=" line on the next restart, losing the comment entirely instead of just truncating it. The fix bounds the comment copy into db.opt by the already-validated comment->length via strmake(), instead of relying on the source string's own NUL terminator, matching the put_dbopt() fix. Bug 3: validate_comment_length() only runs on a COMMENT clause given in the current statement. ALTER DATABASE without one instead pulls the existing comment off disk via load_db_opt(), which never bounded it. That unvalidated length then reached write_db_opt()'s own comment= copy into its stack buffer, so a legacy or hand-edited db.opt with an overlong comment= line overflowed it on ALTER DATABASE. The fix: load_db_opt() now clamps the parsed comment to DATABASE_COMMENT_MAXLEN right when it reads the "comment=" line, so every consumer (put_dbopt(), write_db_opt()'s ALTER path) always sees an already-bounded value. The clamp itself must truncate by bytes, not characters: Well_formed_prefix()'s LEX_CSTRING overload takes a character count, but DATABASE_COMMENT_MAXLEN sizes the buffers in bytes. Bug 4: write_db_opt() still trusted validate_comment_length() to bound a directly-given COMMENT to DATABASE_COMMENT_MAXLEN bytes, but it only bounds it to DATABASE_COMMENT_MAXLEN *characters* -- so a multi-byte comment could still overflow the same fixed buffers. The fix: write_db_opt() clamps schema_comment to DATABASE_COMMENT_MAXLEN bytes itself, right after validate_comment_length() returns. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Marko Mäkelä
marko.makela@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-33966: buf_page_make_young() is a contention point The buf_pool.LRU list needs to reasonably accurately reflect recently accessed blocks, so that they will not be evicted prematurely. Because the list is protected by buf_pool.mutex, it is not a good idea to maintain the position on every page access. Instead of maintaining the LRU position on each access, we will decide on each access whether the block qualifies for promotion, and record the decision in a "promote" flag in the block descriptor. The buf_flush_page_cleaner() thread as well as some traversal of the buf_pool.LRU list, all of which hold buf_pool.mutex anyway, will move the flagged blocks to the "recently used" end of buf_pool.LRU. A block qualifies only if it is accessed again at least innodb_old_blocks_time after the reference point, which is its first access and, after that, its most recent promotion. The age is measured in whole seconds and checked on the access, not when a sweep reaches the block, so that a block that a table scan accessed in one burst will not be promoted, however long it stays in buf_pool.LRU_old. After a promotion, the age is measured from the time the sweep moved the block, which can be later than the access that qualified it. The rule applies at any position in buf_pool.LRU, and the flag stays set until a sweep moves the block. Thus, a block that qualified outside buf_pool.LRU_old keeps the flag after it moves into buf_pool.LRU_old; before, such a block was moved on any access when freed_page_clock showed that it was no longer close to the "recently used" end. buf_page_make_young_if_needed(), buf_page_make_young(), buf_page_peek_if_too_old(), buf_page_peek_if_young(), btr_cur_nonleaf_make_young(), buf_page_t::set_accessed(): Replaced by buf_page_t::touch(), buf_page_t::touch_no_stamp() and buf_page_t::make_young_if_needed(). buf_pool_t::freed_page_clock, buf_page_t::freed_page_clock: Remove. This is no longer meaningful in the revised design. INFORMATION_SCHEMA.INNODB_BUFFER_PAGE(_LRU).FREE_PAGE_CLOCK now always reports 0. Before the first eviction, buf_LRU_stat_update() now records statistics intervals, and buf_LRU_evict_from_unzip_LRU() uses its formula instead of assuming a disk-bound workload. page_zip_des_t::state: An atomic 16-bit field that will include the PROMOTE and OLD flags that would more logically belong to buf_page_t. We maintain them here (along with some ROW_FORMAT=COMPRESSED specific state that is protected by page latches) in order to avoid race conditions and unnecessary memory overhead. m_end moves out to its own field, shrinking the packed word to 16 bits; n_blobs narrows from 12 bits to 10 to make room for PROMOTE and OLD, still comfortably above the 744-column maximum. buf_page_t::init(): Takes the ROW_FORMAT=COMPRESSED shift size (ssize) directly and clears the zip descriptor state itself, so callers (buf_block_t::initialise(), buf_page_init_for_read()) no longer need a separate page_zip_des_init()/page_zip_set_size() call. buf_page_t::invalidate(): Replaces buf_block_modify_clock_inc(). Instead of maintaining a 64-bit counter, we will maintain one comprising 32+16=48 bits, in modify_clock_low,modify_clock_high. Worst case there will be exactly n<<48 calls to buf_page_t::invalidate() before some operation such as btr_pcur_t::restore_position() is executed. Such a count should be extremely unlikely but not completely impossible. It is worth noting that the DB_TRX_ID is only 48 bits, and each transaction start and commit/rollback will consume an identifier. buf_page_t::modify_clock(): Replaces the read access of buf_page_t::modify_clock. Assert that the caller is holding a page latch. Note: because invalidate() and modify_clock() are protected with buf_pool.mutex or the buf_page_t::lock, there can be no issue with regard to the atomicity of accessing the 48-bit field. buf_page_t::access_time: Store the 16-bit buf_pool.access_clock rather than a 32-bit millisecond ut_time_ms(). It wraps around every 18.2 hours; ages are computed as uint16_t(now - access_time), which stays correct across that wrap. This avoids any alignment loss: the adjacent fields modify_clock_low, modify_clock_high, access_time of 32+16+16 bits nicely add up to 64 bits. access_time is stamped on the first access after the block was initialized, and on each promotion by make_young_if_needed(), so that the age of a frequently promoted block stays exact across the wrap. buf_pool_t::access_clock: uint16_t(my_interval_timer() / 1000000000), never 0 (see buf_pool_t::now()), refreshed about once per second by buf_pool_t::refresh_clock() so that page accesses need not read the system clock. srv_master_callback() refreshes it. From buf_pool_t::create() until srv_master_timer is started, and for good when srv_master_timer is not started (innodb_read_only, innodb_force_recovery>=2, mariadb-backup), a separate buf_pool_clock_timer refreshes it. buf_pool_t::access_clock, buf_pool_t::LRU_old_threshold: Located in a cache line of their own, because they are read on page accesses and the adjacent buf_pool fields are frequently written. buf_page_t::touch(): Stamp access_time on the first access, then invoke touch_no_stamp(). Return whether this was not the first access, as the result of buf_page_make_young_if_needed() used to be. A buffer-fix is sufficient, as in MVCC undo page lookups. buf_page_t::touch_no_stamp(): Set the PROMOTE flag if innodb_old_blocks_time is 0, or if accessed_at() is at least that old, at any position of the block in buf_pool.LRU. Like btr_cur_nonleaf_make_young(), do not stamp access_time. Once PROMOTE is set, later accesses only load the state and return. buf_page_t::make_young_if_needed(): If the block is in buf_pool.LRU_old and PROMOTE is set, clear the flag and invoke buf_page_t::make_young(). This part is inline, so that a sweep pays no function call for a block that stays in place. PROMOTE is cleared only here. A block outside buf_pool.LRU_old keeps PROMOTE until it is old. The template parameter count_not_young selects whether an old block that was accessed but not flagged is counted in buf_pool.stat.n_pages_not_made_young; only the eviction sweeps buf_LRU_free_from_common_LRU_list() and buf_flush_LRU_list_batch() do this, and a block that a sweep leaves in the list can be counted again by a later sweep. buf_page_t::make_young(): Stamp access_time and move the block to the "recently used" end of buf_pool.LRU. Because only old blocks are moved, buf_pool.stat.n_pages_made_young now counts every move: for the same workload, Innodb_buffer_pool_pages_made_young can be higher than before, although fewer blocks are moved. The block can be read-fixed, because buf_pool_t::unzip() copies PROMOTE and OLD from the compressed-only descriptor and releases buf_pool.mutex during buf_zip_decompress(). Unlike buf_page_make_young(), we do not skip such a block: a page access no longer acquires buf_pool.mutex, and the sweeps hold buf_pool.mutex but no buffer-fix on the block. innodb_old_blocks_time_update(): New sysvar update callback, replacing a NULL one, that calls buf_pool_t::set_old_threshold_ms(), so that SET GLOBAL innodb_old_blocks_time also updates the LRU_old_threshold in seconds that page accesses read. The threshold is clamped to 65535 seconds, matching the access_time wrap period; the sysvar itself still accepts up to UINT_MAX32 milliseconds. buf_pool_t::set_old_threshold_ms(): Round the millisecond threshold up, not down, to the nearest second. innodb_old_blocks_time is documented and accepted in milliseconds; flooring instead of ceiling would make any configured value from 1 to 999 silently behave as 0 (disabled). buf_page_t::is_accessed(): Renamed accessed_at(), to stop reading as a boolean. It returns the access_time stamp of the first access or of the last promotion, in seconds; INFORMATION_SCHEMA.INNODB_BUFFER_PAGE(_LRU).ACCESS_TIME now reflects that. buf_read_ahead_random(): A page now qualifies once accessed_at() holds, together with either zip.is_promote() or !zip.old(). buf_read_ahead_linear(): Compare access_time stamps as a signed 16-bit difference, not raw unsigned, so the monotonic-access check stays correct across the access_time wrap. The resolution of the stamps is 1 second instead of 1 millisecond. buf_flush_page_cleaner(): Refresh abstime before proceeding to LRU eviction after an idle period, so that the next my_cond_timedwait() will not return immediately on a stale deadline. buf_LRU_scan_and_free_block(): Declare static. buf_flush_LRU_list_batch(): In a run of blocks that make_young_if_needed() moves, release and reacquire buf_pool.mutex after every 512 scanned blocks, except on the first scanned block. A move costs much less than an eviction, so the stride is longer than the one of the eviction path. buf_pool_invalidate(): Define in the same compilation unit with buf_LRU_scan_and_free_block(). buf_pool.LRU_old_time_threshold: Replaces buf_LRU_old_threshold_ms. PageConverter::run(): Renamed from fil_iterate(). In debug builds, initialize and acquire a dummy exclusive latch on the block, so the assertion in buf_page_t::invalidate() is satisfied; free the latch on every path, not only on success. AbstractCallback::m_zip_ssize: Replaces m_zip_size. page_zip_des_t: Add calc_ssize()/zip_size() helpers, replacing the zip_size<->ssize conversion duplicated across buf0buf.cc, buf0rea.cc and row0import.cc. innodb.buf_lru_scan_resistance: A new big test that checks that pages of a table that is accessed in one burst are evicted, even if a sweep reaches them only after innodb_old_blocks_time, that pages accessed again after that time are promoted, and that pages read while the buffer pool is being filled are not promoted later. innodb_zip.n_blobs_700: A new test that stores 700 BLOB pointers on one ROW_FORMAT=COMPRESSED page, within the 10-bit n_blobs field. Co-Authored-By: Alessandro Vetere <[email protected]> |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Dave Gosselin
dave.gosselin@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-40573: Crash on multi-table DELETE with an impossible WHERE A DELETE containing a single table, an index hint, an impossible WHERE condition, and a window function will take the multi-delete code path but never initialize tables for deletion, leading to a crash. Such a statement would never delete rows from the target table. Record in the multi_delete whether it was ever initialized for execution, and don't attempt to delete anything if it wasn't initialized. The index hint forces the single table DELETE to take the multi-table codepath. Since this case has an impossible WHERE condition, we set subq_exit_fl which later causes JOIN::optimize_stage2 to skip the multi-delete table initialization. It's not safe to attempt initialization when trying to find a "tableless" subquery plan, so defend against this case with the new multi-delete flag added by this commit. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Raghunandan Bhat
raghunandan.bhat96@gmail.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-41193: ASAN heap-buffer-overflow in ha_connect::CheckCond after select from Connect table Problem: When CONNECT engine pushes a WHERE clause down to an external table, it writes the filter into the work area, without checking how much space is left. A large string literal in the WHERE clause can overflow the work area allocated by the engine. For ex: if connect_work_size is set to 4MB and the string literal in the WHERE clause is larger than 4MB, it can grow past the allocated work area. Fix: Track the space left in the work area and check it before writing. If the filter doesn't fit, drop it instead of writing past the buffer. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Marko Mäkelä
marko.makela@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fixup! b387a4a6f9f9b3194a29c1a80c39c983d5dc4fd5 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Rucha Deodhar
rucha.deodhar@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| MDEV-41181: ASAN heap-buffer-overflow after SELECT JSON_SCHEMA_VALID | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Vladislav Vaintroub
vvaintroub@gmail.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
build mysqlservices without an embedded CRT requirement mysqlservices only exposes a thin C API, no CRT state crosses it, so don't force whatever CRT/config built the server onto a plugin linking it. Without /Zl, a plugin built in a config with no matching installed mysqlservices variant (CMake silently substitutes one - verified with a toy project) gets an ignorable but noisy LNK4098 warning. Assisted-by: Claude:claude-5-sonnet |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Marko Mäkelä
marko.makela@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-40852 Redundant checkpoint after innodb_log_archive startup log_t::set_recovered(): Do not unnecessarily set the circular_recovery_from_sequence_bit_0 flag for innodb_log_archive=ON format files. The purpose of the flag is to ensure that an extra checkpoint will be written when converting the log to innodb_log_archive=OFF format. The scenario that we want to prevent is that the log originally was in innodb_log_archive=OFF format and had wrapped around an odd number of times since the file creation, that is, the sequence bit at the end of the mini-transactions since the latest checkpoint is 0. After a conversion to innodb_log_archive=ON format, old records would carry the sequence bit 0 and new ones the bit 1. This is fine, because the recovery will ignore the sequence bit; innodb_log_archive=ON files never wrap around. However, when the log is converted back to innodb_log_archive=OFF format, we must guarantee that all sequence bits since the latest checkpoint were written as 1. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Aleksey Midenkov
midenok@gmail.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-41157 CREATE DATABASE COMMENT overflows db.opt comment buffer Bug 1: put_dbopt() used strmov() to copy schema_comment into a fixed DATABASE_COMMENT_MAXLEN+1 buffer. validate_comment_length() only truncates comment->length in non-strict sql_mode, leaving comment->str NUL-terminated at its original (unbounded) length. strmov() copies until the source NUL, ignoring the truncated length, overflowing the destination buffer for long comments. The fix uses strmake() bounded by comment->length instead, matching the LEX_CSTRING contract (length is authoritative, str need not be NUL-terminated at length). Bug 2: write_db_opt() used strxnmov() to copy the full un-truncated comment until it ran out of buffer space mid-string with no trailing newline. Which made load_db_opt() silently discard the whole unterminated "comment=" line on the next restart, losing the comment entirely instead of just truncating it. The fix bounds the comment copy into db.opt by the already-validated comment->length via strmake(), instead of relying on the source string's own NUL terminator, matching the put_dbopt() fix. Bug 3: validate_comment_length() only runs on a COMMENT clause given in the current statement. ALTER DATABASE without one instead pulls the existing comment off disk via load_db_opt(), which never bounded it. That unvalidated length then reached write_db_opt()'s own comment= copy into its stack buffer, so a legacy or hand-edited db.opt with an overlong comment= line overflowed it on ALTER DATABASE. The fix: load_db_opt() now clamps the parsed comment to DATABASE_COMMENT_MAXLEN right when it reads the "comment=" line, so every consumer (put_dbopt(), write_db_opt()'s ALTER path) always sees an already-bounded value. The clamp itself must truncate by bytes, not characters: Well_formed_prefix()'s LEX_CSTRING overload takes a character count, but DATABASE_COMMENT_MAXLEN sizes the buffers in bytes. Bug 4: write_db_opt() still trusted validate_comment_length() to bound a directly-given COMMENT to DATABASE_COMMENT_MAXLEN bytes, but it only bounds it to DATABASE_COMMENT_MAXLEN *characters* -- so a multi-byte comment could still overflow the same fixed buffers. The fix: write_db_opt() clamps schema_comment to DATABASE_COMMENT_MAXLEN bytes itself, right after validate_comment_length() returns. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Vladislav Vaintroub
vvaintroub@gmail.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-39533 my_realpath and MY_NOSYMLINKS on Windows On Windows, my_realpath() only called GetFullPathName(), which canonicalizes '.', '..' and drive letters but does not resolve NTFS symlinks, junctions or mount points, unlike POSIX realpath(). At the same time, my_open() and my_delete() ignored MY_NOSYMLINKS entirely, so the symlink-attack protection used for MyISAM/Aria's DATA DIRECTORY/INDEX DIRECTORY (mi_open()/ma_open(), my_handler_delete_with_symlink()) was silently absent on Windows. Fix my_realpath() to actually resolve reparse points: open the (syntactically canonicalized) path with CreateFile(), which follows them, and read back the handle's fully resolved path with GetFinalPathNameByHandle(). A not-found path still gets the same ENOENT/fallback contract as before. Make my_open() and my_delete() honor MY_NOSYMLINKS on Windows. Windows has no per-path-component O_NOFOLLOW equivalent, so instead this mirrors the realpath()-equality branch of the POSIX NOSYMLINK_FUNCTION_BODY macro: the caller-supplied name (expected to already be my_realpath()-resolved) is compared against the actually opened handle's resolved path, and rejected with ENOTDIR -- the same errno POSIX uses for this exact "not already canonical" condition -- on a mismatch, whether caused by a TOCTOU symlink swap or by the name never having been fully resolved to begin with. Add a my_symlink-t.c test that creates a real NTFS junction to verify resolution and enforcement. Co-Authored-By: Claude Sonnet 5 <[email protected]> |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Marko Mäkelä
marko.makela@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-40852 Redundant checkpoint after innodb_log_archive startup log_t::set_recovered(): Do not unnecessarily set the circular_recovery_from_sequence_bit_0 flag for innodb_log_archive=ON format files. The purpose of the flag is to ensure that an extra checkpoint will be written when converting the log to innodb_log_archive=OFF format. The scenario that we want to prevent is that the log originally was in innodb_log_archive=OFF format and had wrapped around an odd number of times since the file creation, that is, the sequence bit at the end of the mini-transactions since the latest checkpoint is 0. After a conversion to innodb_log_archive=ON format, old records would carry the sequence bit 0 and new ones the bit 1. This is fine, because the recovery will ignore the sequence bit; innodb_log_archive=ON files never wrap around. However, when the log is converted back to innodb_log_archive=OFF format, we must guarantee that all sequence bits since the latest checkpoint were written as 1. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Marko Mäkelä
marko.makela@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Fix a race between DROP TABLE and BACKUP SERVER | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Sergei Golubchik
serg@mariadb.org |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fix the build for -G "Ninja Multi-Config" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Mohammad Tafzeel Shams
tafzeel.shams@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-37467: InnoDB Instant ALTER TABLE is not crash safe The hidden metadata record of instant ALTER TABLE was not written crash-safely, and recovery could fail to roll it back. These are independent problems. First, the metadata record may include externally stored BLOB metadata. The existing BLOB storage path in btr_store_big_rec_extern_fields() writes the clustered index record first, with zero BLOB pointers, and only fills in the BLOB pointers afterwards. If the server is killed after the mini-transaction that wrote the (incomplete) metadata record was durably committed, but before the BLOB pointers were written, the table could become inaccessible on recovery. Make metadata BLOB storage crash-safe by writing the BLOB pages and computing their pointers before the metadata record itself is inserted or updated, so that the record is always written with complete BLOB pointers. If the server is killed before the metadata record is written, the already-written BLOB pages are merely orphaned, which is safe. Second, trx_undo_report_row_operation() writes the undo log record in a mini-transaction of its own, which is committed before the mini-transaction that writes the metadata record. Because innobase_instant_try() had already updated SYS_COLUMNS and SYS_TABLES in earlier mini-transactions, a kill in between left a durable undo log record for the table while the metadata record was unchanged. On recovery, trx_resurrect_table_locks() would then load the table definition before the incomplete transaction was rolled back. The data dictionary described the table as it would be after the operation, while the metadata record still described it as it was before, and btr_cur_instant_init() failed on that disagreement. Write the undo log record of the metadata record in the same mini-transaction that inserts or updates the record, so that the two cannot be separated by a crash: until that mini-transaction is committed, neither of them is durable. An undo log record is never split between pages. If the DEFAULT values of the columns being added are large enough that the undo log record for updating the metadata record would not fit on one page, innobase_instant_try() would fail. Determine this before the operation starts, so that it can be performed by another algorithm instead. Third, the table definition that recovery loads need not correspond to the metadata record. dict_load_table_one() reads the committed version of the SYS_TABLES record, and escalates to READ UNCOMMITTED only when it finds a SYS_COLUMNS record that was written by a transaction that is still active. The number of SYS_COLUMNS records that dict_load_columns() reads is derived from SYS_TABLES.N_COLS, which was read from the committed version. The record of a column that the operation appended is located after that many records, so it is never read and the operation goes unnoticed. Only an instant ALTER TABLE that merely appends columns can escape this way: ADD COLUMN ... FIRST, DROP COLUMN and column reordering rewrite the SYS_COLUMNS records of already existing columns. Detect this on the SYS_TABLES record itself, which is located by table name and therefore does not depend on N_COLS. Every instant ALTER TABLE that changes the columns updates that record, because innobase_instant_try() invokes innodb_update_cols(). Fourth, the rollback writes a metadata record that comprises fewer fields than the table definition describes, because btr_cur_trim_alter_metadata() shortens it to the number of fields that it comprised before the operation. That number determines the size of the null flag bitmap, and hence the position of the array of field lengths. rec_init_offsets_comp_ordinary() derives it from the record, while the two functions that write the record derived it from the table definition and asserted that the two agree. - btr_store_big_rec_metadata(): New function to store the off-page columns of a metadata record ahead of time. Each BLOB page is allocated and linked in its own mini-transaction, and the resulting BLOB pointers are written directly into the (heap-resident) index entry. On failure, it frees any pages it already allocated and resets the pointers to zero. - btr_free_big_rec_metadata(): New helper to free the BLOB pages written by btr_store_big_rec_metadata() and reset the entry's BLOB pointers to zero, used both on failure inside that function and by its callers when the metadata record ends up not being written. - row_ins_clust_index_entry_low(): For a metadata entry that needs external storage, convert it to a big record and call btr_store_big_rec_metadata() (with log_free_check() allowed, since no latches are held yet) before inserting the record. On failure, free the metadata BLOBs and convert the entry back. - btr_cur_pessimistic_update(): When updating a metadata record that requires external storage, call btr_store_big_rec_metadata() (without log_free_check(), since index and page latches are held) before modifying the record, and free the temporary big_rec vector via btr_free_big_rec_metadata() or dtuple_big_rec_free() on the various failure/success paths. - btr_cur_optimistic_insert(): Remove the special-cased jump to convert_big_rec for metadata entries, since their BLOBs are now always stored ahead of time by the caller; assert that a metadata entry never needs external storage at this point. - innobase_instant_try(): Since btr_cur_pessimistic_update() now stores metadata BLOBs before updating the record, big_rec is always NULL here; assert this instead of calling btr_store_big_rec_extern_fields(). - trx_undo_report_row_operation(): New parameter caller_mtr. If it is specified, the undo log record is written in that mini-transaction, which is never committed or restarted here. An undo log page is added within the same mini-transaction if the record does not fit on the current one. A temporary table never uses the caller's mini-transaction, because that would require changing its logging mode. All other callers pass NULL and are unaffected. - btr_cur_ins_lock_and_undo(), btr_cur_upd_lock_and_undo(): For an instant ALTER TABLE metadata record, pass the mini-transaction that is going to insert or modify the record. - trx_undo_max_rec_size(): New function to determine the maximum size of an undo log record, that is, the space available on an empty undo log page. - ha_innobase::check_if_supported_inplace_alter(): Refuse ALGORITHM=INSTANT if the metadata record already exists and the undo log record for updating it would exceed trx_undo_max_rec_size(). trx_undo_page_report_modify() stores the DEFAULT value of each column that is being added in that record in full, inline. No such limit applies when the metadata record is being inserted, because trx_undo_page_report_insert() writes TRX_UNDO_INSERT_METADATA and no field data. - dict_load_table_one(): If the SYS_TABLES record was written by a transaction that is still active, load the table definition as READ UNCOMMITTED. A delete-marked record is excluded, because SYS_TABLES.NAME is the clustered index key: RENAME TABLE delete-marks the record of the old name, and the definition that corresponds to that name is the one that precedes the rename. - dict_sys_tables_rec_read(): New parameter uncommitted_rec. This function already determines whether the current version of the record was written by a transaction that has not been committed, in order to decide whether to read an older version of it. Report that to the caller instead of discarding it. - dict_load_table_low(): New parameter uncommitted_rec, which is passed on to dict_sys_tables_rec_read(). - rec_get_converted_size_comp_prefix_low(), rec_convert_dtuple_to_rec_comp(): For a record that includes a metadata BLOB, determine the number of nullable fields from the tuple, by way of dict_index_t::get_n_nullable(), and not from dict_index_t::n_nullable. This is what rec_init_offsets_comp_ordinary() does, and it is equivalent for a tuple that comprises all fields of the index. Relax the assertions that required the tuple to comprise all of them. - Added test in innodb.instant_alter and innodb.instant_alter_crash to test normal working of INSTANT ALTER, crash safety and full table. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Dave Gosselin
dave.gosselin@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-35845: Propagate a constant into an IN predicate SELECT * FROM t1 WHERE v IN ('a','b') AND v = 'b' kept both conjuncts when v is a string column, while the equivalent form written with OR was simplified to v = 'b'. Two mechanisms can perform a rewrite. Multiple equalities handle it when check_simple_equality() builds an Item_equal, which it does only if the field's charset allows constant propagation. Up through 10.5 the default character set was latin1 whose collation handler supports constant propagation. MDEV-19123 made utf8mb4 the default in 11.6, and the utf8 collation handlers report that they do not support constant propagation. The other mechanism is propagate_cond_constants(), which rewrote the OR form under every collation. It descends through change_cond_ref_to_const(), which returns on any node whose eq_cmp_result() is COND_OK. Item_func_in inherits that value, so the IN predicate was skipped. Implement an optimization in change_cond_ref_to_const() that replaces the predicant of an IN predicate with the constant from an equality at the same AND level. The predicant is compared against every value of the list, so the existing per-operand test from MDEV-7152 is applied once for each of them. Only a predicant whose arguments were all aggregated to one comparison data type is replaced. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Sergei Golubchik
serg@mariadb.org |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-40608 MariaDB-devel is incomplete for plugins This works on Linux and on Windows, with rpm/deb/tar.gz/zip installations. For rpm/deb it just works, for tar.gz/zip there is no standard location, so one needs to configure plugin with -DCMAKE_PREFIX_PATH=/pah/to/mariadb/basedir after that, `cmake --install .` works too, installing in the same basedir. `cmake --build . --target package` works, creating rpm/deb/targz/zip depending on whether it's Linux or Windows and whether -DRPM or -DDEB was specified. * create and install mariadb-plugin-config.cmake * for now it only supports one plugin per project, error out if there are many * deb: move all headers that plugins need to libmariadb-dev, together with libmysqlservices.a. At least until we'll create mariadb-plugin-dev. Nobody should need huge libmariadbd-dev to develop a plugin * rpm: all in MariaDB-devel already, no changes here * install wsrep headers too, THD layout depends on WITH_WSREP * show DBUG_OFF, ENABLED_DEBUG_SYNC, and SAFE_MUTEX to plugins, same reason (it doesn't happen automatically as they're not in my_config.h) * but don't install config.h - high chance of name conflict with other projects and it's an exact copy of my_config.h anyway. * adjust plugin.cmake to work for external plugins * move server-internal part of it to top-level CMakeLists.txt * remove double-defined macros from unireg.h (the guard doesn't help if unireg.h is included first) * package plugin metadata as yaml in .tar.gz/.zip ColumnStore, until fixed, needs a backward-compatibility workaround |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Sergei Golubchik
serg@mariadb.org |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
fix errmsg-utf8.txt dependencies for Ninja generator GenError's custom command must specify headers as OUTPUT, otherwise ninja cannot deduce that mysqld.cc depends on errmsg-utf8.txt As a bonus, BYPRODUCTS lists generated files for `ninja clean` |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Rex Johnston
rex.johnston@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-38801 implement Item_cache_year shallow_copy() Implement shallow_copy() for Item_cache_year, same as for Item_cache_bool to avoid typeid mismatch from inheriting this method from Item_cache_int. Triggered when creating a clone of a condition for pushdown into a derived table. (Testcase amended by Sergei Petrunia) |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Thirunarayanan Balathandayuthapani
thiru@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-19574: innodb_stats_method is not honored when innodb_stats_persistent=ON Problem: ======= When persistent statistics are enabled (innodb_stats_persistent=ON), the innodb_stats_method setting is not properly utilized during statistics calculation. The statistics collection functions always use a hardcoded default behavior for NULL value comparison instead of respecting the configured stats method. This affects the accuracy of n_diff_key_vals (distinct key count), particularly for indexes with nullable columns containing NULL values. Moreover, stat_n_non_null_key_vals[] was never computed for persistent statistics; it stayed at the 0 that dict_stats_empty_index() assigns. With innodb_stats_method=nulls_ignored, innodb_rec_per_key() therefore always found n_diff <= n_null and reported one record per key for every index. This impacts the query optimizer, which makes decisions based on inaccurate cardinality estimates. Solution: ======== Introduced IndexLevelStats to collect statistics at a specific B-tree level during index analysis. Introduced PageStats to collect statistics for leaf page analysis. Refactored the following functions: dict_stats_analyze_index_level() to IndexLevelStats::analyze_level() dict_stats_analyze_index_for_n_prefix() to IndexLevelStats::sample_leaf_pages() dict_stats_analyze_index_below_cur() to PageStats::scan_below() dict_stats_scan_page() to PageStats::scan() The innodb_stats_method value is read once per table in dict_stats_update_persistent() and passed down, so that all indexes of a table are analyzed with the same method. Add the stats method name to stat_description when innodb_stats_method has a non-default value. The suffix is dropped when the description is already full. Added the new stat name n_nonnull_fld01, n_nonnull_fld02, etc. with a stats description, to indicate how many non-null values exist for the nth field of the index. This value is retrieved and stored in the index statistics in dict_stats_fetch_index_stats_step(). The counts are per column, not per n-column prefix. rec_get_n_blob_pages(): Calculate the number of externally stored pages for a record, using ceiling division by the usable BLOB page payload (blob_part_size), which differs between ROW_FORMAT=COMPRESSED (zip_size minus FIL_PAGE_DATA) and the other formats (srv_page_size minus the BLOB header and the page trailer). For ROW_FORMAT=COMPRESSED the length in the field reference is the uncompressed length, so the result is an upper bound. When the leaf level is scanned in full, the number of leaf pages that were scanned is reported as n_leaf_pages for a multi level index. Before, result.n_leaf_pages was overwritten with index->stat_n_leaf_pages, which dict_stats_empty_index() had just set to 1, so every index that took the full scan path reported n_leaf_pages=1. Single page indexes report 1. This changes cardinality estimates and therefore leads to multiple changes in existing test cases. Non-null values are counted only at the leaf level, since only leaf pages hold actual records. A full scan of the leaf level counts them exactly. When the level is sampled, the per column count is derived from the sampled leaves with the same formula as n_diff: n_ordinary_leaf_pages * n_non_null_all_analyzed_pages / n_leaf_pages_to_analyze This is an estimate for NOT NULL columns as well: the sampled leaves may hold fewer or more records than the average, and a dive that stops at a boring page contributes nothing to the sum while still counting in the divisor. innodb_rec_per_key(): stat_n_non_null_key_vals[i] holds the number of records in which the i-th indexed column alone is not NULL, while what has to be excluded here is the number of records whose first i+1 columns are all not NULL, because that is the population which the n-column prefix statistic stat_n_diff_key_vals[i] has to be corrected against when innodb_stats_method=nulls_ignored: with NULLs compared as unequal, every record carrying a NULL anywhere in the prefix adds a distinct value of its own to n_diff. PageStats::scan(): n_non_null is accumulated and assigned only for leaf pages, so that a non-leaf scan cannot leave a node pointer count behind when scan_below() stops at a boring page without reaching a leaf. IndexLevelStats::reset_for_level() also clears n_diff[], and dict_stats_analyze_index() zero initializes the buffer backing it, so that a level scan which finds no records (a failed btr_pcur_open_level(), or a non-leaf page whose first record is not marked as the leftmost one on the level) leaves n_diff[] at 0 instead of stale values. IndexLevelStats::sample_leaf_pages() returns early when the group boundaries for the prefix are empty, which is the same condition. IndexLevelStats::analyze_level(): Instead of copying the last record of the page, retain the latch on the page until the record has been compared with the first record of the next page dict_stats_fetch_index_stats_step() no longer resets stat_n_non_null_key_vals[] while processing an n_diff_pfxNN row: dict_stats_empty_table() has already cleared the array before the fetch, and with n_nonnull_fldNN rows now being read too, that reset would make the result depend on the order in which the rows arrive. dict_stats_save(): now static function in dict0stats.cc that takes the innodb_stats_method value, and is removed from dict0stats.h. dict_stats_update_persistent() saves the statistics itself, so its callers no longer have to. Replaced btr_rec_get_externally_stored_len() with rec_get_n_blob_pages() in dict0stats.cc. btr_rec_get_field_ref_offs() and btr_rec_get_field_ref(), together with the BTR_BLOB_HDR_* macros, were moved from btr0cur.cc to btr0cur.h so that rec_get_n_blob_pages() can reuse them; btr_rec_get_field_ref_offs() is now a noexcept function returning size_t. Changed stat_n_diff_key_vals and stat_n_non_null_key_vals from ib_uint64_t* to uint64_t* len_is_stored(): simplified to a single comparison, which is equivalent for the unsigned lengths that it is used with. Removed the unused UNIV_STATS_DEBUG build macro (univ.i) and turned the DEBUG_PRINTF() helper in dict0stats.cc into an unconditional no-op |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
bsrikanth-mariadb
srikanth.bondalapati@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-40553 Print GIS ranges in optimizer trace and context Ranges built over GIS (geometry) columns could not be printed in the optimizer trace or the recorded optimizer context: Field_geom printed every key value as the placeholder "unprintable_geometry_value", regardless of whether the index stored the column's raw value (or a prefix of it) or, for a SPATIAL index, its MBR (Minimum Bounding Rectangle). Field::print_key_part_value() now takes an image_type argument (see Field::image_type()) that says which of the two the key holds. For a SPATIAL index (image_type itMBR), Field_geom::print_key_part_value() decodes the four doubles the key stores and prints them as a WKT POLYGON. For every other index (image_type itRAW), key values print as before, in binary form, the same way Field_blob already does. print_mbr_range_operator() prints the spatial relation a GEOM range carries (MBRWITHIN, MBRCONTAINS, MBRINTERSECTS, MBRDISJOINT, MBREQUALS), inverted where needed so the indexed column reads on the left; print_range() and print_key_value() thread the new image_type argument through to reach it. Writing a test for this surfaced two more bugs in the code that prints/replays the optimizer context; both are fixed here, since a test for either would otherwise fail for reasons unrelated to GIS: - Single_line_formatting_helper::disable_and_flush() (my_json_writer.cc) escaped its buffered values a second time, via add_str() instead of add_escaped_str(). This corrupted any JSON string long enough to make the writer fall back from single-line formatting -- GIS range lists among them. - The context literal that dump_sql_script() writes into the recorded replay script (opt_context_store_replay.cc) escaped backslashes SQL-style. That does not round-trip through INFORMATION_SCHEMA.OPTIMIZER_CONTEXT's regexp-based extraction the same way running the recorded script does, so a context extracted that way no longer matched the ranges the optimizer prints. The literal is now written with NO_BACKSLASH_ESCAPES in effect instead, so its text is identical to the JSON it carries; a single quote is written as its JSON escape \u0027, since NO_BACKSLASH_ESCAPES leaves it as the only character that could still end the literal early. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Thirunarayanan Balathandayuthapani
thiru@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-41055 innodb_encryption_threads=0 hangs indefinitely when rotation IOPS is zero Problem: ======= When innodb_encryption_rotation_iops=0, an encryption thread could be waiting on fil_crypt_iops_cond in fil_crypt_alloc_iops(). fil_crypt_set_thread_cnt() lowers srv_n_fil_crypt_threads and broadcasts only fil_crypt_thread_cond, so that the waiting thread never re-evaluates should_shutdown() and never exits. Solution: ========= fil_crypt_set_thread_cnt(): Broadcast fil_crypt_iops_cond as well, so a thread waiting for IOPS wakes up and sees should_shutdown(), exits. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Rucha Deodhar
rucha.deodhar@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-39049: Memory corruption & crash in check_key_in_list upon using JSON_KEYS after modifying character set name/collation Analysis: Since the length of string is 0, accessing out of boundry memory, leads to crash Fix: If string length is empty, return success. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
DerZc
34330257+DerZc@users.noreply.github.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-40479 SELECT, order by desc, on partitioned table leads to incorrect results A descending scan of a partitioned table through a secondary index can stop early and omit qualifying rows. ha_partition::handle_unordered_prev() validates the unordered prefix with key number 0 instead of the active index. If the secondary index has a different layout from the primary key, that comparison can incorrectly signal end of file. Pass active_index to key_cmp_if_same() so the prefix comparison uses the index that produced the current row. The regression scans a RANGE-partitioned table using a secondary (b,c) index and checks that ORDER BY c DESC returns all three values: 15, 11, and 6. Bug report: https://jira.mariadb.org/browse/MDEV-40479 |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Rex Johnston
rex.johnston@mariadb.com |
|
|
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
MDEV-41228 Test deep clones of Item_cache items Deep clones of Item items are created in very few places, so most of the clone code has no test coverage at all, although Parallel Query relies on it heavily. One of the places where clones are created is the generation of the key parts for a lookup into a split materialized table, in TABLE::add_splitting_info_for_key_field(). An Item_cache reaches that place through the IN->EXISTS transformation. Item_in_optimizer::fix_left() wraps the left expression of the predicate in an Item_cache, and the equality injected into the subquery refers to it, so the key field value being cloned is an Item_direct_ref over that cache. Two more things are needed for the clone to happen: the grouping field the equality matches has to be the first component of some index of the underlying table, otherwise it is not among spl_opt_info->spl_fields and the function returns before cloning, and the subquery must not be converted to a semi-join. The latter is achieved by the shape of the query, a UNION in the subquery, rather than by turning optimizer switches off, so that the plan is the one a user gets with a default optimizer_switch. Add Item::check_deep_copy(), which validates a clone against its original in a debug build. It walks both item trees and reports, as notes, whether they have the same shape with the same Item class at every node, and whether the clone shares an Item object with the original. Sharing is what distinguishes a shallow copy from a deep one: an item that is shallow by design, Item_field for instance, still produces a separate object and only shares a Field, which is not an Item. Call it from TABLE::add_splitting_info_for_key_field() under the "split_materialized_clones" debug flag, which additionally makes the optimizer use a clone as the value of the generated key part. The clone then has to work both in the condition pushed into the materialized table and as the value looked up in the filled table. Note that the clone built for the pushed condition cannot be reused for this, as it has already been made dependent on the select that specifies the materialized table. With the check in place, the clone of the cache turned out not to be a deep one: every Item_cache_* class implemented deep_copy() as a plain shallow copy. That is wrong beyond sharing the example item. A cache is filled by the store()/cache_value() calls of the item that owns it, Item_in_optimizer here, and nobody does that for a clone, so a clone that inherited the cached value of the original kept returning that value for the rest of the query. Implement Item_cache::deep_copy() once for all cache classes instead. The clone is given an empty cache, so that it computes the value itself out of the item the value is read from, and a copy of that item. The exception is an example containing an aggregate or a window function: those are not clonable yet, as a copy of one shares the per-execution data of the original and both would free it, so such an example is shared and the check reports the clone as not fully deep. Item_cache_row is not clonable at all now, as a copy of it shared the values[] array of element caches with the original. The test uses a single row in the outer table, because the answer to the same query with more rows is wrong for an unrelated reason, MDEV-41251: a split materialized table in a dependently executed subquery is never refilled when the outer row changes. Added a force clone in the Item_cache type handler, set debug flag item_cache_clones and any Item_cache class will return a clone (if possible) newly created by deep_copy_with_checks(); |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||