← Back to team overview

drizzle-discuss team mailing list archive

Re: [Branch ~drizzle-developers/drizzle/staging] Rev 1480: Merge range.

 

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 <jaypipes@xxxxxxxxx> 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,  <noreply@xxxxxxxxxxxxx> wrote:
>> ------------------------------------------------------------
>> revno: 1480
>> committer: Brian Aker <brian@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(&param, 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= &param->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(&param->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     : drizzle-discuss@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~drizzle-discuss
> More help   : https://help.launchpad.net/ListHelp
>



Follow ups

References