1berry/MariaDB-server
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Bug#56423: Different count with SELECT and CREATE SELECT queries

Browse Source 
This is a regression from the fix for bug no 38999. A storage engine capable
of reading only a subset of a table's columns updates corresponding bits in
the read buffer to signal that it has read NULL values for the corresponding
columns. It cannot, and should not, update any other bits. Bug no 38999
occurred because the implementation of UPDATE statements compare the NULL bits
using memcmp, inadvertently comparing bits that were never requested from the
storage engine. The regression was caused by the storage engine trying to
alleviate the situation by writing to all NULL bits, even those that it had no
knowledge of. This has devastating effects for the index merge algorithm,
which relies on all NULL bits, except those explicitly requested, being left
unchanged.

The fix reverts the fix for bug no 38999 in both InnoDB and InnoDB plugin and
changes the server's method of comparing records. For engines that always read
entire rows, we proceed as usual. For engines capable of reading only select
columns, the record buffers are now compared on a column by column basis. An
assertion was also added so that non comparable buffers are never read. Some
relevant copy-pasted code was also consolidated in a new function.
This commit is contained in:
Martin Hansson 2010-10-07 10:13:11 +02:00
parent 19a7cf49cc
commit 30f57b3323
8 changed files with 227 additions and 47 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

52
mysql-test/include/index_merge2.inc
View File

@ -343,3 +343,55 @@ explain select * from t1 where (key3 > 30 and key3<35) or (key2 >32 and key2 < 4
select * from t1 where (key3 > 30 and key3<35) or (key2 >32 and key2 < 40);
drop table t1;
--echo #
--echo # Bug#56423: Different count with SELECT and CREATE SELECT queries
--echo #
CREATE TABLE t1 (
a INT,
b INT,
c INT,
d INT,
PRIMARY KEY (a),
KEY (c),
KEY bd (b,d)
);
INSERT INTO t1 VALUES
(1, 0, 1, 0),
(2, 1, 1, 1),
(3, 1, 1, 1),
(4, 0, 1, 1);
EXPLAIN
SELECT a
FROM t1
WHERE c = 1 AND b = 1 AND d = 1;
CREATE TABLE t2 ( a INT )
SELECT a
FROM t1
WHERE c = 1 AND b = 1 AND d = 1;
SELECT * FROM t2;
DROP TABLE t1, t2;
CREATE TABLE t1( a INT, b INT, KEY(a), KEY(b) );
INSERT INTO t1 VALUES (1, 2), (1, 2), (1, 2), (1, 2);
SELECT * FROM t1 FORCE INDEX(a, b) WHERE a = 1 AND b = 2;
DROP TABLE t1;
--echo # Code coverage of fix.
CREATE TABLE t1 ( a INT NOT NULL AUTO_INCREMENT PRIMARY KEY, b INT);
INSERT INTO t1 (b) VALUES (1);
UPDATE t1 SET b = 2 WHERE a = 1;
SELECT * FROM t1;
CREATE TABLE t2 ( a INT NOT NULL AUTO_INCREMENT PRIMARY KEY, b VARCHAR(1) );
INSERT INTO t2 (b) VALUES ('a');
UPDATE t2 SET b = 'b' WHERE a = 1;
SELECT * FROM t2;
DROP TABLE t1, t2;

55
mysql-test/r/index_merge_innodb.result
View File

@ -324,6 +324,61 @@ key1 key2 key3
38 38 38
39 39 39
drop table t1;
#
# Bug#56423: Different count with SELECT and CREATE SELECT queries
#
CREATE TABLE t1 (
a INT,
b INT,
c INT,
d INT,
PRIMARY KEY (a),
KEY (c),
KEY bd (b,d)
);
INSERT INTO t1 VALUES
(1, 0, 1, 0),
(2, 1, 1, 1),
(3, 1, 1, 1),
(4, 0, 1, 1);
EXPLAIN
SELECT a
FROM t1
WHERE c = 1 AND b = 1 AND d = 1;
id select_type table type possible_keys key key_len ref rows Extra
1 SIMPLE t1 index_merge c,bd c,bd 5,10 NULL 1 Using intersect(c,bd); Using where; Using index
CREATE TABLE t2 ( a INT )
SELECT a
FROM t1
WHERE c = 1 AND b = 1 AND d = 1;
SELECT * FROM t2;
a
2
3
DROP TABLE t1, t2;
CREATE TABLE t1( a INT, b INT, KEY(a), KEY(b) );
INSERT INTO t1 VALUES (1, 2), (1, 2), (1, 2), (1, 2);
SELECT * FROM t1 FORCE INDEX(a, b) WHERE a = 1 AND b = 2;
a b
1 2
1 2
1 2
1 2
DROP TABLE t1;
# Code coverage of fix.
CREATE TABLE t1 ( a INT NOT NULL AUTO_INCREMENT PRIMARY KEY, b INT);
INSERT INTO t1 (b) VALUES (1);
UPDATE t1 SET b = 2 WHERE a = 1;
SELECT * FROM t1;
a b
1 2
CREATE TABLE t2 ( a INT NOT NULL AUTO_INCREMENT PRIMARY KEY, b VARCHAR(1) );
INSERT INTO t2 (b) VALUES ('a');
UPDATE t2 SET b = 'b' WHERE a = 1;
SELECT * FROM t2;
a b
1 b
DROP TABLE t1, t2;
#---------------- 2-sweeps read Index merge test 2 -------------------------------
SET SESSION STORAGE_ENGINE = InnoDB;
drop table if exists t1;

55
mysql-test/r/index_merge_myisam.result
View File

@ -1158,6 +1158,61 @@ key1 key2 key3
38 38 38
39 39 39
drop table t1;
#
# Bug#56423: Different count with SELECT and CREATE SELECT queries
#
CREATE TABLE t1 (
a INT,
b INT,
c INT,
d INT,
PRIMARY KEY (a),
KEY (c),
KEY bd (b,d)
);
INSERT INTO t1 VALUES
(1, 0, 1, 0),
(2, 1, 1, 1),
(3, 1, 1, 1),
(4, 0, 1, 1);
EXPLAIN
SELECT a
FROM t1
WHERE c = 1 AND b = 1 AND d = 1;
id select_type table type possible_keys key key_len ref rows Extra
1 SIMPLE t1 ref c,bd bd 10 const,const 2 Using where
CREATE TABLE t2 ( a INT )
SELECT a
FROM t1
WHERE c = 1 AND b = 1 AND d = 1;
SELECT * FROM t2;
a
2
3
DROP TABLE t1, t2;
CREATE TABLE t1( a INT, b INT, KEY(a), KEY(b) );
INSERT INTO t1 VALUES (1, 2), (1, 2), (1, 2), (1, 2);
SELECT * FROM t1 FORCE INDEX(a, b) WHERE a = 1 AND b = 2;
a b
1 2
1 2
1 2
1 2
DROP TABLE t1;
# Code coverage of fix.
CREATE TABLE t1 ( a INT NOT NULL AUTO_INCREMENT PRIMARY KEY, b INT);
INSERT INTO t1 (b) VALUES (1);
UPDATE t1 SET b = 2 WHERE a = 1;
SELECT * FROM t1;
a b
1 2
CREATE TABLE t2 ( a INT NOT NULL AUTO_INCREMENT PRIMARY KEY, b VARCHAR(1) );
INSERT INTO t2 (b) VALUES ('a');
UPDATE t2 SET b = 'b' WHERE a = 1;
SELECT * FROM t2;
a b
1 b
DROP TABLE t1, t2;
#---------------- 2-sweeps read Index merge test 2 -------------------------------
SET SESSION STORAGE_ENGINE = MyISAM;
drop table if exists t1;

3
sql/mysql_priv.h
View File

@ -1047,7 +1047,8 @@ bool dispatch_command(enum enum_server_command command, THD *thd,
char* packet, uint packet_length);
void log_slow_statement(THD *thd);
bool check_dup(const char *db, const char *name, TABLE_LIST *tables);
bool compare_record(TABLE *table);
bool records_are_comparable(const TABLE *table);
bool compare_records(const TABLE *table);
bool append_file_to_dir(THD *thd, const char **filename_ptr,
const char *table_name);
void wait_while_table_is_used(THD *thd, TABLE *table,

4
sql/sql_insert.cc
View File

@ -1483,9 +1483,7 @@ int write_record(THD *thd, TABLE *table,COPY_INFO *info)
table->file->adjust_next_insert_id_after_explicit_value(
table->next_number_field->val_int());
info->touched++;
if ((table->file->ha_table_flags() & HA_PARTIAL_COLUMN_READ &&
!bitmap_is_subset(table->write_set, table->read_set)) ||
compare_record(table))
if (!records_are_comparable(table) || compare_records(table))
{
if ((error=table->file->ha_update_row(table->record[1],
table->record[0])) &&

93
sql/sql_update.cc
View File

@ -25,11 +25,68 @@
#include "sql_trigger.h"
#include "debug_sync.h"
/* Return 0 if row hasn't changed */
bool compare_record(TABLE *table)
/**
True if the table's input and output record buffers are comparable using
compare_records(TABLE*).
*/
bool records_are_comparable(const TABLE *table) {
return ((table->file->ha_table_flags() & HA_PARTIAL_COLUMN_READ) == 0) ||
bitmap_is_subset(table->write_set, table->read_set);
}
/**
Compares the input and outbut record buffers of the table to see if a row
has changed. The algorithm iterates over updated columns and if they are
nullable compares NULL bits in the buffer before comparing actual
data. Special care must be taken to compare only the relevant NULL bits and
mask out all others as they may be undefined. The storage engine will not
and should not touch them.
@param table The table to evaluate.
@return true if row has changed.
@return false otherwise.
*/
bool compare_records(const TABLE *table)
{
DBUG_ASSERT(records_are_comparable(table));
if ((table->file->ha_table_flags() & HA_PARTIAL_COLUMN_READ) != 0)
{
/*
Storage engine may not have read all columns of the record. Fields
(including NULL bits) not in the write_set may not have been read and
can therefore not be compared.
*/
for (Field **ptr= table->field ; *ptr != NULL; ptr++)
{
Field *field= *ptr;
if (bitmap_is_set(table->write_set, field->field_index))
{
if (field->real_maybe_null())
{
uchar null_byte_index= field->null_ptr - table->record[0];
if (((table->record[0][null_byte_index]) & field->null_bit) !=
((table->record[1][null_byte_index]) & field->null_bit))
return TRUE;
}
if (field->cmp_binary_offset(table->s->rec_buff_length))
return TRUE;
}
}
return FALSE;
}
/*
The storage engine has read all columns, so it's safe to compare all bits
including those not in the write_set. This is cheaper than the field-by-field
comparison done above.
*/
if (table->s->blob_fields + table->s->varchar_fields == 0)
// Fixed-size record: do bitwise comparison of the records
return cmp_record(table,record[1]);
/* Compare null bits */
if (memcmp(table->null_flags,
@ -186,7 +243,6 @@ int mysql_update(THD *thd,
bool using_limit= limit != HA_POS_ERROR;
bool safe_update= test(thd->options & OPTION_SAFE_UPDATES);
bool used_key_is_modified, transactional_table, will_batch;
bool can_compare_record;
int res;
int error, loc_error;
uint used_index= MAX_KEY, dup_key_found;
@ -575,15 +631,6 @@ int mysql_update(THD *thd,
if (table->file->ha_table_flags() & HA_PARTIAL_COLUMN_READ)
table->prepare_for_position();
/*
We can use compare_record() to optimize away updates if
the table handler is returning all columns OR if
if all updated columns are read
*/
can_compare_record= (!(table->file->ha_table_flags() &
HA_PARTIAL_COLUMN_READ) ||
bitmap_is_subset(table->write_set, table->read_set));
while (!(error=info.read_record(&info)) && !thd->killed)
{
thd->examined_row_count++;
@ -601,7 +648,7 @@ int mysql_update(THD *thd,
found++;
if (!can_compare_record || compare_record(table))
if (!records_are_comparable(table) || compare_records(table))
{
if ((res= table_list->view_check_option(thd, ignore)) !=
VIEW_CHECK_OK)
@ -1695,18 +1742,8 @@ bool multi_update::send_data(List<Item> &not_used_values)
if (table->status & (STATUS_NULL_ROW | STATUS_UPDATED))
continue;
/*
We can use compare_record() to optimize away updates if
the table handler is returning all columns OR if
if all updated columns are read
*/
if (table == table_to_update)
{
bool can_compare_record;
can_compare_record= (!(table->file->ha_table_flags() &
HA_PARTIAL_COLUMN_READ) ||
bitmap_is_subset(table->write_set,
table->read_set));
table->status|= STATUS_UPDATED;
store_record(table,record[1]);
if (fill_record_n_invoke_before_triggers(thd, *fields_for_table[offset],
@ -1721,7 +1758,7 @@ bool multi_update::send_data(List<Item> &not_used_values)
*/
table->auto_increment_field_not_null= FALSE;
found++;
if (!can_compare_record || compare_record(table))
if (!records_are_comparable(table) || compare_records(table))
{
int error;
if ((error= cur_table->view_check_option(thd, ignore)) !=
@ -1908,7 +1945,6 @@ int multi_update::do_updates()
DBUG_RETURN(0);
for (cur_table= update_tables; cur_table; cur_table= cur_table->next_local)
{
bool can_compare_record;
uint offset= cur_table->shared;
table = cur_table->table;
@ -1945,11 +1981,6 @@ int multi_update::do_updates()
if ((local_error = tmp_table->file->ha_rnd_init(1)))
goto err;
can_compare_record= (!(table->file->ha_table_flags() &
HA_PARTIAL_COLUMN_READ) ||
bitmap_is_subset(table->write_set,
table->read_set));
for (;;)
{
if (thd->killed && trans_safe)
@ -1990,7 +2021,7 @@ int multi_update::do_updates()
TRG_ACTION_BEFORE, TRUE))
goto err2;
if (!can_compare_record || compare_record(table))
if (!records_are_comparable(table) || compare_records(table))
{
int error;
if ((error= cur_table->view_check_option(thd, ignore)) !=

6
storage/innobase/row/row0sel.c
View File

@ -2621,12 +2621,6 @@ row_sel_store_mysql_rec(
prebuilt->blob_heap = NULL;
}
/* init null bytes with default values as they might be
left uninitialized in some cases and this uninited bytes
might be copied into mysql record buffer that leads to
valgrind warnings */
memcpy(mysql_rec, prebuilt->default_rec, prebuilt->null_bitmap_len);
for (i = 0; i < prebuilt->n_template; i++) {
templ = prebuilt->mysql_template + i;

6
storage/innodb_plugin/row/row0sel.c
View File

@ -2696,12 +2696,6 @@ row_sel_store_mysql_rec(
prebuilt->blob_heap = NULL;
}
/* init null bytes with default values as they might be
left uninitialized in some cases and these uninited bytes
might be copied into mysql record buffer that leads to
valgrind warnings */
memcpy(mysql_rec, prebuilt->default_rec, prebuilt->null_bitmap_len);
for (i = 0; i < prebuilt->n_template; i++) {
templ = prebuilt->mysql_template + i;