Problem:
CREATE TABLE ... SELECT ... FOR UPDATE involving a MyISAM temporary table can trigger an assertion in close_thread_tables() when binary logging is enabled in MIXED mode.
Cause:
MyISAM temporary tables do not support row-level locking, so SELECT ... FOR UPDATE acquires a write lock even when the access is read-only. This causes the table to be incorrectly classified as a write operation, preventing binlog_truncate_trx_cache() from running and eventually triggering the assertion.
Fix:
Update decide_logging_format() to use tbl->updating to distinguish actual table modifications from read-only accesses. For read-only access to temporary non-transactional tables, mark the statement as STMT_READS_TEMP_NON_TRANS_TABLE instead of treating it as a write operation. This prevents incorrect write classification and avoids the assertion failure.
When the variable "use_stat_tables" is set to 'preferably', the explain
plan query containing a stored procedure generated no context, even when
the optimizer_record_context flag was set.
The reason being context recorder (for the entire query) was getting cleared
after the stored procedure evaluation was done.
The solution is to not clear the context recorder as soon as the stored
procedure evaluation is finished.
This task loads the stats of the tables that are used in a query,
from the trace into the optimizer. This feature is controlled
by optimizer_replay_context, and points to an user defined variable name,
that has the context information.
The stats such as num_of_records present in the table, indexes if present,
their names, along with the average number of records_per_key
with in each index, cost of reading an index are loaded from the trace.
Additionally, stats from range analysis i.e. ranges, and the
corresponding number of records, along with their costs are also
loaded from the trace.
The trace context which is in JSON format is firstly set into the
user defined session variable, and this variable name is set to
optimizer_replay_context system variable.
Later, when a user issues a query, the contents of the json context is
parsed and an in memory representation is built using the class
Optimizer_context_replay. This class is then used by the optimizer
to update and save original values of the tables, indexes, and range stats
in the methods "set_statistics_for_table()" of sql_statistics.cc, and
"check_quick_select()" of opt_range.cc.
After the query gets finished, the statistics that were updated
in the optimizer are restored back to the saved original values.
The entry point for parsing the json structure is in
"mysql_execute_command()" of sql_parse.cc, and similarly exit point
i.e. to restore the saved stats of the optimizer is at the end
of the same method.
Two new warning/error messages are introduced:
ER_JSON_OPTIMIZER_REPLAY_CONTEXT_PARSE_FAILED, and
ER_JSON_OPTIMIZER_REPLAY_CONTEXT_MATCH_FAILED
This feature stores the basic stats of the base tables that are used in
a query, to the optimizer trace. This feature is also controlled by
optimizer_record_context, and is not enabled by default. The stats
such as num_of_records present in the table, indexes if present
then their names, along with the average number of records_per_key
with in each index are dumped to the trace. Additionally, stats from
range analysis of the queries are also dumped into the trace
The approach taken here is to extend the existing function
store_tables_context_in_trace() and add new dump_table_stats_to_trace()
to opt_trace_ddl_info.cc. For storing the range analysis stats into the
trace, they are first recorded in the hash defined in
Optimizer_Stats_Context_Recorder, and is used later while dumping the
stats into the trace from dump_table_stats_to_trace().
Several new tests are also added to opt_trace_store_stats.test
The "is it my own thread" check compared the login user name
(Security_context::user) and ignored the host, so u1@localhost could see
and kill threads of u1@'127.0.0.1'.
Compare the authenticated account instead:
* all comparisons are done in sctx->is_priv_user() now
* change user_matches() to priv_user_matches(), which uses is_priv_user()
* use it in KILL, KILL USER, SHOW PROCESSLIST, I_S.PROCESSLIST,
COM_PROCESS_INFO and SHOW EXPLAIN/ANALYZE FOR.
* all the remaining places use is_priv_user() directly instead
of doing strcmp: SHOW GRANTS, SHOW CREATE PROCEDURE, I_S.VIEWS,
the DEFINER clause, optimizer trace and change_security_context().
Assisted-by: Claude:claude-5-opus
user_var_entry objects and their names have a connection lifetime,
they exist until the connection ends (or is reset), and then they're
all deleted at once. This is exactly the use case for MEM_ROOT,
let's store them there.
Additionally, let's set MY_ROOT_USE_VMEM flag to keep this memroot
off the general heap where user_var_entry values are stored
and where heap buffer overflows can happen.
The latter makes memory allocations for the MEM_ROOT about 10x more
expensive, so let's always start with an empty memroot (= zero overhead
if no user variables are used) and on THD cleanup let's retain one
memroot block (= zero overhead if the next connection takes THD from the
cache and uses user variables up to one block size).
Assisted-By: Claude:claude-4.8-opus
Do not put any virtual column values into the before-image or after-image of
row events.
These virtual column values were causing assertion and probably other issues
as well. Concretely, a UNIQUE index on (int_col, blob_col) where the int_col
is being updated would include the hidden virtual column for the index in
the after-image, without including the value of the blob_col in
binlog_row_image=MINIMAL. This would cause an assertion during
TABLE::update_virtual_fields() due to missing bit in the bitmap.
Even if it doesn't cause an assertion, including the virtual column values
makes no sense, as they are recomputed anyway on the slave when applying the
row events.
Reviewed-by: Monty <monty@mariadb.org>
Brandon Nesterenko <brandon.nesterenko@mariadb.com>
Signed-off-by: Kristian Nielsen <knielsen@knielsen-hq.org>
- Change thd->backup_commit_lock from MDL_request* to MDL_ticket*,
and convert related call sites in handler.cc, log.cc, sql_class.cc,
and xa.cc to use MDL_ACQUIRE_LOCK.
- Convert reload_acl_and_cache() in sql_reload.cc, purge_master_logs()
and reset_master() in sql_repl.cc to MDL_ACQUIRE_LOCK, holding the
ticket in a local MDL_ticket* (these functions used a local
MDL_request originally and should not touch thd->backup_commit_lock).
- Convert acquire_lock() in partition_info.cc to MDL_ACQUIRE_LOCK.
- Add MDL_REQUEST_LIST_ADD() helper for enqueuing lock requests into
MDL_request_list, and convert call sites in sql_base.cc and sp.cc.
All new code of the whole pull request, including one or several files
that are either new files or modified ones, are contributed under the
BSD-new license. I am contributing on behalf of my employer Amazon Web
Services, Inc.
This issue is regression caused by MDEV-38019 Send ok packet
to client earlier.
BF aborts may be detected very late in command exectution,
somewhere near the end of do_command(). If the ok packet has
already been sent and BF abort is detected very late, this
could cause the protocol packets becoming out of sync.
This is a result of reverting order of releasing MDL and
sending ok packet to the client. If it happens in the
opposite order than before, there will remain some time
window where the select is complete and MDL locks still
held. DROP hits that time window, causing a BF abort,
and extra error packet gets injected into client-server
communication stream.
Fixed by not sending ok packet to client earlier
in wsrep case.
There is a repeatable test case that shows failure
if ok packet is sent client earlier by using
debug sync point. However, this fix skips sending ok
packet earlier causing also skipping the sync
point. Therefore, test case is not usable.
Test case galera_sr.mysql-wsrep-features#8 was failing
because of this regression but it is sporadic.
Co-Authored by : Teemu Ollakka <teemu.ollakka@mariadb.com>
The patch introduces the OLD_VALUE() expression to reference the value
of a column before it was updated. The parser is extended to support
RETURNING and OLD_VALUE(), and RETURNING expressions are stored in a
separate returning_list in SELECT_LEX with independent wildcard tracking.
RETURNING is rejected for multi-table UPDATE.
During setup of RETURNING fields, THD::is_setting_returning is used
when resolving fields, particularly for updates through views.
When resolving view fields, Item_direct_view_ref may point to the
view's item_list, losing the information about whether the value
should be old or new. The original item in returning_list still
contains the correct is_old_value_reference flag, which is copied
back to the resolved item.
OLD_VALUE() is implemented by extending Item_field with a new
Item_old_field class. Item_field::set_field() initializes
Field::ptr_old to the corresponding location in record[1],
which stores the old row during UPDATE execution.
When sending result rows, Item_old_field::send() temporarily switches
the field pointer from record[0] (current row) to ptr_old so
OLD_VALUE() returns the value before the update.
The UPDATE execution path is modified to send a result set when
RETURNING is present instead of an OK packet.
Problem:
========
Purge threads computing virtual columns could crash due to:
1. Stale TABLE* pointers when tables are flushed/rebuilt during purge
2. open_purge_table() called close_thread_tables() on failure, making
MDL tickets invalid before purge could release them
3. Purge coordinator opened TABLE* but workers accessed it with wrong
TABLE->in_use
4. No retry mechanism when open_purge_table() failed due to concurrent
FLUSH TABLES, BACKUP STAGE, or ALTER TABLE operations
Solution:
========
1. Removed close_thread_tables() from open_purge_table(). Purge
coordinator thread should close explicitly in close_and_reopen()
2. Added retry logic: when open_purge_table() returns NULL due to
table flush/rebuild, set must_wait() flag and retry in
close_and_reopen()
3. Update close_and_reopen() with purge_table parameter to
close the failed table. Pass it to trx_purge_close_tables()
for proper cleanup
4. Properly set and reset TABLE::in_use during purge operations:
- Set to coordinator_thd in row_purge_parse_undo_rec() when opening
- Reset in trx_purge_close_table() when closing
5. The auto_increment initialization now happens unconditionally for
purge threads , ensuring the auto_increment counter is always
properly initialized when purge opens tables with virtual columns
Problem:
========
A single InnoDB purge worker thread can process undo logs from different
tables within the same batch. But get_purge_table(), open_purge_table()
incorrectly assumes that a 1:1 relationship between a purge worker thread
and a table within a single batch. Based on this wrong assumtion,
InnoDB attempts to reuse TABLE objects cached in thd->open_tables for
virtual column computation.
1) Purge worker opens Table A and caches the TABLE pointer in thd->open_tables.
2) Same purge worker moves to Table B in the same batch, get_purge_table()
retrieves the cached pointer for Table A instead of opening Table B.
3) Because innobase::open() is ignored for Table B, the virtual column
template is never initialized.
4) virtual column computation for Table B aborts the server
Solution:
========
- Introduced purge_table class which has the following
purge_table: Stores either TABLE* (for tables with indexed virtual
columns) or MDL_ticket* (for tables without) in a single union
using LSB as a flag.
For tables with indexed virtual columns: opens TABLE*, accesses
MDL_ticket* via TABLE->mdl_ticket
For tables without indexed virtual columns: stores only MDL_ticket*.
trx_purge_attach_undo_recs(): Coordinator opens both dict_table_t*
and TABLE* with proper MDL protection. Workers access cached
table pointers from purge_node_t->tables without opening
their own handles
purge_sys.coordinator_thd: Distinguish coordinator from workers
in cleanup logic. Skip innobase_reset_background_thd() for
coordinator thread to prevent premature table closure during
batch processing. Workers still call cleanup to release their
thread-local resources
trx_purge_close_tables():
Rewrite for purge coordinator thread
1) Close all dict_table_t* objects first
2) Call close_thread_tables() once for all TABLE* objects
3) Release MDL tickets last, after tables are closed
Added table->lock_mutex protection when reading (or) writing
vc_templ->mysql_table and mysql_table_query_id. Clear cached
TABLE* pointers before closing tables to prevent stale pointer
access
Declared open_purge_table() and close_thread_tables() in trx0purge.cc
Declared reset_thd() in row0purge.cc and dict0stats_bg.cc.
Removed innobase_reset_background_thd()
Remove the unscoped #pragma GCC diagnostic ignored
"-Wdeprecated-declarations" from include/violite.h and replace all
sprintf/vsprintf calls with snprintf/vsnprintf across the codebase.
Where possible, pass actual buffer sizes as function parameters
instead of using hardcoded constants or recomputing strlen().
Notable API changes:
- get_date(): added to_len parameter for buffer size
- nice_time(), end_timer() in client/mysql.cc: added buff_size
- generate_new_name(): added name_size parameter
- calc_md5(): added buffer_size parameter (fixes off-by-one)
- MD5_HASH_TO_STRING macro: added _size parameter
- Added MYSQL_UDF_MAX_RESULT_LENGTH constant in mysql_com.h
Also fixed a pre-existing bug in sql/rpl_mi.cc where the ellipsis
"..." was written to the wrong buffer (dbuff instead of buff).
Vendored code is excluded: extra/readline, extra/wolfssl,
wsrep-lib, libmariadb, storage/mroonga/vendor, zlib.
MDL_context_owner was introduced in MySQL for gtest unit testing of MDL,
which MariaDB doesn't use. Its virtual methods (enter_cond, exit_cond,
is_killed, get_thd, notify_shared_lock) are not MDL-specific - they are
part of THD and were only encapsulated in MDL_context_owner as a
misleading abstraction.
This is a cleanup that:
- Removes the MDL_context_owner class entirely
- Removes THD's inheritance from MDL_context_owner
- Changes MDL to use THD* directly instead of MDL_context_owner*
- Removes the redundant MDL_context::get_owner() method
- Moves ENTER_COND/EXIT_COND macros to sql_class.h near THD_ENTER_COND
don't create a permanent stack of paths on the heap and in THD.
Do it on the stack like for sql_mode, sctx, abort_on_warning, and
other THD properties that often need to be changed temporarily.
Fix Sql_path constructor.
Definition
==========
Implement the SET PATH statement.
```
SET PATH 'schema1, schema2, schema3';
```
Function
--------
In MariaDB, the SQL-path will also be used to lookup packages in
package routine invocations, so the following script:
SET PATH 'sys';
SELECT UTL_ENCODE.BASE64_DECODE('data');
invokes SYS.UTL_ENCODE.BASE64_DECODE('data'), i.e. the function
BASE64_DECODE() in the packge UTL_ENCODE in the database SYS
(if such package and package routine exist).
MariaDB implementation details
------------------------------
The ticket's requirement stated the grammar should support delimited
identifiers with backticks or double quotes within the string quote,
ie: the single tick. A simple parser was implemented to handle this
requirement in Sql_path::from_text.
Only 16 schemas are supported per session - we do not want too many
schemas in the path anyway since it will impact performance. This also
means that the path cache is a simple static array for easier memory
management.
The error message shown when MAX_EXECUTION_TIME hint causes a timeout
incorrectly referred to "max_statement_time", which is a different
variable with different units (seconds vs milliseconds). This could
confuse users.
Before: "Query execution was interrupted (max_statement_time exceeded)"
After: "Query was interrupted: execution time limit 1.5 sec exceeded"
The new message is neutral and accurate for both max_statement_time
system variable and MAX_EXECUTION_TIME optimizer hint, displaying the
actual timeout value that was exceeded.
Problem was in wsrep_handle_mdl_conflict function was comparing
thd->lex->sql_command variable for granted MDL-lock.
There is two possible schedules:
(1) FLUSH TABLES ... FOR EXPORT that will take MDL-lock (granted_thd).
INSERT from other node is conflicting operation (request_thd)
and sees MDL-conflict. Because granted_thd has not executed anything
else thd->lex->sql_command == SQLCOM_FLUSH and this case was
correctly handled in wsrep_handle_mdl_conflict i.e. INSERT needs
to wait.
(2) FLUSH TABLES ... FOR EXPORT that will take MDL-lock (granted_thd).
SET SESSION wsrep_sync_wait=0; (granted_thd)
INSERT from other node is conflicting operation (request_thd)
However, thd->lex->sql_command is not stored to taken MDL-lock. Now
as granted_thd is executing SET thd->lex->sql_command != SQLCOM_FLUSH
and INSERT that is BF will abort it and that means also FTFE is
killed and MDL-lock relesed. This is incorrect as FTFE has written
file on filesystem and it can't be really killed.
In this fix wsrep_handle_mdl_conflict is refactored not to use
thd->lex->sql_command as a variable used for decisions. Instead
connection state can be determined also via THD members. E.g.:
* wsrep_thd_is_toi() || wsrep_thd_is_applying - ongoing TOI or applier
* wsrep_thd_is_BF - thread is brute force
* wsrep_thd_is_SR - thread is streaming replication thread
* thd->current_backup_stage != BACKUP_FINISHED - there's ongoing BACKUP
* thd->global_read_lock.is_acquired() - ongoing FTWRL
* thd->locked_tables_mode == LTM_LOCK_TABLES - ongoing FTFE or LOCK TABLES
Send ok packet earlier for SELECT queries that does not have any updates.
This is done in select_send::send_eof()
Axel's select benchmarks shows that this has a notable speed improvement:
1 Thread: 28% TPS speedup
8 Threads: 23%
64 Threads 6%
This was meassured with t_oltp_point_select With InnoDB running the client
over sockets.
Other things:
- Moved error reporting of LIMIT ROWS EXAMINED from handle_select() to
check_limit_rows_examined(). This is to ensure that the error is
reported before send_eof() is called.
- Removed duplicate "Query execution was interrupted" messages for the
same query. killed_for_exceeding_limit_rows_warning_given was introduced
for this purpose. We cannot use 'killed' flag in
killed_for_exceeding_limit_rows() to detect if we already have produced
a warning as the 'killed' in this case is reset for each union.
Reviewer: Sergei Golubchik <serg@mariadb.org>