From 3a2e3851ee46bad83a8ae9647a8e710e09f51714 Mon Sep 17 00:00:00 2001 From: Arcadiy Ivanov Date: Tue, 18 Aug 2026 02:13:59 -0400 Subject: [PATCH] MDEV-40800 Assert in `ha_close()` when a temporary table converts `JOIN_CACHE::join_records()` puts the handler of its table into PFS batch mode and only then opens the scan, and that scan is what materializes a derived table for the first time. When the temporary table outgrows the in-memory engine at that point, `create_internal_tmp_table_from_heap()` closes and deletes the very handler the batch was started on, and `handler::ha_close()` asserts that no batch is in progress. The close is not the only call that breaks. After the swap `table->file` is the replacement handler, whose mode is `PSI_BATCH_MODE_NONE`, so the caller's `end_psi_batch_mode()` would assert too. Ending the batch and leaving it ended is therefore not a fix. Hand the batch over to the replacement: end it on the old handler right before `ha_close()` and start it again on the new one once the swap is complete. It is started at the very end so that the writes performed by the conversion itself are not counted into the reader's batch. `sub_select()` is not affected: it materializes in `join_tab_execution_startup()` before it starts its own batch. ## An incidental second defect in the same function `create_internal_tmp_table_from_heap()` creates and opens the on-disk replacement table before it starts the scan of the in-memory table that supplies the rows. When that scan could not be started the function returned immediately, skipping all of the cleanup: the replacement was left open with its files on disk, its handler was never freed, the saved `proc_info` was not restored, and the blocks allocated on `new_table.mem_root` were orphaned, since `new_table` holds a copy of the `MEM_ROOT` taken before those allocations. Route that exit through a new `err_drop` label placed between `err_killed` and `err1`. `err_killed` cannot be used because its `ha_rnd_end()` asserts that a scan is in progress, and `err1` cannot be used because it removes the files of a table that is still open. That failure is not reachable as the code stands, since the in-memory engine cannot fail to start a scan, so the test drives it with a new debug injection next to the one that already covers the copy loop of the same function. Both tests pin the character set of their table, so that the record width, and with it the point at which the temporary table stops fitting in memory, does not depend on the server default. --- mysql-test/main/error_simulation.result | 23 +++++++ mysql-test/main/error_simulation.test | 35 ++++++++++ .../main/tmp_table_convert_while_read.result | 55 ++++++++++++++++ .../main/tmp_table_convert_while_read.test | 65 +++++++++++++++++++ sql/handler.h | 3 + sql/sql_select.cc | 21 +++++- 6 files changed, 201 insertions(+), 1 deletion(-) create mode 100644 mysql-test/main/tmp_table_convert_while_read.result create mode 100644 mysql-test/main/tmp_table_convert_while_read.test diff --git a/mysql-test/main/error_simulation.result b/mysql-test/main/error_simulation.result index 244f572413ffb..a19f63122d69f 100644 --- a/mysql-test/main/error_simulation.result +++ b/mysql-test/main/error_simulation.result @@ -126,3 +126,26 @@ Got one of the listed errors DROP FUNCTION f1; SET debug_dbug= @saved_dbug; # End of 10.2 tests +# +# Conversion of an internal temporary table to the on-disk engine when +# the scan of the in-memory table cannot be started +# +CREATE TABLE t1 (v VARCHAR(1024)) CHARACTER SET utf8mb4; +INSERT INTO t1 SELECT CONCAT('v', LPAD(seq, 6, '0')) FROM seq_1_to_100; +SET @save_tmp_table_size= @@tmp_table_size; +SET @save_max_heap_table_size= @@max_heap_table_size; +SET SESSION tmp_table_size= 262144; +SET SESSION max_heap_table_size= 262144; +SET SESSION debug_dbug='+d,heap_conversion_rnd_init_error'; +SELECT COUNT(*) FROM t1 a JOIN (SELECT DISTINCT v FROM t1) d ON a.v = d.v; +ERROR HY000: Table definition has changed, please retry transaction +SET debug_dbug= @saved_dbug; +#The on-disk table created for the failed conversion is gone. +#Without the injected failure the query is answered as usual. +SELECT COUNT(*) FROM t1 a JOIN (SELECT DISTINCT v FROM t1) d ON a.v = d.v; +COUNT(*) +100 +SET SESSION tmp_table_size= @save_tmp_table_size; +SET SESSION max_heap_table_size= @save_max_heap_table_size; +DROP TABLE t1; +# End of 13.1 tests diff --git a/mysql-test/main/error_simulation.test b/mysql-test/main/error_simulation.test index 3beeefce39fde..69840fa88af7d 100644 --- a/mysql-test/main/error_simulation.test +++ b/mysql-test/main/error_simulation.test @@ -157,3 +157,38 @@ DROP FUNCTION f1; SET debug_dbug= @saved_dbug; --echo # End of 10.2 tests + +--echo # +--echo # Conversion of an internal temporary table to the on-disk engine when +--echo # the scan of the in-memory table cannot be started +--echo # + +CREATE TABLE t1 (v VARCHAR(1024)) CHARACTER SET utf8mb4; +INSERT INTO t1 SELECT CONCAT('v', LPAD(seq, 6, '0')) FROM seq_1_to_100; + +SET @save_tmp_table_size= @@tmp_table_size; +SET @save_max_heap_table_size= @@max_heap_table_size; + +# Small enough that the temporary table of the query below does not fit in +# memory and is converted to the on-disk engine. +SET SESSION tmp_table_size= 262144; +SET SESSION max_heap_table_size= 262144; + +--let $tmpdir= `SELECT @@tmpdir` + +SET SESSION debug_dbug='+d,heap_conversion_rnd_init_error'; +--error ER_TABLE_DEF_CHANGED +SELECT COUNT(*) FROM t1 a JOIN (SELECT DISTINCT v FROM t1) d ON a.v = d.v; +SET debug_dbug= @saved_dbug; + +--echo #The on-disk table created for the failed conversion is gone. +--list_files $tmpdir #sql-temptable* + +--echo #Without the injected failure the query is answered as usual. +SELECT COUNT(*) FROM t1 a JOIN (SELECT DISTINCT v FROM t1) d ON a.v = d.v; + +SET SESSION tmp_table_size= @save_tmp_table_size; +SET SESSION max_heap_table_size= @save_max_heap_table_size; +DROP TABLE t1; + +--echo # End of 13.1 tests diff --git a/mysql-test/main/tmp_table_convert_while_read.result b/mysql-test/main/tmp_table_convert_while_read.result new file mode 100644 index 0000000000000..e0830af26e7a2 --- /dev/null +++ b/mysql-test/main/tmp_table_convert_while_read.result @@ -0,0 +1,55 @@ +CREATE TABLE t1 (v VARCHAR(1024)) CHARACTER SET utf8mb4; +INSERT INTO t1 SELECT CONCAT('v', LPAD(seq, 6, '0')) FROM seq_1_to_100; +CREATE TABLE t2 (a INT); +INSERT INTO t2 VALUES (1),(2); +SET @save_tmp_table_size= @@tmp_table_size; +SET @save_max_heap_table_size= @@max_heap_table_size; +SET SESSION tmp_table_size= 262144; +SET SESSION max_heap_table_size= 262144; +# Derived table with DISTINCT +FLUSH STATUS; +SELECT COUNT(*) FROM t1 a JOIN (SELECT DISTINCT v FROM t1) d ON a.v = d.v; +COUNT(*) +100 +SHOW STATUS LIKE 'Created_tmp_disk_tables'; +Variable_name Value +Created_tmp_disk_tables 2 +# Derived table with GROUP BY +FLUSH STATUS; +SELECT COUNT(*) FROM t1 a JOIN (SELECT v FROM t1 GROUP BY v) d ON a.v = d.v; +COUNT(*) +100 +SHOW STATUS LIKE 'Created_tmp_disk_tables'; +Variable_name Value +Created_tmp_disk_tables 2 +SET SESSION optimizer_switch='derived_merge=off'; +# Derived table that is not merged into the outer query +FLUSH STATUS; +SELECT COUNT(*) FROM t2, (SELECT v FROM t1) AS sq; +COUNT(*) +200 +SHOW STATUS LIKE 'Created_tmp_disk_tables'; +Variable_name Value +Created_tmp_disk_tables 1 +# Derived table filled by a UNION +FLUSH STATUS; +SELECT COUNT(*) FROM t2, (SELECT v FROM t1 UNION SELECT v FROM t1) AS sq; +COUNT(*) +200 +SHOW STATUS LIKE 'Created_tmp_disk_tables'; +Variable_name Value +Created_tmp_disk_tables 2 +SET SESSION optimizer_switch=DEFAULT; +# View materialized into a temporary table +CREATE ALGORITHM=TEMPTABLE VIEW v1 AS SELECT * FROM t1; +FLUSH STATUS; +SELECT COUNT(*) FROM v1, t2 WHERE a = 1; +COUNT(*) +100 +SHOW STATUS LIKE 'Created_tmp_disk_tables'; +Variable_name Value +Created_tmp_disk_tables 1 +DROP VIEW v1; +SET SESSION tmp_table_size= @save_tmp_table_size; +SET SESSION max_heap_table_size= @save_max_heap_table_size; +DROP TABLE t1, t2; diff --git a/mysql-test/main/tmp_table_convert_while_read.test b/mysql-test/main/tmp_table_convert_while_read.test new file mode 100644 index 0000000000000..196ac0a2b4c3f --- /dev/null +++ b/mysql-test/main/tmp_table_convert_while_read.test @@ -0,0 +1,65 @@ +# +# An internal temporary table can be converted from the in-memory engine to +# the on-disk engine while a reader of that same table is already in progress. +# The reader keeps state on the handler of the table, so the conversion, which +# replaces that handler, has to carry the state over to the new one. +# +--source include/have_sequence.inc + +CREATE TABLE t1 (v VARCHAR(1024)) CHARACTER SET utf8mb4; +INSERT INTO t1 SELECT CONCAT('v', LPAD(seq, 6, '0')) FROM seq_1_to_100; + +CREATE TABLE t2 (a INT); +INSERT INTO t2 VALUES (1),(2); + +SET @save_tmp_table_size= @@tmp_table_size; +SET @save_max_heap_table_size= @@max_heap_table_size; + +# Small enough that the temporary tables below do not fit in memory. Set +# explicitly so that the test does not depend on the server defaults. +SET SESSION tmp_table_size= 262144; +SET SESSION max_heap_table_size= 262144; + +# Created_tmp_disk_tables is checked after every query so that a future change +# of the sizes above cannot silently leave these queries without a conversion. +--disable_ps2_protocol +--disable_cursor_protocol + +--echo # Derived table with DISTINCT +FLUSH STATUS; +SELECT COUNT(*) FROM t1 a JOIN (SELECT DISTINCT v FROM t1) d ON a.v = d.v; +SHOW STATUS LIKE 'Created_tmp_disk_tables'; + +--echo # Derived table with GROUP BY +FLUSH STATUS; +SELECT COUNT(*) FROM t1 a JOIN (SELECT v FROM t1 GROUP BY v) d ON a.v = d.v; +SHOW STATUS LIKE 'Created_tmp_disk_tables'; + +SET SESSION optimizer_switch='derived_merge=off'; + +--echo # Derived table that is not merged into the outer query +FLUSH STATUS; +SELECT COUNT(*) FROM t2, (SELECT v FROM t1) AS sq; +SHOW STATUS LIKE 'Created_tmp_disk_tables'; + +--echo # Derived table filled by a UNION +FLUSH STATUS; +SELECT COUNT(*) FROM t2, (SELECT v FROM t1 UNION SELECT v FROM t1) AS sq; +SHOW STATUS LIKE 'Created_tmp_disk_tables'; + +SET SESSION optimizer_switch=DEFAULT; + +--echo # View materialized into a temporary table +CREATE ALGORITHM=TEMPTABLE VIEW v1 AS SELECT * FROM t1; +FLUSH STATUS; +SELECT COUNT(*) FROM v1, t2 WHERE a = 1; +SHOW STATUS LIKE 'Created_tmp_disk_tables'; +DROP VIEW v1; + +--enable_cursor_protocol +--enable_ps2_protocol + +SET SESSION tmp_table_size= @save_tmp_table_size; +SET SESSION max_heap_table_size= @save_max_heap_table_size; + +DROP TABLE t1, t2; diff --git a/sql/handler.h b/sql/handler.h index f81b26a7642a2..4f9d2bf0d0151 100644 --- a/sql/handler.h +++ b/sql/handler.h @@ -3326,6 +3326,9 @@ class handler :public Sql_alloc void start_psi_batch_mode(); /** End a batch started with @c start_psi_batch_mode. */ void end_psi_batch_mode(); + /** Check if a batch started with @c start_psi_batch_mode is in progress. */ + bool is_in_psi_batch_mode() const + { return m_psi_batch_mode != PSI_BATCH_MODE_NONE; } /* If we have row logging enabled for this table */ bool row_logging, row_logging_init; diff --git a/sql/sql_select.cc b/sql/sql_select.cc index 183c29a933930..5bce7d6810b4c 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -22437,6 +22437,7 @@ create_internal_tmp_table_from_heap(THD *thd, TABLE *table, TABLE_SHARE share; const char *save_proc_info; int write_err= 0; + bool psi_batch_mode= false; DBUG_ENTER("create_internal_tmp_table_from_heap"); if (is_duplicate) *is_duplicate= FALSE; @@ -22478,8 +22479,13 @@ create_internal_tmp_table_from_heap(THD *thd, TABLE *table, if (table->file->indexes_are_disabled()) new_table.file->ha_disable_indexes(key_map(0), false); table->file->ha_index_or_rnd_end(); + DBUG_EXECUTE_IF("heap_conversion_rnd_init_error", + { + table->file->print_error(HA_ERR_TABLE_DEF_CHANGED, MYF(0)); + goto err_drop; + }); if (table->file->ha_rnd_init_with_error(1)) - DBUG_RETURN(1); + goto err_drop; if (new_table.no_rows) new_table.file->extra(HA_EXTRA_NO_ROWS); else @@ -22523,6 +22529,16 @@ create_internal_tmp_table_from_heap(THD *thd, TABLE *table, /* remove heap table and change to use myisam table */ (void) table->file->ha_rnd_end(); + /* + A reader of this table may have put its handler into PFS batch mode, see + JOIN_TAB::pfs_batch_update(). The reader ends the batch on the handler + that table->file points to at that time, which is the new handler, so the + batch has to be moved over to it. Ending it here without starting it again + would leave that call without a matching start_psi_batch_mode(). + */ + psi_batch_mode= table->file->is_in_psi_batch_mode(); + if (psi_batch_mode) + table->file->end_psi_batch_mode(); (void) table->file->ha_close(); // This deletes the table ! delete table->file; table->file=0; @@ -22534,6 +22550,8 @@ create_internal_tmp_table_from_heap(THD *thd, TABLE *table, table->file->change_table_ptr(table, table->s); table->use_all_columns(); + if (psi_batch_mode) + table->file->start_psi_batch_mode(); if (save_proc_info) thd_proc_info(thd, (!strcmp(save_proc_info,"Copying to tmp table") ? "Copying to tmp table on disk" : save_proc_info)); @@ -22544,6 +22562,7 @@ create_internal_tmp_table_from_heap(THD *thd, TABLE *table, table->file->print_error(write_err, MYF(0)); err_killed: (void) table->file->ha_rnd_end(); +err_drop: (void) new_table.file->ha_close(); err1: TMP_ENGINE_HTON->drop_table(TMP_ENGINE_HTON, new_table.s->path.str);