Bug#33472935: dict_stats_analyze_index() may always discard optimal

index-based access paths Post-push fix: Making sure that during calculation of transient statistics no intentionally deinitilized stats are used. They are deinitilized in dict_stats_deinit. Change-Id: I902442553a2b9dbb6471fc868ba0fde34c92c31d

Author: KrystianWarzocha

mysql-test/suite/innodb/r/innodb_autoextend_table_ddl.result

@@ -416,14 +416,14 @@ WHERE NAME LIKE '%tcopy_part%';
 NAME	FILE_SIZE	AUTOEXTEND_SIZE
 test/tcopy_part	8388608	8388608
 # Exchange a partition
-SELECT PARTITION_NAME, TABLE_ROWS FROM INFORMATION_SCHEMA.PARTITIONS WHERE TABLE_NAME = 'tpart';
+SELECT PARTITION_NAME, TABLE_ROWS FROM INFORMATION_SCHEMA.PARTITIONS WHERE TABLE_NAME = 'tpart' ORDER BY PARTITION_NAME;
 PARTITION_NAME	TABLE_ROWS
 p0	1
 p1	0
 p2	0
 p3	3
 ALTER TABLE tpart EXCHANGE PARTITION p3 WITH TABLE tcopy_part;
-SELECT PARTITION_NAME, TABLE_ROWS FROM INFORMATION_SCHEMA.PARTITIONS WHERE TABLE_NAME = 'tpart';
+SELECT PARTITION_NAME, TABLE_ROWS FROM INFORMATION_SCHEMA.PARTITIONS WHERE TABLE_NAME = 'tpart' ORDER BY PARTITION_NAME;
 PARTITION_NAME	TABLE_ROWS
 p0	0
 p1	0

mysql-test/suite/innodb/t/innodb_autoextend_table_ddl.test

@@ -291,9 +291,9 @@ SELECT NAME, FILE_SIZE, AUTOEXTEND_SIZE FROM information_schema.innodb_tablespac
  WHERE NAME LIKE '%tcopy_part%';
 
 --echo # Exchange a partition
-SELECT PARTITION_NAME, TABLE_ROWS FROM INFORMATION_SCHEMA.PARTITIONS WHERE TABLE_NAME = 'tpart';
+SELECT PARTITION_NAME, TABLE_ROWS FROM INFORMATION_SCHEMA.PARTITIONS WHERE TABLE_NAME = 'tpart' ORDER BY PARTITION_NAME;
 ALTER TABLE tpart EXCHANGE PARTITION p3 WITH TABLE tcopy_part;
-SELECT PARTITION_NAME, TABLE_ROWS FROM INFORMATION_SCHEMA.PARTITIONS WHERE TABLE_NAME = 'tpart';
+SELECT PARTITION_NAME, TABLE_ROWS FROM INFORMATION_SCHEMA.PARTITIONS WHERE TABLE_NAME = 'tpart' ORDER BY PARTITION_NAME;
 
 SELECT COUNT(*) FROM tpart;
 SELECT COUNT(*) FROM tcopy_part;

storage/innobase/btr/btr0cur.cc

@@ -146,22 +146,22 @@ uint btr_cur_limit_optimistic_insert_debug = 0;
 can be released by page reorganize, then it is reorganized */
 #define BTR_CUR_PAGE_REORGANIZE_LIMIT (UNIV_PAGE_SIZE / 32)
 
+#ifndef UNIV_HOTBACKUP
 /** Estimated table level stats from sampled value.
 @param value sampled stats
-@param index index being sampled
+@param n_leaf_pages number of leaf pages of the index
 @param sample number of sampled rows
 @param ext_size external stored data size
 @param not_empty table not empty
 @return estimated table wide stats from sampled value */
-constexpr uint64_t BTR_TABLE_STATS_FROM_SAMPLE(uint64_t value,
- dict_index_t *index,
- uint64_t sample, ulint ext_size,
- ulint not_empty) {
- return (value * index->stat_n_leaf_pages + sample - 1 + ext_size +
- not_empty) /
+static uint64_t BTR_TABLE_STATS_FROM_SAMPLE(uint64_t value,
+ uint64_t n_leaf_pages,
+ uint64_t sample, ulint ext_size,
+ ulint not_empty) {
+ return (value * n_leaf_pages + sample - 1 + ext_size + not_empty) /
  (sample + ext_size);
 }
-#ifndef UNIV_HOTBACKUP
+
 /** Adds path information to the cursor for the current page, for which
 the binary search has been performed.
 @param[in, out] cursor Cursor positioned on a page.
@@ -5450,9 +5450,9 @@ bool btr_estimate_number_of_different_key_vals(
 
  /* It makes no sense to test more pages than are contained
  in the index, thus we lower the number if it is too high */
- if (srv_stats_transient_sample_pages > index->stat_index_size) {
- if (index->stat_index_size > 0) {
- n_sample_pages = index->stat_index_size;
+ if (srv_stats_transient_sample_pages > index_stats->index_size) {
+ if (index_stats->index_size > 0) {
+ n_sample_pages = index_stats->index_size;
  } else {
  n_sample_pages = 1;
  }
@@ -5562,15 +5562,18 @@ bool btr_estimate_number_of_different_key_vals(
 
  /* If we saw k borders between different key values on
  n_sample_pages leaf pages, we can estimate how many
- there will be in index->stat_n_leaf_pages */
+ there will be in index_stats->n_leaf_pages (to be later stored in
+ index->stat_n_leaf_pages) */
 
  /* We must take into account that our sample actually represents
  also the pages used for external storage of fields (those pages are
- included in index->stat_n_leaf_pages) */
+ included in index_stats->n_leaf_pages (to be later stored in
+ index->stat_n_leaf_pages) */
 
  for (j = 0; j < n_cols; j++) {
  index_stats->n_diff_key_vals[j] = BTR_TABLE_STATS_FROM_SAMPLE(
- n_diff[j], index, n_sample_pages, total_external_size, not_empty_flag);
+ n_diff[j], index_stats->n_leaf_pages, n_sample_pages,
+ total_external_size, not_empty_flag);
 
  /* If the tree is small, smaller than
  10 * n_sample_pages + total_external_size, then
@@ -5596,9 +5599,9 @@ bool btr_estimate_number_of_different_key_vals(
  and initialized to zero in dict_index_add_to_cache(),
  along with n_diff_key_vals[] array */
  if (n_not_null != nullptr) {
- index_stats->n_non_null_key_vals[j] =
- BTR_TABLE_STATS_FROM_SAMPLE(n_not_null[j], index, n_sample_pages,
-  total_external_size, not_empty_flag);
+ index_stats->n_non_null_key_vals[j] = BTR_TABLE_STATS_FROM_SAMPLE(
+ n_not_null[j], index_stats->n_leaf_pages, n_sample_pages,
+ total_external_size, not_empty_flag);
  }
  }
 

storage/innobase/dict/dict0mem.cc

@@ -918,9 +918,44 @@ page_size_t dict_index_t::get_page_size() const {
  return (dict_table_page_size(table));
 }
 
+#ifndef UNIV_HOTBACKUP
+
+void dict_index_t::assert_stats_initialized() const {
+ UNIV_MEM_ASSERT_RW_ABORT(stat_n_diff_key_vals,
+ n_uniq * sizeof(stat_n_diff_key_vals[0]));
+
+ UNIV_MEM_ASSERT_RW_ABORT(stat_n_sample_sizes,
+ n_uniq * sizeof(stat_n_sample_sizes[0]));
+
+ UNIV_MEM_ASSERT_RW_ABORT(stat_n_non_null_key_vals,
+ n_uniq * sizeof(stat_n_non_null_key_vals[0]));
+
+ UNIV_MEM_ASSERT_RW_ABORT(&stat_index_size, sizeof(stat_index_size));
+
+ UNIV_MEM_ASSERT_RW_ABORT(&stat_n_leaf_pages, sizeof(stat_n_leaf_pages));
+}
+
+#endif /* !UNIV_HOTBACKUP */
+
 bool dict_table_t::has_pk() const {
  const dict_index_t *first = first_index();
  const size_t len = strlen(innobase_index_reserve_name);
  const int cmp = strncmp(innobase_index_reserve_name, first->name(), len);
  return cmp != 0;
 }
+
+#ifndef UNIV_HOTBACKUP
+void dict_index_stats_t::assert_initialized() const {
+ UNIV_MEM_ASSERT_RW_ABORT(n_diff_key_vals,
+ n_uniq * sizeof(n_diff_key_vals[0]));
+
+ UNIV_MEM_ASSERT_RW_ABORT(n_sample_sizes, n_uniq * sizeof(n_sample_sizes[0]));
+
+ UNIV_MEM_ASSERT_RW_ABORT(n_non_null_key_vals,
+ n_uniq * sizeof(n_non_null_key_vals[0]));
+
+ UNIV_MEM_ASSERT_RW_ABORT(&index_size, sizeof(index_size));
+
+ UNIV_MEM_ASSERT_RW_ABORT(&n_leaf_pages, sizeof(n_leaf_pages));
+}
+#endif /* !UNIV_HOTBACKUP */

storage/innobase/dict/dict0stats.cc

@@ -442,30 +442,6 @@ static void dict_stats_empty_table(dict_table_t *table) /*!< in/out: table */
  dict_table_stats_unlock(table, RW_X_LATCH);
 }
 
-/** Check whether index's stats are initialized (assert if they are not). */
-template <typename T>
-static void dict_stats_assert_initialized_index(
- const T *index [[maybe_unused]]) /*!< in: index */
-{
- UNIV_MEM_ASSERT_RW_ABORT(
- index->stat_n_diff_key_vals,
- index->n_uniq * sizeof(index->stat_n_diff_key_vals[0]));
-
- UNIV_MEM_ASSERT_RW_ABORT(
- index->stat_n_sample_sizes,
- index->n_uniq * sizeof(index->stat_n_sample_sizes[0]));
-
- UNIV_MEM_ASSERT_RW_ABORT(
- index->stat_n_non_null_key_vals,
- index->n_uniq * sizeof(index->stat_n_non_null_key_vals[0]));
-
- UNIV_MEM_ASSERT_RW_ABORT(&index->stat_index_size,
- sizeof(index->stat_index_size));
-
- UNIV_MEM_ASSERT_RW_ABORT(&index->stat_n_leaf_pages,
- sizeof(index->stat_n_leaf_pages));
-}
-
 /** Check whether table's stats are initialized (assert if they are not). */
 static void dict_stats_assert_initialized(
  const dict_table_t *table) /*!< in: table */
@@ -500,7 +476,7 @@ static void dict_stats_assert_initialized(
  for (const dict_index_t *index = table->first_index(); index != nullptr;
  index = index->next()) {
  if (!dict_stats_should_ignore_index(index)) {
- dict_stats_assert_initialized_index(index);
+ index->assert_stats_initialized();
  }
  }
 }
@@ -726,8 +702,6 @@ static void dict_stats_copy_index(dict_index_t *dst_idx,
  const size_t n_copy_el = dst_idx->n_uniq;
  ut_a_eq(dst_idx->n_uniq, src_idx_stats->n_uniq);
 
- dict_stats_assert_initialized_index(dst_idx);
-
  memmove(dst_idx->stat_n_diff_key_vals, src_idx_stats->n_diff_key_vals,
  n_copy_el * sizeof(dst_idx->stat_n_diff_key_vals[0]));
 
@@ -746,6 +720,8 @@ static void dict_stats_copy_index(dict_index_t *dst_idx,
  dst_idx->stat_index_size = src_idx_stats->index_size;
 
  dst_idx->stat_n_leaf_pages = src_idx_stats->n_leaf_pages;
+
+ dst_idx->assert_stats_initialized();
 }
 
 /** Calculates new estimates for index statistics. This function is
@@ -1878,7 +1854,6 @@ static bool dict_stats_analyze_index_low(uint64_t &n_sample_pages,
  ulint size;
 
  ut_ad(index_stats);
- dict_stats_assert_initialized_index(index_stats);
 
  DBUG_TRACE;
 
@@ -1987,6 +1962,8 @@ static bool dict_stats_analyze_index_low(uint64_t &n_sample_pages,
 
  mtr_commit(&mtr);
 
+ index_stats->assert_initialized();
+
  return true;
  }
 
@@ -2208,6 +2185,10 @@ static bool dict_stats_analyze_index_low(uint64_t &n_sample_pages,
 
  ut::delete_arr(n_diff_data);
 
+ if (succeeded) {
+ index_stats->assert_initialized();
+ }
+
  return succeeded;
 }
 

storage/innobase/include/dict0mem.h

@@ -1631,6 +1631,11 @@ struct dict_index_t {
  /** Check if it is a full-text search (FTS) index
  @return true if this is a FTS index, false otherwise. */
  bool is_fts_index() const { return type & DICT_FTS; }
+
+#ifndef UNIV_HOTBACKUP
+ /** Check whether index's stats are initialized (assert if they are not).*/
+ void assert_stats_initialized() const;
+#endif /* !UNIV_HOTBACKUP */
 };
 
 /** The status of online index creation */
@@ -3110,6 +3115,7 @@ inline bool dict_table_autoinc_own(const dict_table_t *table) {
 }
 #endif /* UNIV_DEBUG */
 
+#ifndef UNIV_HOTBACKUP
 /* Data structure storing index statistics. Used as temporary state during
 statistics calculation. The final version of statistics for reading by
 optimizer are stored in dict_index_t.*/
@@ -3127,8 +3133,13 @@ struct dict_index_stats_t {
  unsigned type : DICT_IT_BITS;
 
  unsigned n_uniq : 10;
+
+ /** Check whether index's stats are initialized (assert if they are not). */
+ void assert_initialized() const;
 };
 
+#endif /* !UNIV_HOTBACKUP */
+
 #include "dict0mem.ic"
 
 #endif /* dict0mem_h */