Skip to content

MDEV-19574 innodb_stats_method is not honored when innodb_stats_persistent=ON#4204

Open
Thirunarayanan wants to merge 1 commit into
10.11from
10.11-MDEV-19574
Open

MDEV-19574 innodb_stats_method is not honored when innodb_stats_persistent=ON#4204
Thirunarayanan wants to merge 1 commit into
10.11from
10.11-MDEV-19574

Conversation

@Thirunarayanan

@Thirunarayanan Thirunarayanan commented Jul 16, 2025

Copy link
Copy Markdown
Member
  • The Jira issue number for this PR is: MDEV-19574

Description

MDEV-19574: innodb_stats_method ignored with persistent statistics

Problem:

When persistent statistics are enabled (innodb_stats_persistent=ON),
the innodb_stats_method setting was not being properly passed
through the statistics calculation chain. This caused NULL handling
to always use the default behavior, regardless of the
configured stats method.

The issue affected query optimization for queries involving
NULL values, as the statistics collection wasn't respecting
the user's preference for NULL value treatment
(NULLS_EQUAL, NULLS_UNEQUAL, or NULLS_IGNORED).

Solution:

  1. Passed innodb_stats_method parameter through the
    statistics calculation chain:

    • dict_stats_scan_page()
    • dict_stats_analyze_index_below_cur()
    • dict_stats_analyze_index_for_n_prefix()
    • dict_stats_analyze_index_level
    • dict_stats_analyze_index
    • dict_stats_update_persistent
    • dict_stats_save
  2. Conditionally set n_non_null_key_vals based on the NULLS_IGNORED
    method.

  3. Introduced IndexLevelStats which is to collect statistics
    at a specific B-tree level during index analysis

  4. Introduced LeafPageStats which is to collect statistics
    for leaf page analysis

  5. IndexLevelStats, LeafPageStats replaces multiple individual
    parameters in function signatures.

  6. Add stats method name to stat_description in case of non
    default innodb_stats_method variable value

  7. When InnoDB scan the leaf page directly, assign leaf page
    count as number of pages scanned in case of multi-level index.
    For single page indexes, use 1. This change leads to multiple
    changes in existing test case.

Release Notes

Fixed innodb_stats_method being ignored when innodb_stats_persistent=ON, causing
incorrect cardinality estimates for indexes with NULL values. Now it gives more accurate
cardinality estimates for NULL-containing indexes for all three stats method.

How can this PR be tested?

./mtr innodb.stats_method

Basing the PR against the correct MariaDB version

  • This is a new feature or a refactoring, and the PR is based against the main branch.
  • This is a bug fix, and the PR is based against the earliest maintained branch in which the bug can be reproduced.

PR quality check

  • I checked the CODING_STANDARDS.md file and my PR conforms to this where appropriate.
  • For any trivial modifications to the PR, I am ok with the reviewer making the changes themselves.

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Comment thread storage/innobase/include/dict0mem.h Outdated
Comment thread mysql-test/main/mysqldump-system.result Outdated
Comment thread storage/innobase/dict/dict0stats.cc Outdated
@grooverdan

Copy link
Copy Markdown
Member

msan failure is MDEV-28165

@Thirunarayanan
Thirunarayanan force-pushed the 10.11-MDEV-19574 branch 4 times, most recently from a1aa0d8 to ab6c670 Compare July 18, 2025 11:21
Comment thread mysql-test/suite/innodb/r/innodb_stats_method,NULLS_EQUAL.rdiff Outdated
Comment thread storage/innobase/include/ut0new.h Outdated
Comment thread mysql-test/suite/innodb/t/innodb_stats_method.test Outdated
Comment thread storage/innobase/include/dict0stats.h Outdated
Comment thread storage/innobase/include/dict0stats.h Outdated
Comment thread storage/innobase/dict/dict0stats.cc Outdated
Comment thread storage/innobase/dict/dict0stats.cc Outdated
Comment thread storage/innobase/dict/dict0stats.cc Outdated
Comment thread storage/innobase/dict/dict0stats.cc Outdated
Comment thread storage/innobase/dict/dict0stats.cc Outdated
Comment thread storage/innobase/dict/dict0stats.cc Outdated
Comment thread storage/innobase/dict/dict0stats.cc Outdated
Comment thread storage/innobase/handler/ha_innodb.cc
Comment thread storage/innobase/dict/dict0stats.cc Outdated
Comment thread storage/innobase/dict/dict0stats.cc Outdated
Comment thread storage/innobase/dict/dict0stats.cc Outdated
Comment thread storage/innobase/include/dict0stats.h
Comment thread mysql-test/suite/innodb/t/innodb_stats_method.test Outdated
Comment thread mysql-test/suite/innodb/r/innodb_stats_method,NULLS_UNEQUAL.rdiff Outdated
Comment thread mysql-test/suite/innodb/r/innodb_stats_method,NULLS_UNEQUAL.rdiff Outdated
@Thirunarayanan
Thirunarayanan force-pushed the 10.11-MDEV-19574 branch 3 times, most recently from 012accc to 12cd66c Compare September 29, 2025 12:06
Comment thread storage/innobase/dict/dict0stats.cc Outdated
Comment thread storage/innobase/dict/dict0stats.cc Outdated
Comment thread storage/innobase/dict/dict0stats.cc Outdated
Comment thread storage/innobase/dict/dict0stats.cc Outdated
Comment thread storage/innobase/dict/dict0stats.cc Outdated
Comment thread storage/innobase/dict/dict0stats.cc Outdated
Comment thread storage/innobase/dict/dict0stats.cc Outdated
Comment thread storage/innobase/dict/dict0stats.cc Outdated
Comment thread storage/innobase/dict/dict0stats.cc Outdated
Comment on lines +114 to +115
@retval DB_SUCCESS_LOCKED_REC if the table under bulk insert operation
@return DB_SUCCESS or error code */

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is confusing. Usually @return is before any @retval and only @retval names any specific values.

@return error code
@retval DB_SUCCESS            if the statistics were calculated and updated
@retval DB_SUCCESS_LOCKED_REC if the table is being bulk-inserted to

@Thirunarayanan
Thirunarayanan requested a review from dr-m October 14, 2025 13:24
@Thirunarayanan
Thirunarayanan force-pushed the 10.11-MDEV-19574 branch 3 times, most recently from 9b58c4c to 5437204 Compare October 15, 2025 08:18
@dr-m
dr-m removed their request for review October 15, 2025 09:54
@Thirunarayanan
Thirunarayanan force-pushed the 10.11-MDEV-19574 branch 2 times, most recently from a770ed9 to 6ca9ae4 Compare November 25, 2025 07:00

@dr-m dr-m left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Here are some more comments. There are also some earlier comments that have not been addressed.

Comment thread storage/innobase/dict/dict0stats.cc Outdated
Comment thread storage/innobase/dict/dict0stats.cc Outdated
Comment thread storage/innobase/dict/dict0stats.cc Outdated
Comment thread storage/innobase/btr/btr0cur.cc Outdated
Comment thread storage/innobase/dict/dict0stats.cc Outdated
Comment thread storage/innobase/dict/dict0stats.cc Outdated
Comment thread storage/innobase/dict/dict0stats.cc Outdated
Comment thread storage/innobase/dict/dict0stats.cc Outdated
Comment on lines +1406 to +1409
IndexLevelStats(unsigned stats_method, dict_index_t *index) :
level(0), nulls_unequal(stats_method > SRV_STATS_NULLS_EQUAL)
{
n_diff= index->stat_n_diff_key_vals;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why is n_diff not initialized in the initializer list? Why do we not store a pointer to the index instead?

@Thirunarayanan Thirunarayanan Feb 2, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Leaf level constructor: n_diff points directly to index->stat_n_diff_key_vals
Non-leaf constructor: n_diff gets freshly allocated memory that needs cleanup

Only non-leaf levels (level > 0) needs to free the memory.

Comment thread storage/innobase/dict/dict0stats.cc
Comment thread storage/innobase/dict/dict0stats.cc Outdated
@Thirunarayanan
Thirunarayanan requested a review from dr-m February 2, 2026 16:36
@dr-m
dr-m requested a review from Olernov February 3, 2026 14:33
@Thirunarayanan
Thirunarayanan force-pushed the 10.11-MDEV-19574 branch 3 times, most recently from de4da9f to 5da940d Compare February 4, 2026 14:44

@Olernov Olernov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Works in accordance with the requirements. Thank you!

@spetrunia

spetrunia commented Feb 9, 2026

Copy link
Copy Markdown
Member

One interesting effect:

create table t1 (pk int primary key, a int, b int, index(a,b));
insert into t1 select seq,  if (seq< 1000, seq, NULL), floor(seq/10) from seq_1_to_10000;
set global innodb_stats_method=nulls_ignored;
analyze table t1;

create table t0 (a int, b int , c int);
insert into t0 select seq, seq, seq from seq_1_to_10;
explain select * from t0, t1 where t0.a=t1.a;
+------+-------------+-------+------+---------------+------+---------+-----------+------+-------------+
| id   | select_type | table | type | possible_keys | key  | key_len | ref       | rows | Extra       |
+------+-------------+-------+------+---------------+------+---------+-----------+------+-------------+
|    1 | SIMPLE      | t0    | ALL  | NULL          | NULL | NULL    | NULL      | 10   | Using where |
|    1 | SIMPLE      | t1    | ref  | a             | a    | 5       | test.t0.a | 1    | Using index |
+------+-------------+-------+------+---------------+------+---------+-----------+------+-------------+
2 rows in set (0.001 sec)

Ok. lookup using t1.a=... is expected to produce 1 row. This is true.

explain select * from t0, t1 where t0.a=t1.a and t0.b=t1.b;
+------+-------------+-------+------+---------------+------+---------+---------------------+------+-------------+
| id   | select_type | table | type | possible_keys | key  | key_len | ref                 | rows | Extra       |
+------+-------------+-------+------+---------------+------+---------+---------------------+------+-------------+
|    1 | SIMPLE      | t0    | ALL  | NULL          | NULL | NULL    | NULL                | 10   | Using where |
|    1 | SIMPLE      | t1    | ref  | a             | a    | 10      | test.t0.a,test.t0.b | 2    | Using index |
+------+-------------+-------+------+---------------+------+---------+---------------------+------+-------------+
2 rows in set (0.001 sec)

But if we do a lookup on a narrower range, (t1.a, t1.b)= (..., ...) we expect to get two rows...

I didn't yet figure out why this happens. I continue looking.

@dr-m dr-m left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is somewhat hard to review for me, because I am not familiar with this code. We seem to be missing some test coverage of the logic that is related to externally stored columns.

Comment thread storage/innobase/btr/btr0cur.cc Outdated
Comment thread storage/innobase/btr/btr0cur.cc Outdated
Comment thread storage/innobase/btr/btr0cur.cc Outdated
Comment thread storage/innobase/btr/btr0cur.cc Outdated
Comment thread storage/innobase/dict/dict0stats.cc
Comment thread storage/innobase/dict/dict0stats.cc Outdated
Comment on lines 1930 to 1950

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

leaf_stats is being used for both input and output. The @param[out] is misleading. It would be logical for this to be a member function of LeafPageStats. The cursor should be a member of LeafPageStats, as should mtr. Instead of starting a new mini-transaction we could make use of mtr_t::rollback() here.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

for (i = 0; i < n_diff_data->n_leaf_pages_to_analyze; i++) {
    // Navigate using pcur with caller's mtr
    while (rec_idx < dive_below_idx && btr_pcur_is_on_user_rec(&pcur)) {
        btr_pcur_move_to_next_user_rec(&pcur, mtr);  // Uses caller's mtr
    }
    
    /* Create fresh LeafPageStats for each sample */
    LeafPageStats leaf_stats(...);
    
    /* scan_below() creates its OWN mtr to access the lower level pages */
    leaf_stats.scan_below(btr_pcur_get_btr_cur(&pcur));
}

caller mtr holds latches on the mid-level pages for navigation.
scan_below() which uses its own mtr to access level 1 and leaf pages.

btr_cur_t* cur - is being used only one as a entry point to descend the
index. cursor() is never being used during scan or any other function.

Comment thread storage/innobase/dict/dict0stats.cc Outdated
Comment thread storage/innobase/dict/dict0stats.cc Outdated
Comment on lines 1793 to 1794
@param[in] index index of the page
@param[in] page the page to scan

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

These should be derived from LeafPageStats::cur (which should be a data member).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Answered here that why cursor shouldn't be part of LeafPageStats

Comment thread storage/innobase/dict/dict0stats.cc Outdated
Comment on lines 2354 to 2395
if (data->level == 1) {
/* If we know the number of records on level 1, then
this number is the same as the number of pages on
level 0 (leaf). */
n_ordinary_leaf_pages = data->n_recs_on_level;
} else {
/* If we analyzed D ordinary leaf pages and found E
external pages in total linked from those D ordinary
leaf pages, then this means that the ratio
ordinary/external is D/E. Then the ratio ordinary/total
is D / (D + E). Knowing that the total number of pages
is T (including ordinary and external) then we estimate
that the total number of ordinary leaf pages is
T * D / (D + E). */
n_ordinary_leaf_pages
= index_stats.n_leaf_pages
* data->n_leaf_pages_to_analyze
/ (data->n_leaf_pages_to_analyze
+ data->n_external_pages_sum);
}

/* See REF01 for an explanation of the algorithm */
index_stats.stats[n_prefix - 1].n_diff_key_vals
= n_ordinary_leaf_pages

* data->n_diff_on_level
/ data->n_recs_on_level

* data->n_diff_all_analyzed_pages
/ data->n_leaf_pages_to_analyze;

index_stats.stats[n_prefix - 1].n_non_null_key_vals
= n_ordinary_leaf_pages

* data->n_non_null_on_level
/ data->n_recs_on_level

* data->n_non_null_all_analyzed_pages
/ data->n_leaf_pages_to_analyze;

index_stats.stats[n_prefix - 1].n_sample_sizes
= data->n_leaf_pages_to_analyze;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do you have any idea what the REF01 could be, and why the n_ordinary_leaf_pages are being derived as they are? In which sense is the number of pages equal on levels 0 and 1? Usually, we expect each non-leaf B-tree page to have several child pages. The only exception to that is when a ROW_FORMAT=COMPRESSED page cannot be split, and we allow to store only one record per leaf page.

The comments about REF01 were introduced in mysql/mysql-server@bb3cf12. The commit message does not reveal anything, and there does not seem to be any public bug report or test case related to this change.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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


The statistics calculation uses a two-phase sampling approach to estimate index cardinality efficiently.

Level sampling phase:
=====================
 - level sampling descends the B-tree from the root, analyzing each non-leaf level until
1) stopping at either level 1 (the first non-leaf level above leaves) (or)
2) a higher level containing at least N_DIFF_REQUIRED distinct keys (typically 10× the sample size).

InnoDB never scans level 0 (leaf pages) directly during this phase
because it may contain millions of pages. At each analyzed level, it counts
total records (n_recs_on_level) and distinct key values (n_diff_on_level) to
establish the distinctness ratio (R = n_diff_on_level / n_recs_on_level)

The descent stops at level 1 when possible because each record on level 1
is a node pointer to exactly one leaf page (Here n_recs_on_level equals the total number of leaf pages)

leaf sampling phase:
====================
Leaf sampling selects N_SAMPLE_PAGES non-boring records from the stopped level
and dives below each to analyze the corresponding leaf pages,
counting distinct keys per page and averaging them as N_DIFF_AVG_LEAF.

Let's come to the REF01 formula:
The REF01 formula is N × R × N_DIFF_AVG_LEAF :

It multiplies the total leaf pages (N) by the distinctness ratio from the sampled level (R) by
the average distinct keys per sampled leaf (N_DIFF_AVG_LEAF).

This works because the ratio R measured at the sampled level approximates the same ratio
at the leaf level due to B-tree structure—keys are distributed similarly across levels.

When stopped at level 1, the calculation simplifies since N = n_recs_on_level directly,
eliminating the need to estimate total leaf pages separately.

when level > 1:
===============
For higher levels, the algorithm adjusts N by accounting for the ratio of
ordinary to external BLOB pages found during sampling.

I had to understand why we sample only non-boring records on leaf pages:
====================================================================
We dive below only non-boring records because the distinctness ratio
R = n_diff_on_level / n_recs_on_level measured at the sampled non-leaf level
already accounts for boring records. Pages below all this boring record on 
level l does contribute nothing to cardinality

Formula N × R × N_DIFF_AVG_LEAF automatically compensates:
the ratio R is already reduced by the presence of boring records at the sampled level.

Regarding row_format=compressed:
==================================
The 1:1 relationship between level-1 records and level-0 pages always holds
ROW_FORMAT=COMPRESSED can cause degenerate leaf pages (one record per page)
But this doesn't change the fact that each node pointer on level 1 still points
to exactly one page on level 0.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think I understood the original intention of the comment. There are two comments in this file, which apparently are supposed to refer to each other, by the following:

/* Sampling algorithm description @{

See REF01 for the implementation.

@} */

and in the function dict_stats_index_set_n_diff():

		/* See REF01 for an explanation of the algorithm */

The first comment probably used to immediately precede the definition of the macro N_SAMPLE_PAGES. Currently, there are some unrelated definitions in between.

I think that a more appropriate place for the "Sampling algorithm description" comment would be at the start of the file. There is also another comment:

/* @{ Pseudo code about the relation between the following functions

Why do we have two comments for describing the algorithm? Can we merge them and push down as much detail to the function-level comments as possible? For example, move the definition of N-prefix-boring record to the comment of PageStats::n_diff, and move some of the high-level description to a comment of the function dict_stats_analyze_index().

One of the comments is referring to N_SAMPLE_PAGES by A while the other one uses N, which is being used for a different purpose in the other comment. Consistent naming and less renaming would make it more understandable.

In the "Sampling algorithm description" comment, there is some confusing wording too:

Let the total number of leaf pages in the table be T.

We are collecting statistics for each index separately, right? Why should the total number of leaf pages in the table (the sum of total number of all indexes of the table) matter? Are BLOB pages be counted as leaf pages here? They are being attached to BTR_SEG_LEAF just like actual leaf pages are.

@Thirunarayanan
Thirunarayanan force-pushed the 10.11-MDEV-19574 branch 4 times, most recently from 196f4e4 to c572b75 Compare March 26, 2026 12:49
@Thirunarayanan
Thirunarayanan requested a review from dr-m March 26, 2026 13:37

@dr-m dr-m left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sorry, I haven’t been able to spend time on this. I appreciated your previous comment that explains the REF01. I think that while implementing this, we should try to improve the state of the comments, to make this logic more understandable and maintainable.

Comment thread storage/innobase/dict/dict0stats.cc Outdated
Comment thread storage/innobase/btr/btr0cur.cc Outdated
Comment thread storage/innobase/dict/dict0stats.cc Outdated
Comment thread storage/innobase/dict/dict0stats.cc Outdated
Comment on lines 2354 to 2395
if (data->level == 1) {
/* If we know the number of records on level 1, then
this number is the same as the number of pages on
level 0 (leaf). */
n_ordinary_leaf_pages = data->n_recs_on_level;
} else {
/* If we analyzed D ordinary leaf pages and found E
external pages in total linked from those D ordinary
leaf pages, then this means that the ratio
ordinary/external is D/E. Then the ratio ordinary/total
is D / (D + E). Knowing that the total number of pages
is T (including ordinary and external) then we estimate
that the total number of ordinary leaf pages is
T * D / (D + E). */
n_ordinary_leaf_pages
= index_stats.n_leaf_pages
* data->n_leaf_pages_to_analyze
/ (data->n_leaf_pages_to_analyze
+ data->n_external_pages_sum);
}

/* See REF01 for an explanation of the algorithm */
index_stats.stats[n_prefix - 1].n_diff_key_vals
= n_ordinary_leaf_pages

* data->n_diff_on_level
/ data->n_recs_on_level

* data->n_diff_all_analyzed_pages
/ data->n_leaf_pages_to_analyze;

index_stats.stats[n_prefix - 1].n_non_null_key_vals
= n_ordinary_leaf_pages

* data->n_non_null_on_level
/ data->n_recs_on_level

* data->n_non_null_all_analyzed_pages
/ data->n_leaf_pages_to_analyze;

index_stats.stats[n_prefix - 1].n_sample_sizes
= data->n_leaf_pages_to_analyze;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think I understood the original intention of the comment. There are two comments in this file, which apparently are supposed to refer to each other, by the following:

/* Sampling algorithm description @{

See REF01 for the implementation.

@} */

and in the function dict_stats_index_set_n_diff():

		/* See REF01 for an explanation of the algorithm */

The first comment probably used to immediately precede the definition of the macro N_SAMPLE_PAGES. Currently, there are some unrelated definitions in between.

I think that a more appropriate place for the "Sampling algorithm description" comment would be at the start of the file. There is also another comment:

/* @{ Pseudo code about the relation between the following functions

Why do we have two comments for describing the algorithm? Can we merge them and push down as much detail to the function-level comments as possible? For example, move the definition of N-prefix-boring record to the comment of PageStats::n_diff, and move some of the high-level description to a comment of the function dict_stats_analyze_index().

One of the comments is referring to N_SAMPLE_PAGES by A while the other one uses N, which is being used for a different purpose in the other comment. Consistent naming and less renaming would make it more understandable.

In the "Sampling algorithm description" comment, there is some confusing wording too:

Let the total number of leaf pages in the table be T.

We are collecting statistics for each index separately, right? Why should the total number of leaf pages in the table (the sum of total number of all indexes of the table) matter? Are BLOB pages be counted as leaf pages here? They are being attached to BTR_SEG_LEAF just like actual leaf pages are.

Comment thread storage/innobase/dict/dict0stats.cc
Comment thread storage/innobase/dict/dict0stats.cc Outdated
Comment thread storage/innobase/dict/dict0stats.cc Outdated
Comment thread storage/innobase/dict/dict0stats.cc Outdated
Comment thread storage/innobase/dict/dict0stats.cc Outdated
Comment thread storage/innobase/dict/dict0stats.cc Outdated
…istent=ON

Problem:
=======
When persistent statistics are enabled (innodb_stats_persistent=ON),
the innodb_stats_method setting is not properly utilized during
statistics calculation.

The statistics collection functions always use a hardcoded default
behavior for NULL value comparison instead of respecting the
configured stats method (NULLS_EQUAL, NULLS_UNEQUAL, or
NULLS_IGNORED). This affects the accuracy of n_diff_key_vals
(distinct key count) and n_non_null_key_val estimates, particularly
for indexes with nullable columns containing NULL values. This
impacts the query optimizer, which makes decisions based on
inaccurate cardinality estimates.

Solution:
========
Introduced IndexLevelStats to collect statistics at a specific
B-tree level during index analysis.

Introduced PageStats to collect statistics for leaf page analysis.

Refactored the following functions:
dict_stats_analyze_index_level() to IndexLevelStats::analyze_level()
dict_stats_analyze_index_for_n_prefix() to IndexLevelStats::sample_leaf_pages()
dict_stats_analyze_index_below_cur() to PageStats::scan_below()
dict_stats_scan_page() to PageStats::scan()

Add the stats method name to stat_description when innodb_stats_method
has a non-default value.

Added the new stat name n_nonnull_fld01, n_nonnull_fld02, etc. with a
stats description, to indicate how many non-null values exist for the
nth field of the index. This value is retrieved and stored in the index
statistics in dict_stats_fetch_index_stats_step().

rec_get_n_blob_pages(): Calculate the number of externally stored pages
for a record. It uses ceiling division with the actual usable blob page
space (blob_part_size) and now correctly handles both compressed and
uncompressed table formats for accurate BLOB page counting.

When InnoDB scans the leaf page directly, assign the leaf page count as
the number of pages scanned for a multi-level index. For single-page
indexes, use 1. This change leads to multiple changes in existing
test cases.

Non-null values are only counted at the leaf level, since only leaf
pages hold actual records. Both nullable and NOT NULL columns are
estimated with the same leaf-sampling formula:

  n_ordinary_leaf_pages * (n_non_null_all_analyzed_pages
                           / n_leaf_pages_to_analyze)

For a NOT NULL column every record is counted, so this yields the
estimated record count (no NULLs).

dict_stats_save(): now static function in dict0stats.cc that
takes the innodb_stats_method value, and is removed from dict0stats.h.

Replaced btr_rec_get_externally_stored_len() with rec_get_n_blob_pages()
in dict0stats.cc. btr_rec_get_field_ref_offs() and btr_rec_get_field_ref(),
together with the BTR_BLOB_HDR_* macros, were moved from btr0cur.cc
to btr0cur.h so that rec_get_n_blob_pages()
can reuse them;

btr_rec_get_field_ref_offs() is now a noexcept function returning size_t.

Changed stat_n_diff_key_vals and stat_n_non_null_key_vals from
ib_uint64_t* to uint64_t*

Removed the unused UNIV_STATS_DEBUG build macro (univ.i) and turned the
DEBUG_PRINTF() helper in dict0stats.cc into an unconditional no-op.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes InnoDB persistent statistics so that innodb_stats_method is honored end-to-end when innodb_stats_persistent=ON, improving cardinality estimates (especially for indexes containing NULLs) and updating the persisted stats payload accordingly.

Changes:

  • Threads innodb_stats_method through the persistent stats calculation pipeline, and updates cardinality/non-NULL estimation logic (including external/BLOB-page adjustments).
  • Refactors index analysis internals by introducing per-level/per-page stats helpers and simplifying several function signatures.
  • Adds/updates MTR test coverage and expected outputs for the new/changed persistent statistics rows and optimizer behavior.

Reviewed changes

Copilot reviewed 37 out of 37 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
storage/innobase/include/univ.i Removes stats debug macro configuration.
storage/innobase/include/dict0stats.h Updates API docs for persistent stats update behavior.
storage/innobase/include/dict0mem.h Adjusts index statistics array element types.
storage/innobase/include/data0type.h Updates len_is_stored() predicate.
storage/innobase/include/btr0cur.h Moves BLOB header constants and adds external-field reference helpers.
storage/innobase/handler/ha_innodb.cc Removes redundant explicit persistent-stats save calls.
storage/innobase/dict/dict0stats.cc Main fix/refactor: propagate stats method, compute/store non-NULL stats, refactor sampling/full-scan logic, and enhance stat_description.
storage/innobase/btr/btr0cur.cc Exposes external-field reference offset helper; removes old external-length helper.
mysql-test/suite/innodb/t/stats_method.test Adds coverage for NULL-handling stats methods and persistent-stat outputs.
mysql-test/suite/innodb/t/stats_method.combinations Adds combinations for running the stats_method test under each stats method.
mysql-test/suite/innodb/t/stats_external_pages.test Adds test exercising external/BLOB-page adjustment behavior.
mysql-test/suite/innodb/t/records_in_range.test Updates test messaging to reflect changed leaf-page accounting.
mysql-test/suite/innodb/r/stats_method.result Expected output for the new stats_method test.
mysql-test/suite/innodb/r/stats_method,NULLS_UNEQUAL.rdiff Expected output delta for NULLS_UNEQUAL run.
mysql-test/suite/innodb/r/stats_method,NULLS_IGNORED.rdiff Expected output delta for NULLS_IGNORED run.
mysql-test/suite/innodb/r/stats_external_pages.result Expected output for the new stats_external_pages test.
mysql-test/suite/innodb/r/records_in_range.result Expected output updates reflecting leaf-page accounting change.
mysql-test/suite/innodb/r/records_in_range,8k.rdiff Page-size-specific expected output update for 8k.
mysql-test/suite/innodb/r/records_in_range,4k.rdiff Page-size-specific expected output update for 4k.
mysql-test/suite/innodb/r/instant_alter_index_rename.result Updates expected persistent stats rows (adds non-null stats entries).
mysql-test/suite/innodb/r/innodb-index-online.result Updates optimizer/stat output expectations impacted by new stats rows/values.
mysql-test/suite/innodb/r/innodb_stats_rename_table.result Updates expected stats rows (adds non-null stats entries).
mysql-test/suite/innodb/r/innodb_stats_rename_table_if_exists.result Updates expected stats rows (adds non-null stats entries).
mysql-test/suite/innodb/r/innodb_stats_fetch.result Updates expected stats fetch output to include non-null stats rows.
mysql-test/suite/innodb/r/innodb_stats_drop_locked.result Updates expected output counts impacted by additional stats rows.
mysql-test/suite/innodb/r/innodb_stats_auto_recalc.result Updates expected auto-recalc outputs to include non-null stats rows.
mysql-test/suite/innodb/r/innodb_stats_auto_recalc_on_nonexistent.result Updates expected row counts for additional stats rows.
mysql-test/suite/innodb/r/innodb_stats_auto_recalc_ddl.result Updates expected DDL auto-recalc outputs to include non-null stats rows.
mysql-test/suite/gcol/r/innodb_virtual_stats.result Updates expected persistent stats rows for generated columns scenario.
mysql-test/main/stat_tables_innodb.result Updates expected plan output affected by statistics changes.
mysql-test/main/selectivity_innodb_notembedded.result Updates expected cost outputs affected by statistics changes.
mysql-test/main/opt_trace_index_merge_innodb.result Updates expected optimizer trace costs affected by statistics changes.
mysql-test/main/mysqldump-system.result Updates expected dump output for new persistent stats rows.
mysql-test/main/innodb_ext_key,on,unoptimized.rdiff Updates expected diff metadata impacted by changed outputs.
mysql-test/main/innodb_ext_key,off.rdiff Updates expected diff content impacted by changed outputs.
mysql-test/main/innodb_ext_key,covering,on.rdiff Updates expected diff metadata impacted by changed outputs.
mysql-test/main/index_intersect_innodb.result Updates expected plans for index merge behavior affected by statistics changes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

mtr_t *mtr_) :
nulls_unequal(nulls_unequal_), level(level_), n_pages(0),
n_diff(n_diff_), n_non_null(n_non_null_), bounds(bounds_),
n_recs(level_ ? 1 : 0), index(index_), mtr(mtr_)
size_t method_len = strlen(stats_method_name);
size_t remaining = sizeof(stat_description)
- desc_len - 1;
if (method_len < remaining) {
uint32_t n_sample_pages=1; /* number of pages to sample */
ulint not_empty_flag = 0;
ulint total_external_size = 0;
uint32_t n_blob_pages = 0;
Comment on lines +73 to +82
/** The structure of a BLOB part header */
/* @{ */
/*--------------------------------------*/
#define BTR_BLOB_HDR_PART_LEN 0 /*!< BLOB part len on this
page */
#define BTR_BLOB_HDR_NEXT_PAGE_NO 4 /*!< next BLOB part page no,
FIL_NULL if none */
/*--------------------------------------*/
#define BTR_BLOB_HDR_SIZE 8 /*!< Size of a BLOB
part header, in bytes */
Comment on lines +186 to +190
SELECT index_name, stat_name, stat_value
FROM mysql.innodb_index_stats
WHERE table_name='t_not_null'
AND stat_name IN ('n_diff_pfx01', 'n_nonnull_pfx01')
ORDER BY index_name, stat_name;
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

7 participants