OK, thx for the explanation, Padraig! I'll take a look at your branch and provide input if I can.
Cheers! jay On Tue, Apr 20, 2010 at 10:25 AM, Padraig O'Sullivan <[email protected]> wrote: > 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
