Commit 8ef094fe authored by Sergey Petrunya's avatar Sergey Petrunya

BUG#707925: Wrong result with join_cache_level=6 optimizer_use_mrr = force (incremental, BKA join)

- The problem was that Mrr_ordered_index_reader's interrupt_read() and resume_read() would 
  save and restore 1) index tuple  2) the rowid (as bytes returned by handler->position()).  Clustered 
  primary key columns were not saved/restored. 
  They are not explicitly present in the index tuple (i.e. table->key_info[secondary_key].key_parts 
  doesn't list them), but they are actually there, in particular 
  table->field[clustered_primary_key_member].part_of_key(secondary_key) == 1. Index condition pushdown
  code [correctly] uses the latter as inidication that pushed index condition can refer to clustered PK
  members. 

  The fix was to make interrupt_read()/resume_read() to save/restore clustered primary key members as well,
  so that we get correct values for them when evaluating pushed index condition.
[3rd attempt: remove the debugging aids, fix comments in testcase]
parent c6ba9598
...@@ -736,3 +736,32 @@ SELECT * FROM t1 WHERE c2 <=> NULL ORDER BY c2 LIMIT 2; ...@@ -736,3 +736,32 @@ SELECT * FROM t1 WHERE c2 <=> NULL ORDER BY c2 LIMIT 2;
c1 c2 c3 c1 c2 c3
08:29:45 NULL 2009-02-01 08:29:45 NULL 2009-02-01
drop table `t1`; drop table `t1`;
#
# BUG#707925: Wrong result with join_cache_level=6 optimizer_use_mrr =
# force (incremental, BKA join)
#
set @_save_join_cache_level= @@join_cache_level;
set join_cache_level = 6;
CREATE TABLE t1 (
f1 int(11), f2 int(11), f3 varchar(1), f4 varchar(1),
PRIMARY KEY (f1),
KEY (f3),
KEY (f2)
) ENGINE=InnoDB;
INSERT INTO t1 VALUES ('11','8','f','f'),('12','5','v','v'),('13','8','s','s'),
('14','8','a','a'),('15','6','p','p'),('16','7','z','z'),('17','2','a','a'),
('18','5','h','h'),('19','7','h','h'),('20','2','v','v'),('21','9','v','v'),
('22','142','b','b'),('23','3','y','y'),('24','0','v','v'),('25','3','m','m'),
('26','5','z','z'),('27','9','n','n'),('28','1','d','d'),('29','107','a','a');
select count(*) from (
SELECT alias1.f2
FROM
t1 AS alias1 JOIN (
t1 AS alias2 FORCE KEY (f3) JOIN
t1 AS alias3 FORCE KEY (f2) ON alias3.f2 = alias2.f2 AND alias3.f4 = alias2.f3
) ON alias3.f1 <= alias2.f1
) X;
count(*)
361
set join_cache_level=@_save_join_cache_level;
drop table t1;
...@@ -426,3 +426,32 @@ SELECT * FROM t1 WHERE c2 <=> NULL ORDER BY c2 LIMIT 2; ...@@ -426,3 +426,32 @@ SELECT * FROM t1 WHERE c2 <=> NULL ORDER BY c2 LIMIT 2;
SELECT * FROM t1 WHERE c2 <=> NULL ORDER BY c2 LIMIT 2; SELECT * FROM t1 WHERE c2 <=> NULL ORDER BY c2 LIMIT 2;
drop table `t1`; drop table `t1`;
--echo #
--echo # BUG#707925: Wrong result with join_cache_level=6 optimizer_use_mrr =
--echo # force (incremental, BKA join)
--echo #
set @_save_join_cache_level= @@join_cache_level;
set join_cache_level = 6;
CREATE TABLE t1 (
f1 int(11), f2 int(11), f3 varchar(1), f4 varchar(1),
PRIMARY KEY (f1),
KEY (f3),
KEY (f2)
) ENGINE=InnoDB;
INSERT INTO t1 VALUES ('11','8','f','f'),('12','5','v','v'),('13','8','s','s'),
('14','8','a','a'),('15','6','p','p'),('16','7','z','z'),('17','2','a','a'),
('18','5','h','h'),('19','7','h','h'),('20','2','v','v'),('21','9','v','v'),
('22','142','b','b'),('23','3','y','y'),('24','0','v','v'),('25','3','m','m'),
('26','5','z','z'),('27','9','n','n'),('28','1','d','d'),('29','107','a','a');
select count(*) from (
SELECT alias1.f2
FROM
t1 AS alias1 JOIN (
t1 AS alias2 FORCE KEY (f3) JOIN
t1 AS alias3 FORCE KEY (f2) ON alias3.f2 = alias2.f2 AND alias3.f4 = alias2.f3
) ON alias3.f1 <= alias2.f1
) X;
set join_cache_level=@_save_join_cache_level;
drop table t1;
...@@ -387,15 +387,25 @@ int Mrr_ordered_index_reader::get_next(char **range_info) ...@@ -387,15 +387,25 @@ int Mrr_ordered_index_reader::get_next(char **range_info)
bool Mrr_ordered_index_reader::set_interruption_temp_buffer(uint rowid_length, bool Mrr_ordered_index_reader::set_interruption_temp_buffer(uint rowid_length,
uint key_len, uint key_len,
uint saved_pk_len,
uchar **space_start, uchar **space_start,
uchar *space_end) uchar *space_end)
{ {
if (space_end - *space_start <= (ptrdiff_t)(rowid_length + key_len)) if (space_end - *space_start <= (ptrdiff_t)(rowid_length + key_len + saved_pk_len))
return TRUE; return TRUE;
support_scan_interruptions= TRUE; support_scan_interruptions= TRUE;
saved_rowid= *space_start; saved_rowid= *space_start;
*space_start += rowid_length; *space_start += rowid_length;
if (saved_pk_len)
{
saved_primary_key= *space_start;
*space_start += saved_pk_len;
}
else
saved_primary_key= NULL;
saved_key_tuple= *space_start; saved_key_tuple= *space_start;
*space_start += key_len; *space_start += key_len;
...@@ -406,18 +416,26 @@ bool Mrr_ordered_index_reader::set_interruption_temp_buffer(uint rowid_length, ...@@ -406,18 +416,26 @@ bool Mrr_ordered_index_reader::set_interruption_temp_buffer(uint rowid_length,
void Mrr_ordered_index_reader::set_no_interruption_temp_buffer() void Mrr_ordered_index_reader::set_no_interruption_temp_buffer()
{ {
support_scan_interruptions= FALSE; support_scan_interruptions= FALSE;
saved_key_tuple= saved_rowid= NULL; /* safety */ saved_key_tuple= saved_rowid= saved_primary_key= NULL; /* safety */
have_saved_rowid= FALSE; have_saved_rowid= FALSE;
} }
void Mrr_ordered_index_reader::interrupt_read() void Mrr_ordered_index_reader::interrupt_read()
{ {
DBUG_ASSERT(support_scan_interruptions); DBUG_ASSERT(support_scan_interruptions);
TABLE *table= file->get_table();
/* Save the current key value */ /* Save the current key value */
key_copy(saved_key_tuple, file->get_table()->record[0], key_copy(saved_key_tuple, table->record[0],
&file->get_table()->key_info[file->active_index], &table->key_info[file->active_index],
keypar.key_tuple_length); keypar.key_tuple_length);
if (saved_primary_key)
{
key_copy(saved_primary_key, table->record[0],
&table->key_info[table->s->primary_key],
table->key_info[table->s->primary_key].key_length);
}
/* Save the last rowid */ /* Save the last rowid */
memcpy(saved_rowid, file->ref, file->ref_length); memcpy(saved_rowid, file->ref, file->ref_length);
have_saved_rowid= TRUE; have_saved_rowid= TRUE;
...@@ -433,9 +451,16 @@ void Mrr_ordered_index_reader::position() ...@@ -433,9 +451,16 @@ void Mrr_ordered_index_reader::position()
void Mrr_ordered_index_reader::resume_read() void Mrr_ordered_index_reader::resume_read()
{ {
key_restore(file->get_table()->record[0], saved_key_tuple, TABLE *table= file->get_table();
&file->get_table()->key_info[file->active_index], key_restore(table->record[0], saved_key_tuple,
&table->key_info[file->active_index],
keypar.key_tuple_length); keypar.key_tuple_length);
if (saved_primary_key)
{
key_restore(table->record[0], saved_primary_key,
&table->key_info[table->s->primary_key],
table->key_info[table->s->primary_key].key_length);
}
} }
...@@ -819,9 +844,17 @@ int DsMrr_impl::dsmrr_init(handler *h_arg, RANGE_SEQ_IF *seq_funcs, ...@@ -819,9 +844,17 @@ int DsMrr_impl::dsmrr_init(handler *h_arg, RANGE_SEQ_IF *seq_funcs,
/* Ordered index reader needs some space to store an index tuple */ /* Ordered index reader needs some space to store an index tuple */
if (strategy != index_strategy) if (strategy != index_strategy)
{ {
uint saved_pk_length=0;
if (h_idx->primary_key_is_clustered())
{
uint pk= h_idx->get_table()->s->primary_key;
saved_pk_length= h_idx->get_table()->key_info[pk].key_length;
}
if (reader_factory.ordered_index_reader. if (reader_factory.ordered_index_reader.
set_interruption_temp_buffer(primary_file->ref_length, set_interruption_temp_buffer(primary_file->ref_length,
keypar.key_tuple_length, keypar.key_tuple_length,
saved_pk_length,
&full_buf, full_buf_end)) &full_buf, full_buf_end))
goto use_default_impl; goto use_default_impl;
} }
......
...@@ -272,6 +272,7 @@ public: ...@@ -272,6 +272,7 @@ public:
} }
bool set_interruption_temp_buffer(uint rowid_length, uint key_len, bool set_interruption_temp_buffer(uint rowid_length, uint key_len,
uint saved_pk_len,
uchar **space_start, uchar *space_end); uchar **space_start, uchar *space_end);
void set_no_interruption_temp_buffer(); void set_no_interruption_temp_buffer();
...@@ -301,19 +302,27 @@ private: ...@@ -301,19 +302,27 @@ private:
/* TRUE == reached eof when enumerating ranges */ /* TRUE == reached eof when enumerating ranges */
bool source_exhausted; bool source_exhausted;
bool support_scan_interruptions;
/* /*
Space where we save the rowid of the last record we've returned. This is Following members are for interrupt_read()/resume_read(). The idea is that
needed for the cases where index scan is interrupted by some other activity in some cases index scan that is done by this object is interrupted by
that destroys contents in file->record[0] (which some storage engines use rnd_pos() calls made by Mrr_ordered_rndpos_reader. The problem is that
to store the last rowid value) we're sharing handler->record[0] with that object, and it destroys its
contents.
We need to save/restore our current
- index tuple (for pushed index condition checks)
- clustered primary key values (again, for pushed index condition checks)
- rowid of the last record we've retrieved (in case this rowid matches
multiple ranges and we'll need to return it again)
*/ */
bool support_scan_interruptions;
/* Space where we save the rowid of the last record we've returned */
uchar *saved_rowid; uchar *saved_rowid;
/* TRUE <=> saved_rowid has the last saved rowid */ /* TRUE <=> saved_rowid has the last saved rowid */
bool have_saved_rowid; bool have_saved_rowid;
uchar *saved_key_tuple; uchar *saved_key_tuple; /* Saved current key tuple */
uchar *saved_primary_key; /* Saved current primary key tuple */
static int compare_keys(void* arg, uchar* key1, uchar* key2); static int compare_keys(void* arg, uchar* key1, uchar* key2);
static int compare_keys_reverse(void* arg, uchar* key1, uchar* key2); static int compare_keys_reverse(void* arg, uchar* key1, uchar* key2);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment