|List:||Commits||« Previous MessageNext Message »|
|From:||Guilhem Bichot||Date:||December 30 2010 11:34am|
|Subject:||Re: bzr commit into mysql-next-mr-bugfixing branch (jorgen.loland:3242)|
|View as plain text|
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.