maria-developers team mailing list archive

[Zardosht Kasheff <zardosht@xxxxxxxxx>]

The issue that I see with this proposal is that if quick_records is
much much greater than best_records, then we may not want to set
best_key to nr even though this nr is a covering index.

On Thu, May 19, 2011 at 6:04 AM, Michael Widenius <monty@xxxxxxxxxxxx> wrote:
>
> Hi!
>
>>>>>> "Zardosht" == Zardosht Kasheff <zardosht@xxxxxxxxx> writes:
>
> Zardosht> Hello all,
> Zardosht> Continuing with patch contributions (as encouraged at the storage
> Zardosht> engine summit last Friday), here is a very simple patch we have to fix
> Zardosht> MySQL bug 57430 (http://bugs.mysql.com/bug.php?id=57430).
>
> Zardosht> The problem is described in the bug report we sent to MySQL. We think
> Zardosht> this simple fix addresses the problem that we ran into, but we do not
> Zardosht> understand the code well enough to know if this fix is sufficient.
>
> Zardosht> Feedback is welcome.
>
>> +++ sql/sql_select.cc (revision 25793)
>> @@ -13964,7 +13964,8 @@
>>              if (best_key < 0 ||
>>                  (select_limit <= min(quick_records,best_records) ?
>>                   keyinfo->key_parts < best_key_parts :
>> -                 quick_records < best_records))
>> +                 quick_records < best_records) ||
>> +                (quick_records == best_records && !is_best_covering && is_covering))
>>              {
>>                best_key= nr;
>>                best_key_parts= keyinfo->key_parts;
>
> The above suggestion is not good as it depends on
> quick_records == best_records which is not likely to be the same for
> two indexes for any larger tables.
>
> In the same loop code we have the test:
>  if ((is_best_covering && !is_covering) || ...)
>  continue;
>
> This is done without any testing of cost and to make things symetrical
> we could just remove the 'quick_records == best_records' from the
> above patch.
>
> A better solution is to try to calculate the cost of using covered and
> not covered keys. This will allow us to use covering keys also when we
> will hit a bit more rows that way.
>
> However, this is not an easy task (thought and talked about this for
> 2-4 hours) so I will leave the cost calculation to after 5.3.
>
> The change I did was simple:
>
>> +                 quick_records < best_records) ||
>> +                 (!is_best_covering && is_covering))
>
> This makes covering indexes work identical independent of their
> position in the key array.
>
> After 5.3 I will change the filesort code to calculate the true cost
> for each index (instead of as now do the decision based of number of
> key parts, which doesn't make any sense).
>
> I expect my changes to be pushed today into 5.3
>
> Regards,
> Monty
>