MDEV-15656 Assertion `is_last_prefix <= 0' failed in QUICK_GROUP_MIN_MAX_SELECT::get_next

When QUICK_GROUP_MIN_MAX_SELECT is initialized or being reset
it stores the prefix of the last group of the index chosen for
retrieving data (last_value). Later, when looping through records
at get_next() method, the server checks whether the retrieved
group is the last, and if so, it finishes processing.

At the same time, it looks like there is no need for that additional
check since method next_prefix() returns HA_ERR_KEY_NOT_FOUND
or HA_ERR_END_OF_FILE when there are no more satisfying records.
If we do not perform the check, we do not need to retrieve and
store last_value either.

This commit removes using of last_value from QUICK_GROUP_MIN_MAX_SELECT.

Reviewer: Sergei Petrunia <sergey@mariadb.com>

mysql-test/main/group_min_max_innodb.result

@@ -330,6 +330,113 @@ JOIN t1 ON dt.a=t1.b;
 a
 Australia
 DROP TABLES t1, t2;
+#
+# MDEV-15656: Assertion `is_last_prefix <= 0' failed in
+#             QUICK_GROUP_MIN_MAX_SELECT::get_next
+#
+SET @lru_depth.save= @@innodb_lru_scan_depth;
+SET GLOBAL innodb_lru_scan_depth= 1024;
+CREATE TABLE t1 (
+pk_part1 INT AUTO_INCREMENT,
+a VARCHAR(4),
+row_start timestamp(6) default current_timestamp,
+PRIMARY KEY (pk_part1, row_start)
+) ENGINE=InnoDB;
+INSERT INTO t1 (a) VALUES
+('foo'),('bar'),('foo'),('bar'),('foo'),
+('foo'),('bar'),('foo'),('bar'),('foo'),
+('foo'),('bar'),('foo'),('bar'),('foo'),
+('foo'),('bar'),('foo'),('bar'),('foo'),
+('foo'),('bar'),('foo'),('bar'),('foo'),
+('foo'),('bar'),('foo'),('bar'),('foo'),
+('foo'),('bar'),('foo'),('bar'),('foo'),
+('foo'),('bar'),('foo'),('bar'),('foo');
+connect  con1,localhost,root,,test;
+SET SESSION TRANSACTION ISOLATION LEVEL READ UNCOMMITTED;
+SELECT DISTINCT pk_part1 FROM t1;
+connection default;
+INSERT INTO t1 (pk_part1) VALUES (NULL);
+connection con1;
+SELECT DISTINCT pk_part1 FROM t1;
+connection default;
+INSERT INTO t1 (pk_part1) VALUES (NULL);
+connection con1;
+SELECT DISTINCT pk_part1 FROM t1;
+connection default;
+INSERT INTO t1 (pk_part1) VALUES (NULL);
+connection con1;
+SELECT DISTINCT pk_part1 FROM t1;
+connection default;
+INSERT INTO t1 (pk_part1) VALUES (NULL);
+connection con1;
+SELECT DISTINCT pk_part1 FROM t1;
+connection default;
+INSERT INTO t1 (pk_part1) VALUES (NULL);
+connection con1;
+SELECT DISTINCT pk_part1 FROM t1;
+connection default;
+INSERT INTO t1 (pk_part1) VALUES (NULL);
+connection con1;
+SELECT DISTINCT pk_part1 FROM t1;
+connection default;
+INSERT INTO t1 (pk_part1) VALUES (NULL);
+connection con1;
+SELECT DISTINCT pk_part1 FROM t1;
+connection default;
+INSERT INTO t1 (pk_part1) VALUES (NULL);
+connection con1;
+SELECT DISTINCT pk_part1 FROM t1;
+connection default;
+INSERT INTO t1 (pk_part1) VALUES (NULL);
+connection con1;
+SELECT DISTINCT pk_part1 FROM t1;
+connection default;
+INSERT INTO t1 (pk_part1) VALUES (NULL);
+connection con1;
+SELECT DISTINCT pk_part1 FROM t1;
+connection default;
+INSERT INTO t1 (pk_part1) VALUES (NULL);
+connection con1;
+SELECT DISTINCT pk_part1 FROM t1;
+connection default;
+INSERT INTO t1 (pk_part1) VALUES (NULL);
+connection con1;
+SELECT DISTINCT pk_part1 FROM t1;
+connection default;
+INSERT INTO t1 (pk_part1) VALUES (NULL);
+connection con1;
+SELECT DISTINCT pk_part1 FROM t1;
+connection default;
+INSERT INTO t1 (pk_part1) VALUES (NULL);
+connection con1;
+SELECT DISTINCT pk_part1 FROM t1;
+connection default;
+INSERT INTO t1 (pk_part1) VALUES (NULL);
+connection con1;
+SELECT DISTINCT pk_part1 FROM t1;
+connection default;
+INSERT INTO t1 (pk_part1) VALUES (NULL);
+connection con1;
+SELECT DISTINCT pk_part1 FROM t1;
+connection default;
+INSERT INTO t1 (pk_part1) VALUES (NULL);
+connection con1;
+SELECT DISTINCT pk_part1 FROM t1;
+connection default;
+INSERT INTO t1 (pk_part1) VALUES (NULL);
+connection con1;
+SELECT DISTINCT pk_part1 FROM t1;
+connection default;
+INSERT INTO t1 (pk_part1) VALUES (NULL);
+connection con1;
+SELECT DISTINCT pk_part1 FROM t1;
+connection default;
+INSERT INTO t1 (pk_part1) VALUES (NULL);
+connection con1;
+disconnect con1;
+connection default;
+DROP TABLE t1;
+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;

mysql-test/main/group_min_max_innodb.test

@@ -273,6 +273,53 @@ eval $query;
 
 DROP TABLES t1, t2;
 
+--echo #
+--echo # MDEV-15656: Assertion `is_last_prefix <= 0' failed in
+--echo #             QUICK_GROUP_MIN_MAX_SELECT::get_next
+--echo #
+SET @lru_depth.save= @@innodb_lru_scan_depth;
+SET GLOBAL innodb_lru_scan_depth= 1024;
+
+CREATE TABLE t1 (
+  pk_part1 INT AUTO_INCREMENT,
+  a VARCHAR(4),
+  row_start timestamp(6) default current_timestamp,
+  PRIMARY KEY (pk_part1, row_start)
+) ENGINE=InnoDB;
+
+INSERT INTO t1 (a) VALUES
+('foo'),('bar'),('foo'),('bar'),('foo'),
+('foo'),('bar'),('foo'),('bar'),('foo'),
+('foo'),('bar'),('foo'),('bar'),('foo'),
+('foo'),('bar'),('foo'),('bar'),('foo'),
+('foo'),('bar'),('foo'),('bar'),('foo'),
+('foo'),('bar'),('foo'),('bar'),('foo'),
+('foo'),('bar'),('foo'),('bar'),('foo'),
+('foo'),('bar'),('foo'),('bar'),('foo');
+
+--connect (con1,localhost,root,,test)
+
+SET SESSION TRANSACTION ISOLATION LEVEL READ UNCOMMITTED;
+
+--let $run= 20
+--disable_result_log
+while ($run)
+{
+  --send
+    SELECT DISTINCT pk_part1 FROM t1;
+  --connection default
+  INSERT INTO t1 (pk_part1) VALUES (NULL);
+  --connection con1
+  --reap
+  --dec $run
+}
+--enable_result_log
+
+--disconnect con1
+--connection default
+DROP TABLE t1;
+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;

mysql-test/main/innodb_ext_key,covering,innodb,on.rdiff

@@ -0,0 +1,20 @@
+--- ./main/innodb_ext_key.result
++++ ./main/innodb_ext_key.reject
+@@ -244,7 +244,7 @@
+ Variable_name	Value
+ Handler_read_first	0
+ Handler_read_key	21
+-Handler_read_last	1
++Handler_read_last	0
+ Handler_read_next	0
+ Handler_read_prev	0
+ Handler_read_retry	0
+@@ -266,7 +266,7 @@
+ Variable_name	Value
+ Handler_read_first	0
+ Handler_read_key	6
+-Handler_read_last	1
++Handler_read_last	0
+ Handler_read_next	0
+ Handler_read_prev	0
+ Handler_read_retry	0

mysql-test/main/innodb_ext_key,innodb,on,unoptimized.rdiff

@@ -0,0 +1,20 @@
+--- ./main/innodb_ext_key.result
++++ ./main/innodb_ext_key.reject
+@@ -244,7 +244,7 @@
+ Variable_name	Value
+ Handler_read_first	0
+ Handler_read_key	21
+-Handler_read_last	1
++Handler_read_last	0
+ Handler_read_next	0
+ Handler_read_prev	0
+ Handler_read_retry	0
+@@ -266,7 +266,7 @@
+ Variable_name	Value
+ Handler_read_first	0
+ Handler_read_key	6
+-Handler_read_last	1
++Handler_read_last	0
+ Handler_read_next	0
+ Handler_read_prev	0
+ Handler_read_retry	0

sql/opt_range.cc

@@ -14669,13 +14669,6 @@ int QUICK_GROUP_MIN_MAX_SELECT::init()
 {
   if (group_prefix) /* Already initialized. */
     return 0;
-  
-  /*
-    We allocate one byte more to serve the case when the last field in
-    the buffer is compared using uint3korr (e.g. a Field_newdate field)
-  */
-  if (!(last_prefix= (uchar*) alloc_root(&alloc, group_prefix_len+1)))
-      return 1;
   /*
     We may use group_prefix to store keys with all select fields, so allocate
     enough space for it.
@@ -14931,8 +14924,7 @@ void QUICK_GROUP_MIN_MAX_SELECT::update_key_stat()
     QUICK_GROUP_MIN_MAX_SELECT::reset()
 
   DESCRIPTION
-    Initialize the index chosen for access and find and store the prefix
+    Initialize the index chosen for access.
-    of the last group. The method is expensive since it performs disk access.
 
   RETURN
     0      OK
@@ -14954,12 +14946,6 @@ int QUICK_GROUP_MIN_MAX_SELECT::reset(void)
   }
   if (quick_prefix_select && quick_prefix_select->reset())
     DBUG_RETURN(1);
-  result= file->ha_index_last(record);
-  if (result == HA_ERR_END_OF_FILE)
-    DBUG_RETURN(0);
-  /* Save the prefix of the last group. */
-  key_copy(last_prefix, record, index_info, group_prefix_len);
-
   DBUG_RETURN(0);
 }
 
@@ -15005,34 +14991,20 @@ int QUICK_GROUP_MIN_MAX_SELECT::get_next()
 #else
   int result;
 #endif
-  int is_last_prefix= 0;
-
   DBUG_ENTER("QUICK_GROUP_MIN_MAX_SELECT::get_next");
 
   /*
-    Loop until a group is found that satisfies all query conditions or the last
+    Loop until a group is found that satisfies all query conditions or
-    group is reached.
+    there are no satisfying groups left
   */
   do
   {
     result= next_prefix();
-    /*
+    if (result != 0)
-      Check if this is the last group prefix. Notice that at this point
-      this->record contains the current prefix in record format.
-    */
-    if (!result)
-    {
-      is_last_prefix= key_cmp(index_info->key_part, last_prefix,
-                              group_prefix_len);
-      DBUG_ASSERT(is_last_prefix <= 0);
-    }
-    else 
-    {
-      if (result == HA_ERR_KEY_NOT_FOUND)
-        continue;
       break;
-    }
+    /*
-
+      At this point this->record contains the current prefix in record format.
+    */
     if (have_min)
     {
       min_res= next_min();
@@ -15061,8 +15033,7 @@ int QUICK_GROUP_MIN_MAX_SELECT::get_next()
                                       HA_READ_KEY_EXACT);
 
     result= have_min ? min_res : have_max ? max_res : result;
-  } while ((result == HA_ERR_KEY_NOT_FOUND || result == HA_ERR_END_OF_FILE) &&
+  } while (result == HA_ERR_KEY_NOT_FOUND || result == HA_ERR_END_OF_FILE);
-           is_last_prefix != 0);
 
   if (result == HA_ERR_KEY_NOT_FOUND)
     result= HA_ERR_END_OF_FILE;

sql/opt_range.h

@@ -1527,7 +1527,6 @@ private:
   uchar *group_prefix;    /* Key prefix consisting of the GROUP fields. */
   const uint group_prefix_len; /* Length of the group prefix. */
   uint group_key_parts;  /* A number of keyparts in the group prefix */
-  uchar *last_prefix;     /* Prefix of the last group for detecting EOF. */
   bool have_min;         /* Specify whether we are computing */
   bool have_max;         /*   a MIN, a MAX, or both.         */
   bool have_agg_distinct;/*   aggregate_function(DISTINCT ...).  */