← Back to team overview

maria-developers team mailing list archive

More Review input for MDEV-21092, 21095, 29997

 

Hi Rex,

Nice to see progress with the patch, but there is further input.

In ./mysql-test/main/opt_trace.test

Do you see this comment at the end:

> --echo # End of 10.6 tests

Please move the newly added testcases to be AFTER that line.
This is important for those who do merges between version.

sql_select.cc, coding style comments:
instead of 

  trace_attached_conditions( thd, join );

use 

  trace_attached_conditions(thd, join);

In opt_range.cc:
instead of

  make_used_partitions_str(thd->mem_root, prune_param.part_info, &parts, parts_list );

Use

  make_used_partitions_str(thd->mem_root, prune_param.part_info, &parts,
                           parts_list);

No spaces, observe line length limit.


IMPORTANT: I'm looking at the change in opt_trace_security.cc, and in the new 
file I see:

          {
            "attaching_conditions_to_tables": {
              "attached_conditions_computation": [],
              "attached_conditions_summary": [
                {
                  "table": "t1",
                  "attached": null
                }
              ]
            }
          }
        ]
      }
    },
    {
      "attaching_conditions_to_tables": {
        "attached_conditions_computation": [],
        "attached_conditions_summary": [
          {
            "table": "t1",
            "attached_condition": null
          }
        ]
      }
    },

(One can see similar picture in other examples, too).

This looks as if "attaching_conditions_to_tables" step was performed twice. 

I think we should then get back to the principle of "Optimizer Trace is a log 
of what optimizer does", which means things are logged as soon as they're done.

This means: Please put back the tracing into make_join_select() as it originally was.
With the exception that it should print "attached_condition" instead of "attached".

Index Condition Pushdown is done in make_join_readinfo(). So, please add tracing like so:

 {
   "make_join_readinfo" : [
      { 
         "table": "t1",
         "index_condition": ....
       }
       // further array elements follow
    ]
 }

The array elements should only be printed if the table has a pushed index condition. 
This will occur naturally if the code that prints the JSON object is push_index_cond().

It is ok if the code prints an empty  "make_join_readinfo": [] for cases
when there weren't any pushed index conditions.

BR
 Sergei
-- 
Sergei Petrunia, Software Developer
MariaDB Corporation | Skype: sergefp | Blog: http://petrunia.net




References