Re: bzr commit into mysql-next-mr-bugfixing branch (jorgen.loland:3242) WL#4800

[Guilhem Bichot]

Hello Jorgen,

Jorgen Loland a écrit, Le 21.12.2010 08:34:
> #At
> file:///export/home/jl208045/mysql/wl4800/mysql-next-mr-opt-backporting-wl4800-revert_printquick/
> based on revid:guilhem@stripped
> 
>  3242 Jorgen Loland	2010-12-21
>       WL#4800: 
>       
>       Revert print_quick() and friends to how they are in 
>       next-mr-opt-backporting, i.e., remove optimizer tracing from 
>       this function since tracing of quick is performed in 
>       TABLE_READ_PLAN::trace_basic_info() instead.

> === modified file 'sql/opt_range.cc'
> --- a/sql/opt_range.cc	2010-12-19 14:24:03 +0000
> +++ b/sql/opt_range.cc	2010-12-21 07:34:33 +0000
> @@ -792,17 +792,13 @@ TABLE_READ_PLAN *get_best_disjunct_quick
>  static
>  TRP_GROUP_MIN_MAX *get_best_group_min_max(PARAM *param, SEL_TREE *tree,
>                                            double read_time);
> -#ifndef DBUG_OFF
> +#if !defined(DBUG_OFF) || defined(OPTIMIZER_TRACE)
>  static void print_sel_tree(PARAM *param, SEL_TREE *tree, key_map *tree_map,
>                             const char *msg);

print_sel_tree() is defined only if DBUG_OFF is not defined, so adding 
defined(OPTIMIZER_TRACE) to the declaration is "inconsistent".
Same for print_ror_scans_arr().
As for print_quick(), it's used only in DBUG_EXECUTE() so should also be 
in just "#ifndef DBUG_OFF", for declaration and definition. Same for 
::dbug_dump() functions.
Basically, all those functions above are not used in optimizer tracing.

Note: if DBUG_OFF is not defined then OPTIMIZER_TRACE is defined (this 
is enforced in opt_trace.h).

I'm looking at the diff of opt_range.cc between main tree and the 
feature tree, after this patch has been pushed. There is still:
* in SQL_SELECT::test_quick_select():
main tree:
   DBUG_PRINT("info", ("records: %lu", (ulong) head->file->stats.records));
wl4800 tree:
   //DBUG_PRINT("info", ("records: %lu", (ulong) 
head->file->stats.records));
I suggest just deleting the line above, as the same info is printed to 
the debug trace by
   Opt_trace_object(trace, "table_scan").
     add("records", head->file->stats.records).
a few lines down.

This was added:
           DBUG_ASSERT(tree->type == SEL_TREE::ALWAYS); //anything else 
possible?
do you want to keep it (I haven't researched whether it is correct)?

Those lines got commented out:
       //DBUG_PRINT("info",
       //           ("Returning range plan for key %s, cost %g, records 
%lu",
       //            param->table->key_info[param->real_keynr[idx]].name,
       //            read_plan->read_cost, (ulong) read_plan->records));
I suggest uncommenting or deleting them.

I had added this comment:
   /**
      @todo problem: with optimizer trace on and --debug not used, this 
prints
      to stderr
   */
but it is wrong now, because print_quick() is not used in opt trace 
anymore, so there is no problem anymore, could you please delete the 
comment?

Thanks.
I'll study sql_test.cc, maybe bits can be reverted there too. Or at 
least cleaned up.