Merge branch 'mysql-8.4' into mysql-trunk

Change-Id: If097734470881a8b1f0fbaa74e4fee0ca2a31888

Author: Nimita Joshi

mysql-test/suite/json/r/array_index.result

@@ -3426,10 +3426,69 @@ test.t	analyze	status	OK
 EXPLAIN FORMAT = tree SELECT * FROM t WHERE doc_id >= 3 and doc_id < 4;
 EXPLAIN
 -> Filter: ((t.doc_id >= 3) and (t.doc_id < 4))  (rows=1)
-    -> Index range scan (Multi-Range Read) on t using mv_doc_id over (3 <= doc_id < 4)  (rows=1)
+    -> Index range scan on t using mv_doc_id over (3 <= doc_id < 4)  (rows=1)
 
 SELECT * FROM t WHERE doc_id >= 3 and doc_id < 4;
 id	doc_id	doc
 6	3	{"val": [1]}
 DROP TABLE t;
 # restart:
+#
+# Bug#33334911 - Multi-valued index performance is too slow
+# (Access too many rows than required)
+#
+CREATE TABLE t1 (
+id INT NOT NULL,
+f1 enum('T1','T2','T3') NOT NULL,
+f2 INT NOT NULL,
+json_value json DEFAULT (json_array()),
+PRIMARY KEY (id),
+KEY idx (f1 ,(CAST(json_extract(json_value,_utf8mb4'$') AS UNSIGNED ARRAY)), f2)
+);
+INSERT INTO t1 VALUES (1, 'T2', 15, '[1, 3, 5, 7]');
+INSERT INTO t1 VALUES (2, 'T2', 25, '[10, 20, 30, 40]');
+INSERT INTO t1 VALUES (3, 'T2', 35, '[15, 25, 35, 45]');
+INSERT INTO t1 VALUES (4, 'T2', 45, '[100, 200, 300, 400]');
+INSERT INTO t1 VALUES (5, 'T2', 55, '[120, 220, 320, 420]');
+INSERT INTO t1 VALUES (6, 'T2', 65, '[140, 240, 340, 440]');
+INSERT INTO t1 VALUES (7, 'T2', 9, '[100, 200, 300, 400]');
+INSERT INTO t1 VALUES (8, 'T2', 9, '[100, 200, 300, 400]');
+INSERT INTO t1 VALUES (9, 'T2', 9, '[100, 200, 300, 400]');
+INSERT INTO t1 VALUES (10, 'T2', 9, '[100, 200, 300, 400]');
+ANALYZE TABLE t1;
+Table	Op	Msg_type	Msg_text
+test.t1	analyze	status	OK
+EXPLAIN SELECT COUNT(*) FROM t1
+WHERE f1 IN ('T2') AND JSON_OVERLAPS(json_value->'$', CAST('[100]' AS JSON)) AND f2>=10;
+id	select_type	table	partitions	type	possible_keys	key	key_len	ref	rows	filtered	Extra
+1	SIMPLE	t1	NULL	range	idx	idx	14	NULL	1	100.00	Using where
+Warnings:
+Note	1003	/* select#1 */ select count(0) AS `COUNT(*)` from `test`.`t1` where ((`test`.`t1`.`f1` = 'T2') and json_overlaps(cast(json_extract(`json_value`,_utf8mb4'$') as unsigned array),json'[100]') and (`test`.`t1`.`f2` >= 10))
+FLUSH STATUS;
+SELECT COUNT(*) FROM t1
+WHERE f1 IN ('T2') AND JSON_OVERLAPS(json_value->'$', CAST('[100]' AS JSON)) AND f2>=10;
+COUNT(*)
+1
+SHOW STATUS LIKE 'Handler_read%';
+Variable_name	Value
+Handler_read_first	0
+Handler_read_key	1
+Handler_read_last	0
+Handler_read_next	1
+Handler_read_prev	0
+Handler_read_rnd	0
+Handler_read_rnd_next	0
+SET @save_opt=@@optimizer_switch;
+SET @@optimizer_switch="mrr_cost_based=off";
+EXPLAIN SELECT COUNT(*) FROM t1
+WHERE f1 IN ('T2') AND JSON_OVERLAPS(json_value->'$', CAST('[100]' AS JSON)) AND f2>=10;
+id	select_type	table	partitions	type	possible_keys	key	key_len	ref	rows	filtered	Extra
+1	SIMPLE	t1	NULL	range	idx	idx	14	NULL	1	100.00	Using where
+Warnings:
+Note	1003	/* select#1 */ select count(0) AS `COUNT(*)` from `test`.`t1` where ((`test`.`t1`.`f1` = 'T2') and json_overlaps(cast(json_extract(`json_value`,_utf8mb4'$') as unsigned array),json'[100]') and (`test`.`t1`.`f2` >= 10))
+SELECT COUNT(*) FROM t1
+WHERE f1 IN ('T2') AND JSON_OVERLAPS(json_value->'$', CAST('[100]' AS JSON)) AND f2>=10;
+COUNT(*)
+1
+SET @@optimizer_switch=@save_opt;
+DROP TABLE t1;

mysql-test/suite/json/t/array_index.test

@@ -2033,3 +2033,48 @@ DROP TABLE t;
 # at the very end of the test.
 --let $restart_parameters= restart:
 --source include/restart_mysqld.inc
+
+--echo #
+--echo # Bug#33334911 - Multi-valued index performance is too slow
+--echo # (Access too many rows than required)
+--echo #
+
+CREATE TABLE t1 (
+ id INT NOT NULL,
+ f1 enum('T1','T2','T3') NOT NULL,
+ f2 INT NOT NULL,
+ json_value json DEFAULT (json_array()),
+ PRIMARY KEY (id),
+ KEY idx (f1 ,(CAST(json_extract(json_value,_utf8mb4'$') AS UNSIGNED ARRAY)), f2)
+);
+
+INSERT INTO t1 VALUES (1, 'T2', 15, '[1, 3, 5, 7]');
+INSERT INTO t1 VALUES (2, 'T2', 25, '[10, 20, 30, 40]');
+INSERT INTO t1 VALUES (3, 'T2', 35, '[15, 25, 35, 45]');
+INSERT INTO t1 VALUES (4, 'T2', 45, '[100, 200, 300, 400]');
+INSERT INTO t1 VALUES (5, 'T2', 55, '[120, 220, 320, 420]');
+INSERT INTO t1 VALUES (6, 'T2', 65, '[140, 240, 340, 440]');
+INSERT INTO t1 VALUES (7, 'T2', 9, '[100, 200, 300, 400]');
+INSERT INTO t1 VALUES (8, 'T2', 9, '[100, 200, 300, 400]');
+INSERT INTO t1 VALUES (9, 'T2', 9, '[100, 200, 300, 400]');
+INSERT INTO t1 VALUES (10, 'T2', 9, '[100, 200, 300, 400]');
+
+ANALYZE TABLE t1;
+
+let $query = SELECT COUNT(*) FROM t1
+  WHERE f1 IN ('T2') AND JSON_OVERLAPS(json_value->'$', CAST('[100]' AS JSON)) AND f2>=10;
+
+--eval EXPLAIN $query
+
+FLUSH STATUS;
+--eval $query
+SHOW STATUS LIKE 'Handler_read%';
+
+SET @save_opt=@@optimizer_switch;
+SET @@optimizer_switch="mrr_cost_based=off";
+
+--eval EXPLAIN $query
+--eval $query
+
+SET @@optimizer_switch=@save_opt;
+DROP TABLE t1;

sql/field.cc

@@ -10311,12 +10311,12 @@ int Field_typed_array::key_cmp(const uchar *key_ptr, uint key_length) const {
   m_conv_item->field->set_key_image(key_ptr, key_length);
   if (sql_scalar_to_json(m_conv_item, "<Field_typed_array::key_cmp>", &value,
                          &tmp, &key, nullptr, true)) {
-    return -1;
+    return 1;
   }
 
   // Compare colum value with the key.
   if (val_json(&col_val)) {
-    return -1;
+    return 1;
   }
   assert(col_val.type() == enum_json_type::J_ARRAY);
   for (uint i = 0; i < col_val.length(); i++) {
@@ -10325,7 +10325,7 @@ int Field_typed_array::key_cmp(const uchar *key_ptr, uint key_length) const {
       return 0;
     }
   }
-  return -1;
+  return 1;
 }
 
 void Field_typed_array::init(TABLE *table_arg) {

sql/field.h

@@ -4307,21 +4307,23 @@ class Field_typed_array final : public Field_json {
   int key_cmp(const uchar *, const uchar *) const override { return -1; }
   /**
    * @brief This function will behave similarly to MEMBER OF json operation,
-   *        unlike regular key_cmp. The key value will be checked against
-   *        members of the array and the presence of the key will be considered
-   *        as the record matching the given key. This particular definition is
-   *        used in descending ref index scans. Descending index scan uses
-   *        handler::ha_index_prev() function to read from the storage engine
-   *        which does not compare the index key with the search key [unlike
-   *        handler::ha_index_next_same()]. Hence each retrieved record needs
-   *        to be validated to find a stop point. Refer key_cmp_if_same() and
-   *        RefIterator<true>::Read() for more details.
+   *        unlike regular key_cmp. Since scans on multi-valued indexes always
+   *        go in the ascending direction, and always start on the first entry
+   *        that is not less than the key, a record not matching the MEMBER OF
+   *        condition is assumed to be greater than the key, so the function
+   *        always returns 1, indicating greater than, for not found.
+   *        This definition is used in descending ref index scans.
+   *        Descending index scan uses handler::ha_index_prev() function to read
+   *        from the storage engine which does not compare the index key with
+   *        the search key [unlike handler::ha_index_next_same()]. Hence each
+   *        retrieved record needs to be validated to find a stop point. Refer
+   *        key_cmp_if_same() and RefIterator<true>::Read() for more details.
    *
    * @param   key_ptr         Pointer to the key
    * @param   key_length      Key length
    * @return
    *      0   Key found in the record
-   *      -1  Key not found in the record
+   *      1   Key not found in the record
    */
   int key_cmp(const uchar *key_ptr, uint key_length) const override;
   /**

sql/handler.cc

@@ -6269,6 +6269,7 @@ static bool key_uses_partial_cols(TABLE *table, uint keyno) {
                          OUT: Size of the buffer that is expected to be actually
                               used, or 0 if buffer is not needed.
   @param [in,out] flags  A combination of HA_MRR_* flags
+  @param [out] force_default_mrr Force default MRR implementation
   @param [out] cost      Estimated cost of MRR access
 
   @note
@@ -6289,12 +6290,14 @@ ha_rows handler::multi_range_read_info_const(uint keyno, RANGE_SEQ_IF *seq,
                                              void *seq_init_param,
                                              uint n_ranges_arg [[maybe_unused]],
                                              uint *bufsz, uint *flags,
+                                             bool *force_default_mrr,
                                              Cost_estimate *cost) {
   KEY_MULTI_RANGE range;
   range_seq_t seq_it;
   ha_rows rows, total_rows = 0;
   uint n_ranges = 0;
   THD *thd = current_thd;
+  *force_default_mrr = false;
 
   /* Default MRR implementation doesn't need buffer */
   *bufsz = 0;
@@ -6315,6 +6318,17 @@ ha_rows handler::multi_range_read_info_const(uint keyno, RANGE_SEQ_IF *seq,
       max_endp = range.end_key.length ? &range.end_key : nullptr;
     }
 
+    /*
+      Allow multi-valued index for DS-MRR only for equality ranges.
+      For non-equality ranges, the storage engine might need to call
+      Field_typed_array::key_cmp(), which is not safe when doing an
+      index-only scan.
+    */
+    if (!*force_default_mrr &&
+        (table->key_info[keyno].flags & HA_MULTI_VALUED_KEY) &&
+        !(range.range_flag & EQ_RANGE)) {
+      *force_default_mrr = true;
+    }
     /*
       Return HA_POS_ERROR if the specified keyno is not capable of
       serving the specified range request. The cases checked for are:
@@ -7016,10 +7030,12 @@ ha_rows DsMrr_impl::dsmrr_info_const(uint keyno, RANGE_SEQ_IF *seq,
   ha_rows rows;
   uint def_flags = *flags;
   uint def_bufsz = *bufsz;
+  bool force_default_mrr = false;
 
   /* Get cost/flags/mem_usage of default MRR implementation */
   rows = h->handler::multi_range_read_info_const(
-      keyno, seq, seq_init_param, n_ranges, &def_bufsz, &def_flags, cost);
+      keyno, seq, seq_init_param, n_ranges, &def_bufsz, &def_flags,
+      &force_default_mrr, cost);
   if (rows == HA_POS_ERROR) {
     /* Default implementation can't perform MRR scan => we can't either */
     return rows;
@@ -7028,10 +7044,12 @@ ha_rows DsMrr_impl::dsmrr_info_const(uint keyno, RANGE_SEQ_IF *seq,
   /*
     If HA_MRR_USE_DEFAULT_IMPL has been passed to us, that is an order to
     use the default MRR implementation (we need it for UPDATE/DELETE).
-    Otherwise, make a choice based on cost and mrr* flags of
-    @@optimizer_switch.
+    Also, if multi_range_read_info_const() detected that "DS_MRR" cannot
+    be used (E.g. Using a multi-valued index for non-equality ranges), we
+    are mandated to use the default implementation. Else, make a choice
+    based on cost and mrr* flags of @@optimizer_switch.
   */
-  if ((*flags & HA_MRR_USE_DEFAULT_IMPL) ||
+  if ((*flags & HA_MRR_USE_DEFAULT_IMPL) || force_default_mrr ||
       choose_mrr_impl(keyno, rows, flags, bufsz, cost)) {
     DBUG_PRINT("info", ("Default MRR implementation choosen"));
     *flags = def_flags;
@@ -7583,14 +7601,19 @@ void handler::set_end_range(const key_range *range,
 int handler::compare_key(key_range *range) {
   int cmp = -1;
   if (!range || in_range_check_pushed_down) return 0;  // No max range
-  /*
-    Virtual fields are not updated during multi-valued index read in MRR.
-    Hence key comparison is skipped for MV index.
-    TODO: Disable MRR on MV index or implement a comparison logic.
-  */
-  if (!(table->key_info[active_index].flags & HA_MULTI_VALUED_KEY)) {
-    cmp = key_cmp(range_key_part, range->key, range->length);
+
+  if ((table->key_info[active_index].flags & HA_MULTI_VALUED_KEY) &&
+      table->key_read) {
+    // For multi-valued indexes, key_cmp() needs to read the virtual column
+    // backing the index. See Field_typed_array::key_cmp(). The virtual column
+    // is not available during index-only scans (typically used by DS-MRR), so
+    // skip the end of range scan in that case, and let the SQL layer do the
+    // filtering. Assuming the scan is ascending, returning -1 (less than range)
+    // makes the scan return the row to the next layer.
+    assert(range_scan_direction == RANGE_SCAN_ASC);
+    return -1;
   }
+  cmp = key_cmp(range_key_part, range->key, range->length);
   if (!cmp) cmp = key_compare_result_on_equal;
   return cmp;
 }

sql/handler.h

@@ -5454,10 +5454,9 @@ class handler {
   double estimate_in_memory_buffer(ulonglong table_index_size) const;
 
  public:
-  virtual ha_rows multi_range_read_info_const(uint keyno, RANGE_SEQ_IF *seq,
-                                              void *seq_init_param,
-                                              uint n_ranges, uint *bufsz,
-                                              uint *flags, Cost_estimate *cost);
+  virtual ha_rows multi_range_read_info_const(
+      uint keyno, RANGE_SEQ_IF *seq, void *seq_init_param, uint n_ranges,
+      uint *bufsz, uint *flags, bool *force_default_mrr, Cost_estimate *cost);
   virtual ha_rows multi_range_read_info(uint keyno, uint n_ranges, uint keys,
                                         uint *bufsz, uint *flags,
                                         Cost_estimate *cost);

sql/range_optimizer/index_range_scan_plan.cc

@@ -624,8 +624,10 @@ ha_rows check_quick_select(THD *thd, RANGE_OPT_PARAM *param, uint idx,
 
   *bufsize = thd->variables.read_rnd_buff_size;
   // Sets is_ror_scan to false for some queries, e.g. multi-ranges
+  bool force_default_mrr = false;
   rows = file->multi_range_read_info_const(keynr, &seq_if, (void *)&seq, 0,
-                                           bufsize, mrr_flags, cost);
+                                           bufsize, mrr_flags,
+                                           &force_default_mrr, cost);
   if (rows != HA_POS_ERROR) {
     param->table->quick_rows[keynr] = rows;
     if (update_tbl_stats) {

storage/innobase/handler/ha_innodb.cc

@@ -23744,11 +23744,10 @@ int ha_innobase::multi_range_read_next(char **range_info) {
   return (m_ds_mrr.dsmrr_next(range_info));
 }
 
-ha_rows ha_innobase::multi_range_read_info_const(uint keyno, RANGE_SEQ_IF *seq,
-                                                 void *seq_init_param,
-                                                 uint n_ranges, uint *bufsz,
-                                                 uint *flags,
-                                                 Cost_estimate *cost) {
+ha_rows ha_innobase::multi_range_read_info_const(
+    uint keyno, RANGE_SEQ_IF *seq, void *seq_init_param, uint n_ranges,
+    uint *bufsz, uint *flags, bool *force_default_mrr [[maybe_unused]],
+    Cost_estimate *cost) {
   /* See comments in ha_myisam::multi_range_read_info_const */
   m_ds_mrr.init(table);
 

storage/innobase/handler/ha_innodb.h

@@ -564,6 +564,7 @@ class ha_innobase : public handler {
   ha_rows multi_range_read_info_const(uint keyno, RANGE_SEQ_IF *seq,
                                       void *seq_init_param, uint n_ranges,
                                       uint *bufsz, uint *flags,
+                                      bool *force_default_mrr,
                                       Cost_estimate *cost) override;
 
   /** Initialize multi range read and get information.

storage/myisam/ha_myisam.cc

@@ -2025,11 +2025,10 @@ int ha_myisam::multi_range_read_next(char **range_info) {
   return ds_mrr.dsmrr_next(range_info);
 }
 
-ha_rows ha_myisam::multi_range_read_info_const(uint keyno, RANGE_SEQ_IF *seq,
-                                               void *seq_init_param,
-                                               uint n_ranges, uint *bufsz,
-                                               uint *flags,
-                                               Cost_estimate *cost) {
+ha_rows ha_myisam::multi_range_read_info_const(
+    uint keyno, RANGE_SEQ_IF *seq, void *seq_init_param, uint n_ranges,
+    uint *bufsz, uint *flags, bool *force_default_mrr [[maybe_unused]],
+    Cost_estimate *cost) {
   /*
     This call is here because there is no location where this->table would
     already be known.

storage/myisam/ha_myisam.h

@@ -196,6 +196,7 @@ class ha_myisam : public handler {
   ha_rows multi_range_read_info_const(uint keyno, RANGE_SEQ_IF *seq,
                                       void *seq_init_param, uint n_ranges,
                                       uint *bufsz, uint *flags,
+                                      bool *force_default_mrr,
                                       Cost_estimate *cost) override;
   ha_rows multi_range_read_info(uint keyno, uint n_ranges, uint keys,
                                 uint *bufsz, uint *flags,

storage/ndb/plugin/ha_ndbcluster.cc

@@ -13304,27 +13304,31 @@ static bool read_multi_needs_scan(NDB_INDEX_TYPE cur_index_type,
 
 ha_rows ha_ndbcluster::multi_range_read_info_const(
     uint keyno, RANGE_SEQ_IF *seq, void *seq_init_param, uint n_ranges,
-    uint *bufsz, uint *flags, Cost_estimate *cost) {
+    uint *bufsz, uint *flags, bool *force_default_mrr, Cost_estimate *cost) {
   ha_rows rows;
   uint def_flags = *flags;
   uint def_bufsz = *bufsz;
 
   DBUG_TRACE;
 
   /* Get cost/flags/mem_usage of default MRR implementation */
-  rows = handler::multi_range_read_info_const(
-      keyno, seq, seq_init_param, n_ranges, &def_bufsz, &def_flags, cost);
+  rows = handler::multi_range_read_info_const(keyno, seq, seq_init_param,
+                                              n_ranges, &def_bufsz, &def_flags,
+                                              force_default_mrr, cost);
   if (unlikely(rows == HA_POS_ERROR)) {
     return rows;
   }
 
   /*
     If HA_MRR_USE_DEFAULT_IMPL has been passed to us, that is
     an order to use the default MRR implementation.
-    Otherwise, make a choice based on requested *flags, handler
-    capabilities, cost and mrr* flags of @@optimizer_switch.
+    Also, if multi_range_read_info_const() detected that "DS_MRR" cannot
+    be used (E.g. Using a multi-valued index for non-equality ranges), we
+    are mandated to use the default implementation. Else, make a choice
+    based on requested *flags, handler capabilities, cost and mrr* flags
+    of @@optimizer_switch.
   */
-  if ((*flags & HA_MRR_USE_DEFAULT_IMPL) ||
+  if ((*flags & HA_MRR_USE_DEFAULT_IMPL) || *force_default_mrr ||
       choose_mrr_impl(keyno, n_ranges, rows, bufsz, flags, cost)) {
     DBUG_PRINT("info", ("Default MRR implementation choosen"));
     *flags = def_flags;

storage/ndb/plugin/ha_ndbcluster.h

@@ -187,6 +187,7 @@ class ha_ndbcluster : public handler, public Partition_handler {
   ha_rows multi_range_read_info_const(uint keyno, RANGE_SEQ_IF *seq,
                                       void *seq_init_param, uint n_ranges,
                                       uint *bufsz, uint *flags,
+                                      bool *force_default_mrr,
                                       Cost_estimate *cost) override;
   ha_rows multi_range_read_info(uint keyno, uint n_ranges, uint keys,
                                 uint *bufsz, uint *flags,