MDEV-36511 dump basic statistics to the trace #4087

bsrikanth-mariadb · 2025-06-04T11:49:19Z

Pull request created in: https://jira.mariadb.org/browse/MDEV-36511

spetrunia · 2025-06-10T12:52:40Z

sql/opt_range.cc


        if (unlikely(thd->trace_started()))
-          trace_ranges(&trace_range, param, idx, key, key_part);
+        {


Please move almost all of this code into a function which should be in `opt_trace_ddl_info.*' (we'll eventually rename that file to reflect that optimizer context capture code is there).

We're reusing trace_ranges to produce the list of ranges. This is ok. I think for better isolation, let trace_ranges just accept a callback (or lambda function) which it will use to report ranges.

sure. will do in next development sprint.

moved most of the changes to opt_trace_ddl_info.cc, and some of them to trace_ranges()

spetrunia · 2025-06-10T13:04:15Z

sql/opt_trace_ddl_info.cc

  return false;
 }

+static void dump_index_range_stats_to_trace(THD *thd, uchar *tbl_name,


Please move this function into opt_trace_ddl_info.cc

this is already in opt_trace_ddl_info.cc

spetrunia · 2025-06-10T13:04:46Z

sql/opt_trace_ddl_info.h

-bool store_table_definitions_in_trace(THD *thd);
+bool store_tables_context_in_trace(THD *thd);

+struct trace_range_context{


Move this into opt_trace_ddl_info.h

this is already in opt_trace_ddl_info.h

spetrunia · 2025-06-10T13:45:47Z

sql/sql_class.h

  HASH ull_hash;
  /* Hash of used sequences (for PREVIOUS value) */
  HASH sequences;
+  /* 


This should be a pointer.
I would expect it to be a pointer to some structure with methods. Something like

Optimizer_context_recorder *opt_context_recorder;

opt_range code should invoke a function to lazily create the recorder, if necessary.
Then, store_tables_context_in_trace() should dump the data from the context recorder.

The code in opt_trace

sure. will do in next development sprint.

made the changes.

spetrunia · 2025-08-04T13:33:52Z

Please checkout review input here:

commit 1ae57e90569bb78ca45d4120146d9cf1cb953602 (HEAD -> 12.1-MDEV-36511-review-input, origin/12.1-MDEV-36511-review-input)
Author: Sergei Petrunia <sergey@mariadb.com>
Date:   Mon Aug 4 16:24:50 2025 +0300

    Make Optimizer_Stats_Context_Recorder::tbl_trace_ctx_hash not to be a pointer.

commit 301254550758517ce86f610b38cf2f48d3e388c9 (12.1-MDEV-36511-dump-basic-statistics)
Author: Sergei Petrunia <sergey@mariadb.com>
Date:   Mon Aug 4 15:49:44 2025 +0300

    Rename list_range_context into "range_list" and make it non-pointer.

commit 5b9a3c8b9fd2a28126da45b7f1e9a7e4ffd95ecf
Author: Sergei Petrunia <sergey@mariadb.com>
Date:   Mon Aug 4 15:21:47 2025 +0300

    Review input 1: trace_table_index_range_context shouldn't be a pointer, rename to 'index'

spetrunia · 2025-08-04T13:34:51Z

What is the point of this allocation in opt_range.cc :

    range_list= (List<char> *) param->thd->calloc(sizeof(List<char>));
    range_list->empty();

spetrunia · 2025-08-04T13:36:05Z

What will be recorded by your patch if one runs this:

select * 
from t1 at TBL, t1 as TBL2
where TBL1.key1 < 'foo' and 
TBL2.key2 < 'bar'

?

bsrikanth-mariadb · 2025-08-05T03:59:15Z

Please checkout review input here:

commit 1ae57e90569bb78ca45d4120146d9cf1cb953602 (HEAD -> 12.1-MDEV-36511-review-input, origin/12.1-MDEV-36511-review-input)
Author: Sergei Petrunia <sergey@mariadb.com>
Date:   Mon Aug 4 16:24:50 2025 +0300

    Make Optimizer_Stats_Context_Recorder::tbl_trace_ctx_hash not to be a pointer.

commit 301254550758517ce86f610b38cf2f48d3e388c9 (12.1-MDEV-36511-dump-basic-statistics)
Author: Sergei Petrunia <sergey@mariadb.com>
Date:   Mon Aug 4 15:49:44 2025 +0300

    Rename list_range_context into "range_list" and make it non-pointer.

commit 5b9a3c8b9fd2a28126da45b7f1e9a7e4ffd95ecf
Author: Sergei Petrunia <sergey@mariadb.com>
Date:   Mon Aug 4 15:21:47 2025 +0300

    Review input 1: trace_table_index_range_context shouldn't be a pointer, rename to 'index'

sure. Incorporated the changes.

bsrikanth-mariadb · 2025-08-05T03:59:40Z

What is the point of this allocation in opt_range.cc :
    range_list= (List<char> *) param->thd->calloc(sizeof(List<char>));
    range_list->empty();

new allocation is not required, so removed the memory allocation code.

However, we require range_list so as to store all the ranges that were created in this method, before it can be passed to recorder.

bsrikanth-mariadb · 2025-08-05T04:09:22Z

What will be recorded by your patch if one runs this:
select * 
from t1 at TBL, t1 as TBL2
where TBL1.key1 < 'foo' and 
TBL2.key2 < 'bar'
?

    {
      "current_database": "db1",
      "list_contexts": [
        {
          "name": "db1.t1",
          "ddl": "CREATE TABLE `t1` (\n  `key1` varchar(10) DEFAULT NULL,\n  `key2` varchar(10) DEFAULT NULL,\n  KEY `t1_idx_key1` (`key1`),\n  KEY `t1_idx_key2` (`key2`)\n) ENGINE=MyISAM DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_uca1400_ai_ci",
          "num_of_records": 5,
          "indexes": [
            {
              "index_name": "t1_idx_key1",
              "rec_per_key": ["0"]
            },
            {
              "index_name": "t1_idx_key2",
              "rec_per_key": ["0"]
            }
          ],
          "list_ranges": [
            {
              "index_name": "t1_idx_key1",
              "ranges": ["(NULL) < (key1) < (foo)"],
              "num_rows": 3
            },
            {
              "index_name": "t1_idx_key2",
              "ranges": ["(NULL) < (key2) < (bar)"],
              "num_rows": 1
            }
          ]
        }
      ]
    }

spetrunia · 2025-08-19T10:25:56Z

@bsrikanth-mariadb , please check this : 9438e15 :

commit 9438e15ce5337dc3f9f4345c885f44be00056084 (HEAD -> 12.1-MDEV-36511-dump-basic-statistics-review-input, origin/12.1-MDEV-36511-dump-basic-statistics-review-input, 12.1-MDEV-36511-dump-basic-statistics)
Author: Sergei Petrunia <sergey@mariadb.com>
Date:   Tue Aug 19 13:22:08 2025 +0300

    MDEV-36511: Dump basic stats of a table to trace: REVIEW INPUT
    
    Make opt_range.cc use fool-proof APIs for recording.

Any comments?

spetrunia · 2025-08-19T10:33:13Z

Next question: can we move implementation details from opt_trace_ddl_info.h to opt_trace_ddl_info.cc ?

bsrikanth-mariadb · 2025-08-20T07:09:45Z

Next question: can we move implementation details from opt_trace_ddl_info.h to opt_trace_ddl_info.cc ?

sure. done

bsrikanth-mariadb · 2025-08-20T07:10:53Z

@bsrikanth-mariadb , please check this : 9438e15 :

commit 9438e15ce5337dc3f9f4345c885f44be00056084 (HEAD -> 12.1-MDEV-36511-dump-basic-statistics-review-input, origin/12.1-MDEV-36511-dump-basic-statistics-review-input, 12.1-MDEV-36511-dump-basic-statistics)
Author: Sergei Petrunia <sergey@mariadb.com>
Date:   Tue Aug 19 13:22:08 2025 +0300

    MDEV-36511: Dump basic stats of a table to trace: REVIEW INPUT
    
    Make opt_range.cc use fool-proof APIs for recording.

Any comments?

yes, liked it. incorporated the changes.

This feature stores the basic stats of the base tables that are used in a query, to the optimizer trace. This feature is also controlled by optimizer_record_context, and is not enabled by default. The stats such as num_of_records present in the table, indexes if present then their names, along with the average number of records_per_key with in each index are dumped to the trace. Additionally, stats from range analysis of the queries are also dumped into the trace The approach taken here is to extend the existing function store_tables_context_in_trace() and add new dump_table_stats_to_trace() to opt_trace_ddl_info.cc. For storing the range analysis stats into the trace, they are first recorded in the hash defined in Optimizer_Stats_Context_Recorder, and is used later while dumping the stats into the trace from dump_table_stats_to_trace(). Several new tests are also added to opt_trace_store_stats.test

bsrikanth-mariadb added the MariaDB Corporation label Jun 4, 2025

bsrikanth-mariadb changed the base branch from main to 12.1-MDEV-36483-dump-ddls-of-tables-views June 4, 2025 11:53

bsrikanth-mariadb force-pushed the 12.1-MDEV-36511-dump-basic-statistics branch 6 times, most recently from 48443ea to 45acb67 Compare June 10, 2025 09:58

spetrunia requested changes Jun 10, 2025

View reviewed changes

bsrikanth-mariadb force-pushed the 12.1-MDEV-36511-dump-basic-statistics branch 8 times, most recently from 8740969 to a71242f Compare July 29, 2025 07:50

bsrikanth-mariadb force-pushed the 12.1-MDEV-36483-dump-ddls-of-tables-views branch from deb8e78 to db937cc Compare July 29, 2025 11:26

bsrikanth-mariadb force-pushed the 12.1-MDEV-36511-dump-basic-statistics branch from a71242f to 52996cd Compare July 29, 2025 15:45

bsrikanth-mariadb force-pushed the 12.1-MDEV-36511-dump-basic-statistics branch from 52996cd to b434979 Compare August 5, 2025 03:57

bsrikanth-mariadb force-pushed the 12.1-MDEV-36511-dump-basic-statistics branch 4 times, most recently from 67397e8 to 97fd0f2 Compare August 8, 2025 08:05

bsrikanth-mariadb force-pushed the 12.1-MDEV-36511-dump-basic-statistics branch from 97fd0f2 to 491ded1 Compare August 20, 2025 07:09

bsrikanth-mariadb changed the base branch from 12.1-MDEV-36483-dump-ddls-of-tables-views to main October 24, 2025 02:41

bsrikanth-mariadb force-pushed the 12.1-MDEV-36511-dump-basic-statistics branch from 491ded1 to c14aad7 Compare October 24, 2025 04:00

bsrikanth-mariadb force-pushed the 12.1-MDEV-36511-dump-basic-statistics branch from c14aad7 to 5437094 Compare November 24, 2025 15:21

bsrikanth-mariadb force-pushed the 12.1-MDEV-36511-dump-basic-statistics branch from 5437094 to f0eb1e5 Compare December 5, 2025 09:18

bsrikanth-mariadb force-pushed the 12.1-MDEV-36511-dump-basic-statistics branch from f0eb1e5 to bbf1393 Compare January 28, 2026 11:35

Uh oh!

MDEV-36511 dump basic statistics to the trace #4087

Are you sure you want to change the base?

MDEV-36511 dump basic statistics to the trace #4087

Conversation

bsrikanth-mariadb commented Jun 4, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

spetrunia commented Aug 4, 2025

Uh oh!

spetrunia commented Aug 4, 2025

Uh oh!

spetrunia commented Aug 4, 2025

Uh oh!

bsrikanth-mariadb commented Aug 5, 2025

Uh oh!

bsrikanth-mariadb commented Aug 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bsrikanth-mariadb commented Aug 5, 2025

Uh oh!

spetrunia commented Aug 19, 2025

Uh oh!

spetrunia commented Aug 19, 2025

Uh oh!

bsrikanth-mariadb commented Aug 20, 2025

Uh oh!

bsrikanth-mariadb commented Aug 20, 2025

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

bsrikanth-mariadb commented Aug 5, 2025 •

edited

Loading