mirror of
https://github.com/MariaDB/server.git
synced 2025-01-17 12:32:27 +01:00
"BUG #18764: Delete conditions causing inconsistencies in Federated tables"
Removed logic in ha_federated::write_row, which checks field query ids in the loop which builds the query to run on the remote server. mysql-test/r/federated.result: "BUG #18764: Delete conditions causing inconsistencies in Federated tables" New test results for test that verifies that one can insert to rows using "insert into... select * from..", delete them by id, then immediately insert them in the same way they were originally inserted. mysql-test/t/federated.test: "BUG #18764: Delete conditions causing inconsistencies in Federated tables" New test that verifies that one can insert to rows using "insert into... select * from..", delete them by id, then immediately insert them in the same way they were originally inserted. sql/ha_federated.cc: "BUG #18764: Delete conditions causing inconsistencies in Federated tables" Removed the logic in ha_federated::write_row which checked the query id of each field and compared it to the thread query id. Each field has a query id, and the problem used to be that if I did an insert no fields specified, the field value would contain the last inserted value for that field. The way to work around this was to see if the query id for that field was the same as the current query id or of the rest of the field query ids. If it wasn't, that told me the query didn't have the field value specified. Somewhere from when I wrote that code to now the problem went away, and there was no longer the need for this logic. Also removed the bool "has_fields", which needs not exist and using table->s->fields is sufficient.
This commit is contained in:
parent
5ac016a814
commit
469813c7f3
3 changed files with 119 additions and 67 deletions
|
@ -1689,6 +1689,44 @@ id c1 c2
|
|||
9 abc ppc
|
||||
drop table federated.t1, federated.t2;
|
||||
drop table federated.t1, federated.t2;
|
||||
DROP TABLE IF EXISTS federated.test;
|
||||
CREATE TABLE federated.test (
|
||||
`id` int(11) NOT NULL,
|
||||
`val1` varchar(255) NOT NULL,
|
||||
`val2` varchar(255) NOT NULL,
|
||||
PRIMARY KEY (`id`)
|
||||
) ENGINE=MyISAM DEFAULT CHARSET=latin1;
|
||||
DROP TABLE IF EXISTS federated.test_local;
|
||||
DROP TABLE IF EXISTS federated.test_remote;
|
||||
CREATE TABLE federated.test_local (
|
||||
`id` int(11) NOT NULL,
|
||||
`val1` varchar(255) NOT NULL,
|
||||
`val2` varchar(255) NOT NULL,
|
||||
PRIMARY KEY (`id`)
|
||||
) ENGINE=MyISAM DEFAULT CHARSET=latin1;
|
||||
INSERT INTO federated.test_local VALUES (1, 'foo', 'bar'),
|
||||
(2, 'bar', 'foo');
|
||||
CREATE TABLE federated.test_remote (
|
||||
`id` int(11) NOT NULL,
|
||||
`val1` varchar(255) NOT NULL,
|
||||
`val2` varchar(255) NOT NULL,
|
||||
PRIMARY KEY (`id`)
|
||||
) ENGINE=FEDERATED DEFAULT CHARSET=latin1
|
||||
CONNECTION='mysql://root@127.0.0.1:SLAVE_PORT/federated/test';
|
||||
insert into federated.test_remote select * from federated.test_local;
|
||||
select * from federated.test_remote;
|
||||
id val1 val2
|
||||
1 foo bar
|
||||
2 bar foo
|
||||
delete from federated.test_remote where id in (1,2);
|
||||
insert into federated.test_remote select * from federated.test_local;
|
||||
select * from federated.test_remote;
|
||||
id val1 val2
|
||||
2 bar foo
|
||||
1 foo bar
|
||||
DROP TABLE federated.test_local;
|
||||
DROP TABLE federated.test_remote;
|
||||
DROP TABLE federated.test;
|
||||
DROP TABLE IF EXISTS federated.t1;
|
||||
DROP DATABASE IF EXISTS federated;
|
||||
DROP TABLE IF EXISTS federated.t1;
|
||||
|
|
|
@ -1365,4 +1365,61 @@ drop table federated.t1, federated.t2;
|
|||
connection slave;
|
||||
drop table federated.t1, federated.t2;
|
||||
|
||||
#
|
||||
# BUG #18764: Delete conditions causing inconsistencies in Federated tables
|
||||
#
|
||||
connection slave;
|
||||
--disable_warnings
|
||||
DROP TABLE IF EXISTS federated.test;
|
||||
--enable_warnings
|
||||
CREATE TABLE federated.test (
|
||||
`id` int(11) NOT NULL,
|
||||
`val1` varchar(255) NOT NULL,
|
||||
`val2` varchar(255) NOT NULL,
|
||||
PRIMARY KEY (`id`)
|
||||
) ENGINE=MyISAM DEFAULT CHARSET=latin1;
|
||||
|
||||
connection master;
|
||||
--disable_warnings
|
||||
DROP TABLE IF EXISTS federated.test_local;
|
||||
DROP TABLE IF EXISTS federated.test_remote;
|
||||
--enable_warnings
|
||||
CREATE TABLE federated.test_local (
|
||||
`id` int(11) NOT NULL,
|
||||
`val1` varchar(255) NOT NULL,
|
||||
`val2` varchar(255) NOT NULL,
|
||||
PRIMARY KEY (`id`)
|
||||
) ENGINE=MyISAM DEFAULT CHARSET=latin1;
|
||||
|
||||
INSERT INTO federated.test_local VALUES (1, 'foo', 'bar'),
|
||||
(2, 'bar', 'foo');
|
||||
|
||||
--replace_result $SLAVE_MYPORT SLAVE_PORT
|
||||
eval CREATE TABLE federated.test_remote (
|
||||
`id` int(11) NOT NULL,
|
||||
`val1` varchar(255) NOT NULL,
|
||||
`val2` varchar(255) NOT NULL,
|
||||
PRIMARY KEY (`id`)
|
||||
) ENGINE=FEDERATED DEFAULT CHARSET=latin1
|
||||
CONNECTION='mysql://root@127.0.0.1:$SLAVE_MYPORT/federated/test';
|
||||
|
||||
insert into federated.test_remote select * from federated.test_local;
|
||||
|
||||
select * from federated.test_remote;
|
||||
|
||||
delete from federated.test_remote where id in (1,2);
|
||||
|
||||
insert into federated.test_remote select * from federated.test_local;
|
||||
|
||||
select * from federated.test_remote;
|
||||
--disable_warnings
|
||||
DROP TABLE federated.test_local;
|
||||
DROP TABLE federated.test_remote;
|
||||
--enable_warnings
|
||||
|
||||
connection slave;
|
||||
--disable_warnings
|
||||
DROP TABLE federated.test;
|
||||
--enable_warnings
|
||||
|
||||
source include/federated_cleanup.inc;
|
||||
|
|
|
@ -1559,10 +1559,6 @@ inline uint field_in_record_is_null(TABLE *table,
|
|||
|
||||
int ha_federated::write_row(byte *buf)
|
||||
{
|
||||
bool has_fields= FALSE;
|
||||
uint all_fields_have_same_query_id= 1;
|
||||
ulong current_query_id= 1;
|
||||
ulong tmp_query_id= 1;
|
||||
char insert_buffer[FEDERATED_QUERY_BUFFER_SIZE];
|
||||
char values_buffer[FEDERATED_QUERY_BUFFER_SIZE];
|
||||
char insert_field_value_buffer[STRING_BUFFER_USUAL_SIZE];
|
||||
|
@ -1580,23 +1576,11 @@ int ha_federated::write_row(byte *buf)
|
|||
insert_string.length(0);
|
||||
insert_field_value_string.length(0);
|
||||
DBUG_ENTER("ha_federated::write_row");
|
||||
DBUG_PRINT("info",
|
||||
("table charset name %s csname %s",
|
||||
table->s->table_charset->name,
|
||||
table->s->table_charset->csname));
|
||||
|
||||
statistic_increment(table->in_use->status_var.ha_write_count, &LOCK_status);
|
||||
if (table->timestamp_field_type & TIMESTAMP_AUTO_SET_ON_INSERT)
|
||||
table->timestamp_field->set_time();
|
||||
|
||||
/*
|
||||
get the current query id - the fields that we add to the insert
|
||||
statement to send to the foreign will not be appended unless they match
|
||||
this query id
|
||||
*/
|
||||
current_query_id= table->in_use->query_id;
|
||||
DBUG_PRINT("info", ("current query id %d", current_query_id));
|
||||
|
||||
/*
|
||||
start both our field and field values strings
|
||||
*/
|
||||
|
@ -1609,64 +1593,37 @@ int ha_federated::write_row(byte *buf)
|
|||
values_string.append(FEDERATED_VALUES);
|
||||
values_string.append(FEDERATED_OPENPAREN);
|
||||
|
||||
/*
|
||||
Even if one field is different, all_fields_same_query_id can't remain
|
||||
0 if it remains 0, then that means no fields were specified in the query
|
||||
such as in the case of INSERT INTO table VALUES (val1, val2, valN)
|
||||
|
||||
*/
|
||||
for (field= table->field; *field; field++)
|
||||
{
|
||||
if (field > table->field && tmp_query_id != (*field)->query_id)
|
||||
all_fields_have_same_query_id= 0;
|
||||
|
||||
tmp_query_id= (*field)->query_id;
|
||||
}
|
||||
/*
|
||||
loop through the field pointer array, add any fields to both the values
|
||||
list and the fields list that match the current query id
|
||||
|
||||
You might ask "Why an index variable (has_fields) ?" My answer is that
|
||||
we need to count how many fields we actually need
|
||||
*/
|
||||
for (field= table->field; *field; field++)
|
||||
{
|
||||
/* if there is a query id and if it's equal to the current query id */
|
||||
if (((*field)->query_id && (*field)->query_id == current_query_id)
|
||||
|| all_fields_have_same_query_id)
|
||||
if ((*field)->is_null())
|
||||
insert_field_value_string.append(FEDERATED_NULL);
|
||||
else
|
||||
{
|
||||
/*
|
||||
There are some fields. This will be used later to determine
|
||||
whether to chop off commas and parens.
|
||||
*/
|
||||
has_fields= TRUE;
|
||||
|
||||
if ((*field)->is_null())
|
||||
insert_field_value_string.append(FEDERATED_NULL);
|
||||
else
|
||||
{
|
||||
(*field)->val_str(&insert_field_value_string);
|
||||
/* quote these fields if they require it */
|
||||
(*field)->quote_data(&insert_field_value_string);
|
||||
}
|
||||
/* append the field name */
|
||||
insert_string.append((*field)->field_name);
|
||||
|
||||
/* append the value */
|
||||
values_string.append(insert_field_value_string);
|
||||
insert_field_value_string.length(0);
|
||||
|
||||
/* append commas between both fields and fieldnames */
|
||||
/*
|
||||
unfortunately, we can't use the logic
|
||||
if *(fields + 1) to make the following
|
||||
appends conditional because we may not append
|
||||
if the next field doesn't match the condition:
|
||||
(((*field)->query_id && (*field)->query_id == current_query_id)
|
||||
*/
|
||||
insert_string.append(FEDERATED_COMMA);
|
||||
values_string.append(FEDERATED_COMMA);
|
||||
(*field)->val_str(&insert_field_value_string);
|
||||
/* quote these fields if they require it */
|
||||
(*field)->quote_data(&insert_field_value_string);
|
||||
}
|
||||
/* append the field name */
|
||||
insert_string.append((*field)->field_name);
|
||||
|
||||
/* append the value */
|
||||
values_string.append(insert_field_value_string);
|
||||
insert_field_value_string.length(0);
|
||||
|
||||
/* append commas between both fields and fieldnames */
|
||||
/*
|
||||
unfortunately, we can't use the logic
|
||||
if *(fields + 1) to make the following
|
||||
appends conditional because we may not append
|
||||
if the next field doesn't match the condition:
|
||||
(((*field)->query_id && (*field)->query_id == current_query_id)
|
||||
*/
|
||||
insert_string.append(FEDERATED_COMMA);
|
||||
values_string.append(FEDERATED_COMMA);
|
||||
}
|
||||
|
||||
/*
|
||||
|
@ -1678,7 +1635,7 @@ int ha_federated::write_row(byte *buf)
|
|||
AND, we don't want to chop off the last char '('
|
||||
insert will be "INSERT INTO t1 VALUES ();"
|
||||
*/
|
||||
if (has_fields)
|
||||
if (table->s->fields)
|
||||
{
|
||||
/* chops off leading commas */
|
||||
values_string.length(values_string.length() - strlen(FEDERATED_COMMA));
|
||||
|
|
Loading…
Reference in a new issue