Apart from Tor's request for documentation, the solution looks good.
However, I think this bug can be solved by checking the range after all. The
question is whether or not we want to invest time in fixing it since this is a
fix for a non-prioritized SE. I have an outline for a solution:
In mi_rkey(), inside the while() that checks the index condition, the newly
found index record is stored in info->lastkey. Further, the end_range is stored
in the ha_myisam object. You can get it from here
These two keys can be fed to my_handler's ha_key_cmp(), something like this:
<inside while(), after _mi_search_next()>
int cmp_res= 0;
uchar *end= ((ha_myisam*)info->index_cond_func_arg)->end_range->key;
cmp_res= ha_key_cmp(keyinfo->seg, key_buff, end,
use_key_length, SEARCH_FIND, not_used);
if (cmp_res > 0)
The problem is that mi_rkey.c does not include ha_myisam.h, so we'll need to
connect these somehow. This can, e.g., be done by having a function pointer (as
done with index_cond_func) or the end_range key in the info object.
On 10/19/2010 02:55 PM, Olav.Sandstaa@stripped wrote:
> #At file:///home/olav/mysql/develop2/opt-bug52660/ based on
> 3264 Olav.Sandstaa@stripped 2010-10-19
> Fix for Bug#52660 Perf. regr. using ICP for MyISAM on range queries on an
> index containing TEXT
> When an index condition is pushed down to MyISAM the content of (most
> of) the index entry will be read into the record buffer. This will be
> used for evaluating the index condition and the range check in case of
> a range scan. When reading the content of the index entry all fields
> will be read with the exception of BLOB fields. This is not a problem
> for evaluating the index condition itself since the server will not
> push down an index condition that contains a BLOB field. But it is a
> problem for the range evaluation that is performed as part of the ICP
> implementation in MyISAM since this might be done using a "non-read"
> BLOB value.
> A simplified version of the query given in the bug report:
> SELECT c1 FROM t3 WHERE c1>= 'c-1004=w' and c1<= 'c-1006=w' and i1>
> it will be done as a range scan and following happens :
> 1. MyISAM locates the first record satisfying the lower range and this
> record will be read into the record buffer. So after this the
> content of the record buffer for the BLOB field will have the value:
> 2. When the server requests the next record in the range MyISAM will
> due to ICP being used read in the index entry (but not including the
> BLOB field), so when the end of range check is done it will evaluate
> the upper range value (c1<= 'c-1006=w') against the value of the
> first record read (ie. it will evaluate to true) and since the ICP
> condition (i1> 1) evaluates to false it will skip this record and go
> to the next record. This creates a loop that instead of stopping at
> the upper range value will loop until it has read the complete index
> (or until the ICP condition evaluates to true).
> So in the example in the bug report about 10 million index entries
> will be read and evaluated instead of just 3 or 4.
> The fix for this is to let MyISAM reject pushed index conditions
> whenever the index used for ICP contains a BLOB field.
> Note that this change disables ICP from being used by MyISAM in some
> cases where it actually was safe to use it. The pushed index condition
> will be reject in all cases where the index contains a BLOB field also
> when the BLOB field is not used in any range evaluation. The reason for
> this is at the time the server pushes the index condition MyISAM does
> not know whether the actually data access will be a range scan with an
> upper range value or not.
> The patch contains a simplified version of the original test
> case. This test case only contains 100 records which is too little to
> show any performance regression. The important point with the test
> case is that without the fix the explain will show "Using index
> condition" while with the fix it will say "Using where".
Jørgen Løland | Senior Software Engineer | +47 73842138