diff --git a/mysql-test/main/group_min_max_innodb.result b/mysql-test/main/group_min_max_innodb.result index 27656374aee38..a62c949d33f0d 100644 --- a/mysql-test/main/group_min_max_innodb.result +++ b/mysql-test/main/group_min_max_innodb.result @@ -440,3 +440,31 @@ SET GLOBAL innodb_lru_scan_depth= @lru_depth.save; set global innodb_stats_persistent= @innodb_stats_persistent_save; set global innodb_stats_persistent_sample_pages= @innodb_stats_persistent_sample_pages_save; +# +# Begin 10.11 tests +# +CREATE TABLE t1 (pk INT, a INT, b INT, PRIMARY KEY (pk), KEY (a,b)) ENGINE=InnoDB; +INSERT INTO t1 (pk,a) VALUES (1,11),(2,12),(3,13),(4,14); +connect con1,localhost,root,,; +DELETE FROM t1 WHERE pk <= 3; +connection default; +INSERT INTO t1 (pk) VALUES (5); +connection con1; +START TRANSACTION; +INSERT INTO t1 (pk) VALUES (6); +connection default; +SET @save_innodb_lock_wait_timeout= @@innodb_lock_wait_timeout; +SET innodb_lock_wait_timeout= 1; +EXPLAIN SELECT MIN(b), MAX(b), a FROM t1 GROUP BY a FOR UPDATE; +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE t1 range NULL a 10 NULL # Using index for group-by +SELECT MIN(b), MAX(b), a FROM t1 GROUP BY a FOR UPDATE; +ERROR HY000: Lock wait timeout exceeded; try restarting transaction +SET innodb_lock_wait_timeout= @save_innodb_lock_wait_timeout; +connection con1; +COMMIT; +DROP TABLE t1; +disconnect con1; +# +# End 10.11 tests +# diff --git a/mysql-test/main/group_min_max_innodb.test b/mysql-test/main/group_min_max_innodb.test index 33a3a8888a5d8..51b3487077a0c 100644 --- a/mysql-test/main/group_min_max_innodb.test +++ b/mysql-test/main/group_min_max_innodb.test @@ -323,3 +323,50 @@ SET GLOBAL innodb_lru_scan_depth= @lru_depth.save; set global innodb_stats_persistent= @innodb_stats_persistent_save; set global innodb_stats_persistent_sample_pages= @innodb_stats_persistent_sample_pages_save; + +--echo # +--echo # Begin 10.11 tests +--echo # + +# Default client connection to the database. +CREATE TABLE t1 (pk INT, a INT, b INT, PRIMARY KEY (pk), KEY (a,b)) ENGINE=InnoDB; +INSERT INTO t1 (pk,a) VALUES (1,11),(2,12),(3,13),(4,14); + +# Start a new, separate client connection to the database called 'con1'. +--connect (con1,localhost,root,,) +--send +DELETE FROM t1 WHERE pk <= 3; + +# On the default connection, insert a value into the table. +--connection default +INSERT INTO t1 (pk) VALUES (5); + +# On 'con1', insert a value within a transaction and hold the transaction while +# on the default connection try to compute an aggregate. +--connection con1 +--reap + +START TRANSACTION; +INSERT INTO t1 (pk) VALUES (6); + +--connection default +SET @save_innodb_lock_wait_timeout= @@innodb_lock_wait_timeout; +SET innodb_lock_wait_timeout= 1; +# SELECT ... FOR UPDATE locks the rows for write operations and prevents other transactions +# from modifying or reading the selected rows until the transaction ends. +# This should lead to the error, not a crash. +--replace_column 9 # +EXPLAIN SELECT MIN(b), MAX(b), a FROM t1 GROUP BY a FOR UPDATE; +--error ER_LOCK_WAIT_TIMEOUT +SELECT MIN(b), MAX(b), a FROM t1 GROUP BY a FOR UPDATE; +SET innodb_lock_wait_timeout= @save_innodb_lock_wait_timeout; + +# Cleanup, commit the 'con1' transaction. +--connection con1 +COMMIT; +DROP TABLE t1; +--disconnect con1 + +--echo # +--echo # End 10.11 tests +--echo # diff --git a/sql/opt_range.cc b/sql/opt_range.cc index b042445a7aa1a..cb064b88eceab 100644 --- a/sql/opt_range.cc +++ b/sql/opt_range.cc @@ -15715,6 +15715,11 @@ int QUICK_GROUP_MIN_MAX_SELECT::get_next() */ if (have_min) { + /* + If next_min returns a handler error (HA_ERR_...), then that error + will be propagated to the caller of this method by setting result + at the bottom of this loop. + */ min_res= next_min(); if (min_res == 0) update_min_result(); @@ -15723,12 +15728,26 @@ int QUICK_GROUP_MIN_MAX_SELECT::get_next() if ((have_max && !have_min) || (have_max && have_min && (min_res == 0))) { + /* + Handler errors returned by next_max get treated in one of two ways: + 1. In a Debug build, the assertion in this block will cause a + crash (because max_res != 0). + 2. In a Release build, the handler error will be swallowed up + by the the result value update at the end of this loop. + There may be valid group of rows available but next_max() will + return a lock wait timeout error on a transaction conflict. + That must be propagated to the caller. + + next_min() and next_max() are not symmetrical with respect to + how they access the underlying table. next_min() may read the + index if there's an infix of constants from equality predicates + available (key_infix_len > 0) but next_max() makes no such check + Hence the lock wait timeout may be seen by next_max() but not by + next_min(). + */ max_res= next_max(); if (max_res == 0) update_max_result(); - /* If a MIN was found, a MAX must have been found as well. */ - DBUG_ASSERT((have_max && !have_min) || - (have_max && have_min && (max_res == 0))); } /* If this is just a GROUP BY or DISTINCT without MIN or MAX and there @@ -15740,7 +15759,16 @@ int QUICK_GROUP_MIN_MAX_SELECT::get_next() make_prev_keypart_map(real_key_parts), HA_READ_KEY_EXACT); + /* + 1. If we're computing a MIN only, then take its result. + 2. If we're computing a MAX only, then take its result. + 3. If we're computing both a MIN and a MAX, then take the MIN + result which swallows the MAX result in all cases... + */ result= have_min ? min_res : have_max ? max_res : result; + // ... 4. so check if there's a MAX error and return it if it exists. + if (result == 0 && have_max && max_res != 0) + result= max_res; } while (result == HA_ERR_KEY_NOT_FOUND || result == HA_ERR_END_OF_FILE); if (result == HA_ERR_KEY_NOT_FOUND) @@ -15797,11 +15825,15 @@ int QUICK_GROUP_MIN_MAX_SELECT::next_min() } /* + The reason that next_min has more complexity than next_max() is + for NULLs handling. In MariaDB, NULLs appear first in a result + and by default sort as less-than every other value. + If the min/max argument field is NULL, skip subsequent rows in the same group with NULL in it. Notice that: - - if the first row in a group doesn't have a NULL in the field, no row - in the same group has (because NULL < any other value), - - min_max_arg_part->field->ptr points to some place in 'record'. + - if the first row in a group doesn't have a NULL in the field, no row + in the same group has (because NULL < any other value), + - min_max_arg_part->field->ptr points to some place in 'record'. */ if (min_max_arg_part && min_max_arg_part->field->is_null()) {