maria-developers team mailing list archive

[Michael Widenius <monty@xxxxxxxxxxxx>]

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