Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions mysql-test/main/mdev-39597.result
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
#
# MDEV-39597: SHOW PROFILE reports duplicate sending data
# stage for INSERT...SELECT due to save/restore in free_tmp_table
#
CREATE TABLE profile_test (
id INT PRIMARY KEY AUTO_INCREMENT,
name VARCHAR(100),
value INT
);
INSERT INTO profile_test (name, value) VALUES
('alpha',1),('beta',2),('gamma',3),('delta',4),('epsilon',5),
('zeta',6),('eta',7),('theta',8),('iota',9),('kappa',10);
SET profiling = 1;
INSERT INTO profile_test (name, value)
SELECT name, value FROM profile_test LIMIT 5;
# Check that no stage appears twice consecutivley around removing tmp table
SELECT COUNT(*) AS duplicate_stage_found
FROM (
SELECT
STATE,
LAG(STATE) OVER (ORDER BY SEQ) AS prev_status,
LEAD(STATE) OVER (ORDER BY SEQ) AS next_status
FROM information_schema.profiling
WHERE QUERY_ID = 1
) t
WHERE STATE = 'Removing tmp table'
AND prev_status = next_status;
duplicate_stage_found
0
SET profiling = 0;
DROP TABLE profile_test;
39 changes: 39 additions & 0 deletions mysql-test/main/mdev-39597.test
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
--echo #
--echo # MDEV-39597: SHOW PROFILE reports duplicate sending data
--echo # stage for INSERT...SELECT due to save/restore in free_tmp_table
--echo #

--source include/have_profiling.inc
--disable_ps2_protocol

CREATE TABLE profile_test (
id INT PRIMARY KEY AUTO_INCREMENT,
name VARCHAR(100),
value INT
);
INSERT INTO profile_test (name, value) VALUES
('alpha',1),('beta',2),('gamma',3),('delta',4),('epsilon',5),
('zeta',6),('eta',7),('theta',8),('iota',9),('kappa',10);
SET profiling = 1;
INSERT INTO profile_test (name, value)
SELECT name, value FROM profile_test LIMIT 5;

--echo # Check that no stage appears twice consecutivley around removing tmp table

SELECT COUNT(*) AS duplicate_stage_found
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not get the effect described using the queries in the test neither in 10.11 nor in 13.1. In 13.1 I get:

MariaDB [test]> SELECT
    ->     STATE,
    ->     LAG(STATE) OVER (ORDER BY SEQ) AS prev_status,
    ->     LEAD(STATE) OVER (ORDER BY SEQ) AS next_status
    ->   FROM information_schema.profiling
    ->   WHERE QUERY_ID = 1;
+------------------------+-----------------+------------------------+
| STATE                  | prev_status     | next_status            |
+------------------------+-----------------+------------------------+
| Starting               | NULL            | Freeing items          |
| Freeing items          | Starting        | Updating status        |
| Updating status        | Freeing items   | Reset for next command |
| Reset for next command | Updating status | NULL                   |
+------------------------+-----------------+------------------------+
4 rows in set (0.006 sec)

And in 10.11 I get:

MariaDB [(none)]> SELECT
    ->     STATE,
    ->     LAG(STATE) OVER (ORDER BY SEQ) AS prev_status,
    ->     LEAD(STATE) OVER (ORDER BY SEQ) AS next_status
    ->   FROM information_schema.profiling
    ->   WHERE QUERY_ID = 1;
+------------------------+-----------------+------------------------+
| STATE                  | prev_status     | next_status            |
+------------------------+-----------------+------------------------+
| Starting               | NULL            | Freeing items          |
| Freeing items          | Starting        | Updating status        |
| Updating status        | Freeing items   | Reset for next command |
| Reset for next command | Updating status | NULL                   |
+------------------------+-----------------+------------------------+
4 rows in set (0.001 sec)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the review.
On reproducing the issue:
The profile output shown has only 4 stages "Starting", "Freeing items", "Updating status", "Reset for next command", which is consistent with a simple statement, not an INSERT...SELECT. An INSERT...SELECT would show stages including "Opening tables", "Creating tmp table", "Sending data". It appears the INSERT...SELECT from the test setup was not run before querying information_schema.profiling.
To reproduce, please run the full sequence in a fresh session:

CREATE TABLE profile_test (
  id INT PRIMARY KEY AUTO_INCREMENT,
  name VARCHAR(100),
  value INT
);
INSERT INTO profile_test (name, value) VALUES
  ('alpha',1),('beta',2),('gamma',3),('delta',4),('epsilon',5),
  ('zeta',6),('eta',7),('theta',8),('iota',9),('kappa',10);
SET profiling = 1;
INSERT INTO profile_test (name, value)
  SELECT name, value FROM profile_test LIMIT 5;
SHOW PROFILE;

SET profiling = 1 is not itself profiled, so the INSERT...SELECT becomes QUERY_ID = 1.

The table creation and data insertion are setup steps required to trigger the tmp table code path, without them the duplicate will not appear.

On the test:
The test was confirmed to fail on unpatched code and pass with the fix applied locally. The trailing newline has been added.

On the lowest affected version:
The same save/restore pattern exists in 10.11. I will confirm the bug reproduces there and rebase accordingly once we confirm the reproduce steps work for you.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my bad: I missed the INSERT part when trying. It does indeed fail when not patched.

FROM (
SELECT
STATE,
LAG(STATE) OVER (ORDER BY SEQ) AS prev_status,
LEAD(STATE) OVER (ORDER BY SEQ) AS next_status
FROM information_schema.profiling
WHERE QUERY_ID = 1
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Hardcoding QUERY_ID = 1 makes the test fragile. If additional queries are added to the test file before profiling is enabled, or if the test is integrated into a larger suite where the connection is reused, the QUERY_ID for the target statement might not be 1. It is more robust to use a subquery to fetch the most recent QUERY_ID.

  WHERE QUERY_ID = (SELECT MAX(QUERY_ID) FROM information_schema.profiling)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QUERY_ID = 1 is intentional. SET profiling = 1 is not profiled, so the INSERT...SELECT is always query 1 in a fresh session. Using MAX(QUERY_ID) would be fragile here as subsequent queries in the test would become the MAX.

) t
WHERE STATE = 'Removing tmp table'
AND prev_status = next_status;

--enable_ps2_protocol

# clean up
SET profiling = 0;
DROP TABLE profile_test;
5 changes: 4 additions & 1 deletion mysql-test/main/query_cache_executable_comments.test
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
--source include/have_query_cache.inc
--source include/not_embedded.inc

if (!$QUERY_CACHE_INFO_SO)
{
--skip Need query_cache_info plugin
}
--disable_ps_protocol
--disable_ps2_protocol
--disable_cursor_protocol
Expand Down
3 changes: 0 additions & 3 deletions sql/sql_select.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23932,12 +23932,10 @@ void
free_tmp_table(THD *thd, TABLE *entry)
{
MEM_ROOT own_root= entry->mem_root;
const char *save_proc_info;
DBUG_ENTER("free_tmp_table");
DBUG_PRINT("enter",("table: %s alias: %s",entry->s->table_name.str,
entry->alias.c_ptr()));

save_proc_info=thd->proc_info;
THD_STAGE_INFO(thd, stage_removing_tmp_table);

if (entry->file && entry->is_created())
Expand Down Expand Up @@ -23980,7 +23978,6 @@ free_tmp_table(THD *thd, TABLE *entry)
}

free_root(&own_root, MYF(0)); /* the table is allocated in its own root */
thd_proc_info(thd, save_proc_info);

DBUG_VOID_RETURN;
}
Expand Down