This merges the patch series that fixes a lot of missing validation
checks for read_log_event(). The merge includes a few added fixes that
are needed in new code introduced in 13.1.
Signed-off-by: Kristian Nielsen <knielsen@knielsen-hq.org>
This merges the patch series that fixes a lot of missing validation
checks for read_log_event(). The merge includes a few added fixes that
are needed in new code introduced in 12.3.
Signed-off-by: Kristian Nielsen <knielsen@knielsen-hq.org>
This merges the patch series that fixes a lot of missing validation
checks for read_log_event(). The merge includes a few added fixes that
are needed in new code introduced in 11.8.
Signed-off-by: Kristian Nielsen <knielsen@knielsen-hq.org>
cover_definer_clause() wrapped DEFINER in "*/ /*!50017 ... */" and broke
when the statement text contained a comment. Dump the server's DDL
as returned.
Also:
- drop /*!NNNNN */ gates below 50700 in mysqldump, mysqlbinlog,
mysqladmin, mysqlcheck, mysqlshow, mysqlimport, mysqlhotcopy,
innotop and mtr_warnings.sql; mysqlbinlog's skip_replication
becomes /*M! ...*/
- SHOW CREATE DATABASE no longer wraps IF NOT EXISTS and DEFAULT
CHARACTER SET in comments
- remove checks for servers older than 5.7 in mysqldump, mysqlcheck,
mysql, innotop and the client library, including dump_trigger_old()
and mysqlcheck's SHOW TABLES fallback
- mysqldump warns on stderr if SHOW CREATE DATABASE fails
Behavior changes:
- --compatible=mysql323 and mysql40 are accepted and ignored. The other
modes wrap only some SET and ALTER TABLE ... KEYS statements in
/*!40101 */.
- mysqlhotcopy --old_server is accepted, ignored, and prints a warning.
Assisted-By: Claude:claude-5.5-sonnet
A locking read with SKIP LOCKED through a secondary index calls
row_sel_get_clust_rec_for_mysql() to lock the clustered index record.
On a lock conflict, that function returned DB_LOCK_WAIT without
setting the output record, and row_search_mvcc() then called
lock_trx_handle_wait() to cancel the wait. If the conflicting
transaction had committed in the meantime, the waiting lock was
already granted and lock_trx_handle_wait() returned DB_SUCCESS.
row_search_mvcc() then found clust_rec == NULL for a locking read:
debug builds failed an assertion, and release builds skipped a row
whose lock the transaction now held.
Row_sel_get_clust_rec_for_mysql::operator()(): Call
lock_trx_handle_wait() here instead of in row_search_mvcc(). When
the lock was granted meanwhile, return the clustered index record
and store the cursor position as for any granted lock, like
row_search_mvcc() does with "goto locks_ok" for the record that it
is positioned on.
DuckDB has no row locks and its parser rejects locking clauses, so
SELECT ... FOR UPDATE on a DuckDB table failed with ER_GET_ERRMSG
("Parser Error: SELECT locking clause is not supported!") coming from
the embedded DuckDB.
Check for locking reads in ha_duckdb::external_lock() and raise
ER_NOT_SUPPORTED_YET from the storage engine instead. The check only
applies to SQLCOM_SELECT, so INSERT ... SELECT, UPDATE and DELETE are
not affected.
Without passing through the compile and linker flags, options
like -stdlib=libc++ can create ABI differences between ducksdb
extensions and what the server links too.
The jemalloc has C files so CMAKE_C_FLAGS is passed through too.
The warnings were in RocksDB (not MyRocks) and looked like this:
rocksdb/db/db_impl/db_impl.cc: In member function ‘virtual rocksdb::Status
rocksdb::DBImpl::FlushWAL(bool)’:
rocksdb/db/db_impl/db_impl.cc:1329:23: warning: redundant move in return
statement [-Wredundant-move]
1329 | return std::move(io_s);
| ~~~~~~~~~^~~~~~
GetJVM() only looked for %JAVA_HOME%\bin\client\jvm.dll, and for the
RuntimeLib registry value of the "Java Runtime Environment" key. Since
JDK 9 there is no "client" JVM, jvm.dll is in bin\server, and the JDK
installers write JavaHome, but not RuntimeLib, under JavaSoft\JDK.
Look for jvm.dll the way Java installations are laid out. Prefer
bin\server over bin\client, in the JDK and in its jre subdirectory.
Without connect_jvm_path (which stays exclusive, as before), try
JAVA_HOME first, then the JavaSoft\{JDK|JRE} registry keys:
CurrentVersion, then the RuntimeLib or JavaHome value of that version.
Keep the first meaningful LoadLibrary or registry error (e.g. a 32/64
bit mismatch) rather than the "not found" of the last probe, reject
over-long paths instead of truncating them, accept REG_EXPAND_SZ
values, ignore an empty JAVA_HOME, and remove a stray printf to stdout.
Tested manually with Oracle JDK 23: with JAVA_HOME unset (registry),
with JAVA_HOME set, and with an invalid JAVA_HOME falling back to the
registry.
Assisted-By: Claude:claude-5.5-sonnet
A locking read on a SPATIAL INDEX collects the matching records into a
shadow block matched_rec_t::block. sel_set_rtr_rec_lock() holds page.lock
on that block while it locks the records, because lock_rec_lock() asserts
that the block of the record being locked is latched. If a lock request
has to wait, the records are collected again after the wait, and
rtr_init_match() re-initializes the block by buf_page_t::init(), which
asserts that the block is not latched.
The assertion is about a latch that would be a leftover from the previous
identity of the block. It does not apply to rtr_init_match(), where the
block is in buf_page_t::MEMORY state, private to the current thread, which
is holding page.lock on it on purpose.
Changes:
- buf_page_t::init(): Tolerate a latch that the current thread is holding
on a block that is being initialized in buf_page_t::MEMORY state.
When a select containing 32 ranges is made on a table containing a
compound key with 32 parts, the range optimizer can run off the end
of a stack variable, invalidly overwriting subsequent stack variables.
In the struct st_sel_arg_range_seq, we have an array
RANGE_SEQ_ENTRY stack[MAX_REF_PARTS];
MAX_REF_PARTS is 32.
check_quick_select
/ sel_arg_range_seq_init
initialises stack[0] as NOT a key part
/ sel_arg_range_seq_next
iterates through the key parts, adding key part n to stack[n+1]
key part #32 gets referenced by step_down_to(), setting seq->i off the
end of the array.
Fix: RANGE_SEQ_ENTRY stack[MAX_REF_PARTS+1];
The above change exposed an issue with key length calculation on
MS Windows. Calling make_prev_keypart_map(32) caused the resultant
bitmap to be calculated as (1UL << 32) - 1. Using the MSVC compiler
this resulted in an empty key length calculation during
handler::index_read_map, causing an assertion in ha_innobase::index_read().
As we only need 32 bits to represent our key map, we change the type thus
-typedef ulong key_part_map;
+typedef uint32 key_part_map;
We correct make_keypart_map() and make_prev_keypart_map() to call our
overflow safe my_set_bits(). We also correct bka_range_seq_next()
and bkah_range_seq_next() to use make_prev_keypart_map().
We also add some DBUG_ASSERTS in key_part_map processing elsewhere,
exposing some issues in our BNLH implementation. We cap the number of
keyuse parts here, altering the explain output of 2 of our tests.
Numerous places needed bit shift operations altered to use
make*keymap_part and various format strings needed to be corrected.
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 until a sweep reaches it, which is
usually after it has moved 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.
The helpers that set or clear one bit of page_zip_des_t::state always
perform the atomic read-modify-write, which is a full barrier on x86.
A caller that can find the bit already in the target state checks it
first: touch_no_stamp() for PROMOTE, buf_LRU_add_block() for OLD, and
page_zip_write_rec() for NONEMPTY. page_zip_des_t::copy_from() and
set_n_blobs_and_empty() skip the fetch_add() when the delta is 0.
page_zip_decompress_clust() counts the BLOB pointers locally and
invokes add_n_blobs() once.
page_zip_des_t::copy_from(): Replaces the copy constructor that
page_zip_copy_recs() invoked. It copies m_end, NONEMPTY and n_blobs
from the source page, but preserves the PROMOTE and OLD flags of the
destination block, because they describe the position of that block
in buf_pool.LRU. Copying OLD from the source would make
buf_pool.LRU_old_len inconsistent with the list. Like
set_n_blobs_and_empty(), it updates the state with a single
fetch_add(), which preserves any concurrent change of PROMOTE or OLD.
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_block_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.
PageBulk::m_modify_clock: Remove. It was assigned but never read.
buf_page_t::relocate_from(): Copies a descriptor that is relocated
to this block, in buf_relocate() and buf_pool_t::shrink(), but keeps
the frame and the modify_clock() of this block. The copy constructor
does not copy the clock: it belongs to the block descriptor and must
only grow during its lifetime, as buf_block_t::modify_clock did.
Otherwise, the clock of this block could return to a value that a
btr_pcur_t saved before the block was freed, and
btr_pcur_t::restore_position() could reuse a record pointer although
the page was modified in another block in the meantime.
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. Because
buf_pool.access_clock advances once per refresh_clock(), about once per
second, a difference of threshold ticks can span less than
innodb_old_blocks_time; the difference must exceed the threshold, so
that a burst of accesses that crosses a refresh is not promoted. 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 PROMOTE is set, 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. It reads the state of
the block once: buf_pool.mutex, which the caller holds, protects OLD,
and only make_young() clears PROMOTE. The block need not be in
buf_pool.LRU_old: while buf_pool.LRU is shorter than
BUF_LRU_OLD_MIN_LEN, buf_pool.LRU_old does not exist, and blocks are
evicted without ever becoming old. The template parameter only_old
leaves a block that is not old in place; buf_do_flush_list_batch()
uses it, because it visits blocks in the order of oldest_modification,
and moving a block that is not old would not save it from eviction.
The template parameter count_not_young selects whether an old block
that has an accessed_at() stamp but is not flagged is counted in
buf_pool.stat.n_pages_not_made_young. This includes a block that had
no access since its last promotion, so the count is of sweep visits,
not of rejected accesses; 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. As before, buf_pool.stat.n_pages_made_young counts
only the moves of old blocks. Because a sweep moves the block, not the
access, and counts the blocks that it does not move,
Innodb_buffer_pool_pages_made_young and
Innodb_buffer_pool_pages_made_not_young can differ from before for the
same workload.
buf_page_t::make_young(): Clear PROMOTE, stamp access_time, and move
the block to the "recently used" end of buf_pool.LRU. 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.
buf_LRU_remove_block(): When buf_pool.LRU becomes too short for
buf_pool.LRU_old, clear the OLD flags from buf_pool.LRU_old to the end
of the list, which holds all the old blocks, not the flags of every
block. While buf_pool.LRU_old does not exist, no block in buf_pool.LRU
is old, so on later removals the loop does nothing.
buf_LRU_add_block(): Write the OLD flag only if it changes. In a list
that is too short for buf_pool.LRU_old, assert that buf_pool.LRU_old
is not defined and that the block is not old, instead of writing the
flag.
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 65534 seconds (UINT16_MAX - 1), because the age
uint16_t(now - access_time) cannot exceed 65535 and a larger threshold
would prevent any promotion. 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_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. The
index_page_splits counter confirms that the clustered index consists
of its root page alone, without depending on which pages remain in the
buffer pool.
Co-Authored-By: Alessandro Vetere <iminelink@gmail.com>
log_t::checkpoint_margin(): In the target LSN computation, account
for the special case resize_log.m_file==log.m_file which signals
that a checkpoint write is in progress. Assert that the target
checkpoint LSN is sensible.
Fixes up commit 3394a9d114 (MDEV-37949).
Reviewed by: Thirunarayanan Balathandayuthapani
recv_recovery_from_checkpoint_start(): Before applying any page-level
log records by invoking recv_scan_log(true, parser), invoke
recv_rename_files() to replay FILE_RENAME records. In this way,
recv_sys_t::recover_deferred() will not overwrite any old data files
that had been renamed before a new file was created.
recv_sys_t::validate_checkpoint(): Refer to recv_sys.scanned_lsn
rather than recv_sys.lsn, because recv_scan_log(false, parser)
may reset the latter to rewound_lsn in case we run out of memory for
recv_sys.pages. In any case, the second call to recv_scan_log() will
proceed to the end of the log and parse all records.
Reviewed by: Thirunarayanan Balathandayuthapani
Failure:
- wsrep_rec_get_foreign_key() returned DB_ERROR when a write set key
exceeded WSREP_MAX_SUPPORTED_KEY_LENGTH; the caller does not know
DB_ERROR and calls ib::fatal().
- wsrep_fk_key_space_left() needed two bytes for a NULL column instead
of one; only the flag byte is ever written for NULL.
- Cutting a whole number, float or double to fit the remaining buffer
produces a different value, not a shorter one, and for a float or
double not even a valid one: mach_float_read()/mach_double_read()
read a fixed 4 or 8 bytes regardless, past an assertion in a debug
build and past the buffer itself in a release one.
- wsrep_store_key_val_for_row()'s equivalent fixed-length branch had
the same unchecked copy, reachable only in theory given InnoDB's own
index length limit, but inconsistent with every other branch in that
function.
- The same function normalized a VARCHAR column's collation weights up
to 3072 bytes while wsrep_rec_get_foreign_key() normalized it up to
3500, so the two produced a different key for a value whose
normalized form falls between the two limits.
- wsrep_store_key_val_for_row() kept iterating columns after one did
not fit, relying on every later branch clamping to zero rather than
stopping outright.
- MW-292 matched its two debug sync waiters by substring, not an exact
match, independent of wait order.
Fix:
- wsrep_rec_get_foreign_key() now cuts the key the same way
wsrep_store_key_val_for_row() does, reports the cut as a warning
(ER_TOO_LONG_KEY under strict mode), and returns DB_TOO_BIG_RECORD.
- wsrep_fk_key_space_left() takes a fixed_len flag: a whole number,
float or double must fit in full or is left out of the key entirely,
the same way a value that does not fit even one byte already was.
- wsrep_store_key_val_for_row() applies the same all-or-nothing rule to
its own fixed-length branch, reusing get_innobase_type_from_mysql_type()
so both functions agree on which types that covers, and now stops
storing further columns as soon as one does not fit, in every branch.
- wsrep_store_key_val_for_row()'s VARCHAR branch normalizes up to
WSREP_MAX_SUPPORTED_KEY_LENGTH, matching wsrep_rec_get_foreign_key(),
with its buffer grown to match.
Test:
- galera_ws_datatypes.test and galera_ws_datatypes_plugin.test cover
every MariaDB data type in a primary key, unique key, plain key and
foreign key (referencing a primary or unique key), including NULL
foreign key columns.
- galera_ws_datatypes_fixed_length.test covers a foreign key column cut
to fewer bytes than a whole number, float or double needs.
- galera_ws_datatypes_varchar_cap.test covers a VARCHAR foreign key
column whose normalized form falls between the two limits above.
Assisted-by: https://mariadb.org/governance/governance-ai-policy/
In dict_table_rename_in_cache(), sql_id = foreign->sql_id() points into
the existing foreign->id string. When the new id is not longer than the old
one (always the case when renaming from "#sql-alter-..." back to the real
table name), the buffer is reused in place, so the final
snprintf(id, fklen, "%s\377%s", table->name.m_name, sql_id)
reads sql_id from the buffer it is writing to (sql_id == id + 50 in gdb).
That is undefined behavior. Most libcs happen to copy forward and get away
with it, but OpenBSD's memcpy detects the overlap and calls abort().
The fix computes the lengths explicitly, memmove()s the sql_id tail to its
final position first, then memcpy()s the table name prefix and writes the
\377 separator. The decision to reuse the old buffer or allocate a new one
is unchanged.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
A multi-table DELETE on a FEDERATED or FederatedX table went wrong
when a primary key lookup made the target a const table. The
optimizer reads a const table's row through index_read_idx_map(),
whose default implementation ends the index scan and frees the result
set. The server asks for the row's position later, during execution,
so the saved position was empty. FederatedX skipped the row and
FEDERATED crashed in rnd_pos().
Both engines now override index_read_idx_map() so that the lookup
leaves its result set open, as index_read() does. position() then
records a valid position, and the result set is freed at the end of
the statement. FEDERATED's reset() now also clears stored_result,
which otherwise pointed at a freed result set and was freed again
when the table was closed.
Tests include a multitable UPDATE with a const target, which crashed
both engines before the fix.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
ha_federated::index_read_idx() and ha_federatedx::index_read_idx()
have no callers and do not override a handler method. Remove them,
and remove the sentence in the comment on each index_read() that says
index_read() calls it. Comments that describe index_read_idx() now
name index_read() or index_read_idx_map(), whichever does that read.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Several error-handling paths returned without releasing a resource
already acquired earlier in the function, or checked the wrong handle
entirely, risking use of an unopened handle.
Changes:
- SysTablespace::read_lsn_and_check_flags(): close the datafile handle
on header-validation failure.
- xb_process_datadir(): check the freshly opened `dir` handle instead
of the stale `dbdir`, fixing a handle leak and a possible use of an
unopened directory handle.
- wsrep.cc / xb_load_list_file(): close file handles before die(), and
null-check fopen() results in wsrep.cc.
- datadir_iter_new(): free datadir_path and destroy the mutex on the
os_file_opendir() failure path.
log_t::write_checkpoint(): Correct an off-by-one error in the assertion
expression. We may reach the very end of the current log file.
A subsequent log_t::write_buf() will invoke archive_new_write(),
which will create a new log file if needed.
Tested by: Saahil Alam
1. Aria/MyISAM engines
A system-versioned DELETE on a MyISAM or Aria table indexed by row_end
could leave the last current row undeleted. The delete scans the current
rows via an equality search on row_end = MAX, which is driven by
mi_rnext_same/maria_rnext_same. That function keeps the search's
reference key in lastkey2 and uses the HA_STATE_RNEXT_SAME flag to
remember it has already stored it. This reference-key mechanism is
exactly what lets the scan modify rows it is walking without skipping
them.
Deleting a versioned row is an in-place update of row_end, and the update
reuses lastkey2 as scratch space for the changed key, so it clears
HA_STATE_RNEXT_SAME to request that rnext_same re-store its reference on
the next call. However, TABLE::delete_row wraps the update in
HA_EXTRA_REMEMBER_POS/HA_EXTRA_RESTORE_POS, and RESTORE_POS restored the
whole saved info->update word, resurrecting the HA_STATE_RNEXT_SAME bit
that the update had just cleared.
As a result rnext_same skipped rebuilding its reference key and compared
subsequent keys against the now-overwritten lastkey2, hitting a spurious
end-of-file and terminating the scan one row early, defeating the engine's
own protection against a Halloween-style skip.
Fixed by preserving the current HA_STATE_RNEXT_SAME bit across
RESTORE_POS instead of restoring the stale saved value. See also the HEAP
fix below: same root cause, different per-engine mechanism.
2. HEAP engine
The row loss also reproduces on the MEMORY (HEAP) engine. A system-
versioned DELETE scans the current rows on the row_end index and turns
each delete into an in-place update of row_end, so it modifies the very
index it is walking.
When the changed key is the scanned one (info->lastinx), hp_delete_key()
repositions the cursor but heap_update() leaves info->update untouched, so
HA_STATE_NEXT_FOUND from the preceding heap_rnext() stays set. The next
heap_rnext() then sees current_ptr == 0 with that bit and takes the
"!current_ptr && HA_STATE_NEXT_FOUND" guard as a false end-of-file,
stopping one row early.
Fixed by clearing HA_STATE_NEXT_FOUND when the scanned index key changed.
HA_STATE_AKTIV is kept (unlike heap_delete): the row is updated, not
removed, so a following op must not fail test_active(). The bit is only
set after a heap_rnext(), so a plain single-row UPDATE never reaches this.
See also the Aria/MyISAM fix above: same root cause, different per-engine
mechanism.
3. Why the fix differs per engine, and InnoDB
Aria and HEAP both trace their handler design back to MyISAM, hence the
same root cause (stale scan bookkeeping after an in-place key change) in
all three, fixed at each engine's own bookkeeping spot. InnoDB needs no
fix: its persistent cursor survives concurrent index modification by
design, already covered by this same test under the timestamp
combination (default-storage-engine=innodb), which passes unmodified.
When rebuilding a table from ROW_FORMAT=COMPACT or DYNAMIC into
ROW_FORMAT=REDUNDANT, row_merge_buf_add() fetches the full value of an
externally stored (off-page) CHAR column in a multi-byte character set
and pads it to REDUNDANT's fixed local width via
row_merge_buf_redundant_convert(). That helper already dereferences the
BLOB and calls dfield_set_data(), which clears the field's "externally
stored" flag, since the value is now held in full locally.
The "flag externally stored fields" step further down in
row_merge_buf_add() did not know this had happened. It still consulted
the row_ext_t cache built from the original (pre-conversion) record and,
for a column that is not part of the clustered index's unique key,
called dfield_set_ext() again on the very field that had just been
converted, without restoring its data pointer to a valid 20-byte
external reference. row_merge_copy_blobs() would then read the tail of
the padded, space-filled buffer as if it were a BTR_EXTERN_FIELD_REF,
deriving a garbage tablespace id and crashing buf_page_get_gen()'s
fil_space_get() assertion when the alter tried to build the new
clustered index.
Skip the re-flagging step for a field whose "externally stored" flag is
no longer set. row_build() flags every off-page column, and the
row_ext_t cache only holds a subset of those columns, so a field that
is not flagged is either a converted one (already fully local) or one
that the cache does not hold.
With the field no longer re-flagged, the rebuild completes, and the
rebuilt table passes CHECK TABLE with the full column value.
The MDEV-31025 case in innodb.default_row_format_alter failed on
innodb_page_size=4k and 8k: its ROW_FORMAT=REDUNDANT table has eight
utf32 CHAR(255) columns, which CREATE TABLE rejects with
ER_TOO_BIG_ROWSIZE on those page sizes. Derive the number of columns
from the page size, so that the record still exceeds the maximum local
record size and the fixed-length column c is stored externally. The
whole test now passes on every page size.
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
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.
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
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.
