maria-developers team mailing list archive
Mailing list archive
Re: [Commits] Rev 2958: better fix for MySQL bugs in http://bazaar.launchpad.net/~maria-captains/maria/5.1/
First: please send questions like this to the list, not only to me.
I wanted to discuss this with Sergey Petrunia and had to paste this on
On Oct 22, Zardosht Kasheff wrote:
> Hello Sergei,
> One of the problems we at Tokutek have run into with the optimizer
> many times is that the optimizer does not always use these functions
> to estimate costs. I think I have found a couple of places in the
> code, and want to verify that my understanding is correct.
> In best_access_path in sql_select.cc, I see the following code in two places:
> if (table->covering_keys.is_set(key))
> /* we can use only index tree */
> uint keys_per_block= table->file->stats.block_size/2/
> tmp= record_count*(tmp+keys_per_block-1)/keys_per_block;
> tmp= record_count*min(tmp,s->worst_seeks);
> is there a reason why tmp cannot be evaluated using the functions
> keyread_time and read_time? If so, is there any reason why MariaDB or
> MySQL cannot use the functions in this place?
I cannot fix it in 5.1, unfortunately.
The code should probably be rewritten as
tmp= record_count*table->file->keyread_time(key, 1, tmp);
tmp= record_count*table->file->read_time(key, 1, min(tmp,s->worst_seeks));
but tmp is double, while the third argument of keyread_time and
read_time is ha_rows - an integer. That is, this change causes a small
precision loss, and it is enough for optimizer to start generating
different query plans. I've got quite a few test failures because
EXPLAIN output changed. We cannot do changes that affect query plans in
a stable version. One thing I've learned in MySQL - no matter how
correct and good such a change is, there will always be queries that it
will affect adversely. And it won't matter that the new plan is
correct, and the old plan was completely illogical and caused by a bug
in the optimizer or something - they'll say that their queries were
faster that way.
I'll see about doing this change it 5.2 though.