From f3c3c9c34189a5ccdacf62aec50e55c197223148 Mon Sep 17 00:00:00 2001 From: unknown Date: Mon, 26 Jun 2006 16:59:52 -0700 Subject: [PATCH 1/2] Bug #16494: Updates that set a column to NULL fail sometimes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When building the UPDATE query to send to the remote server, the federated storage engine built the query incorrectly if it was updating a field to be NULL. Thanks to Bjšrn Steinbrink for an initial patch for the problem. mysql-test/r/federated.result: Add new results mysql-test/t/federated.test: Add new regression test sql/ha_federated.cc: Fix logic of how fields are added to SET and WHERE clauses of an UPDATE statement. Fields that were NULL were being handled incorrectly. Also reorganizes the code a little bit so the update of the two clauses is consistent. --- mysql-test/r/federated.result | 16 ++++++++++++++++ mysql-test/t/federated.test | 17 +++++++++++++++++ sql/ha_federated.cc | 17 +++++++---------- 3 files changed, 40 insertions(+), 10 deletions(-) diff --git a/mysql-test/r/federated.result b/mysql-test/r/federated.result index f11da4ee62f..48f20625826 100644 --- a/mysql-test/r/federated.result +++ b/mysql-test/r/federated.result @@ -1601,6 +1601,22 @@ fld_cid fld_name fld_parentid fld_delt 5 Torkel 0 0 DROP TABLE federated.t1; DROP TABLE federated.bug_17377_table; +create table t1 (id int not null auto_increment primary key, val int); +create table t1 +(id int not null auto_increment primary key, val int) engine=federated +connection='mysql://root@127.0.0.1:9308/test/t1'; +insert into t1 values (1,0),(2,0); +update t1 set val = NULL where id = 1; +select * from t1; +id val +1 NULL +2 0 +select * from t1; +id val +1 NULL +2 0 +drop table t1; +drop table t1; DROP TABLE IF EXISTS federated.t1; DROP DATABASE IF EXISTS federated; DROP TABLE IF EXISTS federated.t1; diff --git a/mysql-test/t/federated.test b/mysql-test/t/federated.test index 80b31c610a2..d24caf1aa08 100644 --- a/mysql-test/t/federated.test +++ b/mysql-test/t/federated.test @@ -1309,5 +1309,22 @@ DROP TABLE federated.t1; connection slave; DROP TABLE federated.bug_17377_table; +# +# Bug #16494: Updates that set a column to NULL fail sometimes +# +connection slave; +create table t1 (id int not null auto_increment primary key, val int); +connection master; +eval create table t1 + (id int not null auto_increment primary key, val int) engine=federated + connection='mysql://root@127.0.0.1:$SLAVE_MYPORT/test/t1'; +insert into t1 values (1,0),(2,0); +update t1 set val = NULL where id = 1; +select * from t1; +connection slave; +select * from t1; +drop table t1; +connection master; +drop table t1; source include/federated_cleanup.inc; diff --git a/sql/ha_federated.cc b/sql/ha_federated.cc index c6d5c77803b..9e9c23c0ccc 100644 --- a/sql/ha_federated.cc +++ b/sql/ha_federated.cc @@ -1857,8 +1857,8 @@ int ha_federated::update_row(const byte *old_data, byte *new_data) In this loop, we want to match column names to values being inserted (while building INSERT statement). - Iterate through table->field (new data) and share->old_filed (old_data) - using the same index to created an SQL UPDATE statement, new data is + Iterate through table->field (new data) and share->old_field (old_data) + using the same index to create an SQL UPDATE statement. New data is used to create SET field=value and old data is used to create WHERE field=oldvalue */ @@ -1870,30 +1870,28 @@ int ha_federated::update_row(const byte *old_data, byte *new_data) update_string.append(FEDERATED_EQ); if ((*field)->is_null()) - new_field_value.append(FEDERATED_NULL); + update_string.append(FEDERATED_NULL); else { /* otherwise = */ (*field)->val_str(&new_field_value); (*field)->quote_data(&new_field_value); - - if (!field_in_record_is_null(table, *field, (char*) old_data)) - where_string.append(FEDERATED_EQ); + update_string.append(new_field_value); + new_field_value.length(0); } if (field_in_record_is_null(table, *field, (char*) old_data)) where_string.append(FEDERATED_ISNULL); else { + where_string.append(FEDERATED_EQ); (*field)->val_str(&old_field_value, (char*) (old_data + (*field)->offset())); (*field)->quote_data(&old_field_value); where_string.append(old_field_value); + old_field_value.length(0); } - update_string.append(new_field_value); - new_field_value.length(0); - /* Only append conjunctions if we have another field in which to iterate @@ -1903,7 +1901,6 @@ int ha_federated::update_row(const byte *old_data, byte *new_data) update_string.append(FEDERATED_COMMA); where_string.append(FEDERATED_AND); } - old_field_value.length(0); } update_string.append(FEDERATED_WHERE); update_string.append(where_string); From 12e62a8e89c2c37268e7a0d845fa4029c7dd3e40 Mon Sep 17 00:00:00 2001 From: unknown Date: Wed, 28 Jun 2006 12:11:52 -0700 Subject: [PATCH 2/2] Optimize stack usage of ha_federated::update_row(). sql/ha_federated.cc: We only need one string buffer for fields in ::update_row() --- sql/ha_federated.cc | 33 +++++++++++++-------------------- 1 file changed, 13 insertions(+), 20 deletions(-) diff --git a/sql/ha_federated.cc b/sql/ha_federated.cc index 9e9c23c0ccc..c4e1549183a 100644 --- a/sql/ha_federated.cc +++ b/sql/ha_federated.cc @@ -1817,19 +1817,13 @@ int ha_federated::update_row(const byte *old_data, byte *new_data) /* buffers for following strings */ - char old_field_value_buffer[STRING_BUFFER_USUAL_SIZE]; - char new_field_value_buffer[STRING_BUFFER_USUAL_SIZE]; + char field_value_buffer[STRING_BUFFER_USUAL_SIZE]; char update_buffer[FEDERATED_QUERY_BUFFER_SIZE]; char where_buffer[FEDERATED_QUERY_BUFFER_SIZE]; - /* stores the value to be replaced of the field were are updating */ - String old_field_value(old_field_value_buffer, - sizeof(old_field_value_buffer), - &my_charset_bin); - /* stores the new value of the field */ - String new_field_value(new_field_value_buffer, - sizeof(new_field_value_buffer), - &my_charset_bin); + /* Work area for field values */ + String field_value(field_value_buffer, sizeof(field_value_buffer), + &my_charset_bin); /* stores the update query */ String update_string(update_buffer, sizeof(update_buffer), @@ -1842,8 +1836,7 @@ int ha_federated::update_row(const byte *old_data, byte *new_data) /* set string lengths to 0 to avoid misc chars in string */ - old_field_value.length(0); - new_field_value.length(0); + field_value.length(0); update_string.length(0); where_string.length(0); @@ -1874,10 +1867,10 @@ int ha_federated::update_row(const byte *old_data, byte *new_data) else { /* otherwise = */ - (*field)->val_str(&new_field_value); - (*field)->quote_data(&new_field_value); - update_string.append(new_field_value); - new_field_value.length(0); + (*field)->val_str(&field_value); + (*field)->quote_data(&field_value); + update_string.append(field_value); + field_value.length(0); } if (field_in_record_is_null(table, *field, (char*) old_data)) @@ -1885,11 +1878,11 @@ int ha_federated::update_row(const byte *old_data, byte *new_data) else { where_string.append(FEDERATED_EQ); - (*field)->val_str(&old_field_value, + (*field)->val_str(&field_value, (char*) (old_data + (*field)->offset())); - (*field)->quote_data(&old_field_value); - where_string.append(old_field_value); - old_field_value.length(0); + (*field)->quote_data(&field_value); + where_string.append(field_value); + field_value.length(0); } /*