Hi Jay! I can't answer for Brian but at developer day on Friday we discussed the optimizer a bit. Basically, I was complaining (as I do) that its impossible to work with now since very few of us understand how it works and the code is a bit hard to understand.
Brian then said there was a bunch of un-needed code he could remove which he would do during the week. I'm assuming that is what is going on here. Also, David Axmark made the awesome suggestion which I really liked of just removing things from the optimizer to make it less code and more understandable and suffering the performance hit. Since this is drizzle with no customers right now, we could add everything we remove back later in a more drizzle like fashion. With that in mind, I have a branch which removes all code related to count, min, max, sum from the optimizer and some of the same stuff Brian removed. I put it at lp:~posulliv/drizzle/optimizer-style-cleanup and it creates a significant regression so I havn't proposed it for merging. Do you have any opinion on this approach? -Padraig On Tue, Apr 20, 2010 at 9:50 AM, Jay Pipes <[email protected]> wrote: > Hi! > > Can someone explain to me why this code was removed? Was it the case > of just not having a test case which adequately covered this code > path? > > Thanks, > > jay > > On Mon, Apr 19, 2010 at 7:11 PM, <[email protected]> wrote: >> ------------------------------------------------------------ >> revno: 1480 >> committer: Brian Aker <br...@gaz> >> branch nick: bad-staging >> timestamp: Sat 2010-04-17 15:02:36 -0700 >> message: >> Merge range. >> modified: >> drizzled/optimizer/range.cc >> >> >> -- >> lp:drizzle/staging >> https://code.launchpad.net/~drizzle-developers/drizzle/staging >> >> You are subscribed to branch lp:drizzle/staging. >> To unsubscribe from this branch go to >> https://code.launchpad.net/~drizzle-developers/drizzle/staging/+edit-subscription >> >> === modified file 'drizzled/optimizer/range.cc' >> --- drizzled/optimizer/range.cc 2010-03-22 05:47:41 +0000 >> +++ drizzled/optimizer/range.cc 2010-04-17 22:02:36 +0000 >> @@ -272,11 +272,6 @@ >> bool >> *are_all_covering); >> >> static >> -optimizer::RorIntersectReadPlan >> *get_best_covering_ror_intersect(optimizer::Parameter *param, >> - >> optimizer::SEL_TREE *tree, >> - double >> read_time); >> - >> -static >> optimizer::TableReadPlan *get_best_disjunct_quick(optimizer::Parameter >> *param, >> optimizer::SEL_IMERGE >> *imerge, >> double read_time); >> @@ -816,14 +811,6 @@ >> { >> best_trp= rori_trp; >> best_read_time= best_trp->read_cost; >> - /* >> - Try constructing covering ROR-intersect only if it looks >> possible >> - and worth doing. >> - */ >> - if (rori_trp->isRowRetrievalNecessary() && can_build_covering && >> - (rori_trp= get_best_covering_ror_intersect(¶m, tree, >> - best_read_time))) >> - best_trp= rori_trp; >> } >> } >> } >> @@ -1276,40 +1263,6 @@ >> return (val1 < val2)? -1: (val1 == val2)? 0 : 1; >> } >> >> -/* >> - Compare two ROR_SCAN_INFO** by >> - (#covered fields in F desc, >> - #components asc, >> - number of first not covered component asc) >> - >> - SYNOPSIS >> - cmp_ror_scan_info_covering() >> - a ptr to first compared value >> - b ptr to second compared value >> - >> - RETURN >> - -1 a < b >> - 0 a = b >> - 1 a > b >> -*/ >> - >> -static int cmp_ror_scan_info_covering(ROR_SCAN_INFO** a, ROR_SCAN_INFO** b) >> -{ >> - if ((*a)->used_fields_covered > (*b)->used_fields_covered) >> - return -1; >> - if ((*a)->used_fields_covered < (*b)->used_fields_covered) >> - return 1; >> - if ((*a)->key_components < (*b)->key_components) >> - return -1; >> - if ((*a)->key_components > (*b)->key_components) >> - return 1; >> - if ((*a)->first_uncovered_field < (*b)->first_uncovered_field) >> - return -1; >> - if ((*a)->first_uncovered_field > (*b)->first_uncovered_field) >> - return 1; >> - return 0; >> -} >> - >> >> /* Auxiliary structure for incremental ROR-intersection creation */ >> typedef struct >> @@ -1830,140 +1783,6 @@ >> >> >> /* >> - Get best covering ROR-intersection. >> - SYNOPSIS >> - get_best_covering_ror_intersect() >> - param Parameter from test_quick_select function. >> - tree optimizer::SEL_TREE with sets of intervals for different >> keys. >> - read_time Don't return table read plans with cost > read_time. >> - >> - RETURN >> - Best covering ROR-intersection plan >> - NULL if no plan found. >> - >> - NOTES >> - get_best_ror_intersect must be called for a tree before calling this >> - function for it. >> - This function invalidates tree->ror_scans member values. >> - >> - The following approximate algorithm is used: >> - I=set of all covering indexes >> - F=set of all fields to cover >> - S={} >> - >> - do >> - { >> - Order I by (#covered fields in F desc, >> - #components asc, >> - number of first not covered component asc); >> - F=F-covered by first(I); >> - S=S+first(I); >> - I=I-first(I); >> - } while F is not empty. >> -*/ >> - >> -static >> -optimizer::RorIntersectReadPlan >> *get_best_covering_ror_intersect(optimizer::Parameter *param, >> - >> optimizer::SEL_TREE *tree, >> - double >> read_time) >> -{ >> - ROR_SCAN_INFO **ror_scan_mark; >> - ROR_SCAN_INFO **ror_scans_end= tree->ror_scans_end; >> - >> - for (ROR_SCAN_INFO **scan= tree->ror_scans; scan != ror_scans_end; ++scan) >> - (*scan)->key_components= >> - param->table->key_info[(*scan)->keynr].key_parts; >> - >> - /* >> - Run covering-ROR-search algorithm. >> - Assume set I is [ror_scan .. ror_scans_end) >> - */ >> - >> - /*I=set of all covering indexes */ >> - ror_scan_mark= tree->ror_scans; >> - >> - MyBitmap *covered_fields= ¶m->tmp_covered_fields; >> - if (! covered_fields->getBitmap()) >> - { >> - my_bitmap_map *tmp_bitmap= (my_bitmap_map*)alloc_root(param->mem_root, >> - param->fields_bitmap_size); >> - covered_fields->setBitmap(tmp_bitmap); >> - } >> - if (! covered_fields->getBitmap() || >> - covered_fields->init(covered_fields->getBitmap(), >> - param->table->s->fields)) >> - return 0; >> - covered_fields->clearAll(); >> - >> - double total_cost= 0.0f; >> - ha_rows records=0; >> - bool all_covered; >> - >> - do >> - { >> - /* >> - Update changed sorting info: >> - #covered fields, >> - number of first not covered component >> - Calculate and save these values for each of remaining scans. >> - */ >> - for (ROR_SCAN_INFO **scan= ror_scan_mark; scan != ror_scans_end; ++scan) >> - { >> - bitmap_subtract(&(*scan)->covered_fields, covered_fields); >> - (*scan)->used_fields_covered= >> - (*scan)->covered_fields.getBitsSet(); >> - (*scan)->first_uncovered_field= >> - (*scan)->covered_fields.getFirst(); >> - } >> - >> - internal::my_qsort(ror_scan_mark, ror_scans_end-ror_scan_mark, >> - sizeof(ROR_SCAN_INFO*), >> - (qsort_cmp)cmp_ror_scan_info_covering); >> - >> - /* I=I-first(I) */ >> - total_cost += (*ror_scan_mark)->index_read_cost; >> - records += (*ror_scan_mark)->records; >> - if (total_cost > read_time) >> - return NULL; >> - /* F=F-covered by first(I) */ >> - bitmap_union(covered_fields, &(*ror_scan_mark)->covered_fields); >> - all_covered= bitmap_is_subset(¶m->needed_fields, covered_fields); >> - } while ((++ror_scan_mark < ror_scans_end) && !all_covered); >> - >> - if (!all_covered || (ror_scan_mark - tree->ror_scans) == 1) >> - return NULL; >> - >> - /* >> - Ok, [tree->ror_scans .. ror_scan) holds covering index_intersection with >> - cost total_cost. >> - */ >> - /* Add priority queue use cost. */ >> - total_cost += rows2double(records)* >> - log((double)(ror_scan_mark - tree->ror_scans)) / >> - (TIME_FOR_COMPARE_ROWID * M_LN2); >> - >> - if (total_cost > read_time) >> - return NULL; >> - >> - optimizer::RorIntersectReadPlan *trp= NULL; >> - if (! (trp= new (param->mem_root) optimizer::RorIntersectReadPlan)) >> - { >> - return trp; >> - } >> - >> - uint32_t best_num= (ror_scan_mark - tree->ror_scans); >> - trp->ror_range_scans.assign(tree->ror_scans, tree->ror_scans + best_num); >> - trp->setRowRetrievalNecessary(true); >> - trp->read_cost= total_cost; >> - trp->records= records; >> - trp->cpk_scan= NULL; >> - set_if_smaller(param->table->quick_condition_rows, records); >> - >> - return(trp); >> -} >> - >> - >> -/* >> Get best "range" table read plan for given optimizer::SEL_TREE, also >> update some info >> >> SYNOPSIS >> >> >> > > _______________________________________________ > Mailing list: https://launchpad.net/~drizzle-discuss > Post to : [email protected] > Unsubscribe : https://launchpad.net/~drizzle-discuss > More help : https://help.launchpad.net/ListHelp > _______________________________________________ Mailing list: https://launchpad.net/~drizzle-discuss Post to : [email protected] Unsubscribe : https://launchpad.net/~drizzle-discuss More help : https://help.launchpad.net/ListHelp
