MySQL Lists are EOL. Please join:

List:Commits« Previous MessageNext Message »
From:Patrick Galbraith Date:July 11 2006 1:51am
Subject:bk commit into 5.0 tree (patg:1.2219) BUG#18764
View as plain text  
Below is the list of changes that have just been committed into a local
5.0 repository of patg. When patg does a push these changes will
be propagated to the main repository and, within 24 hours after the
push, to the public repository.
For information on how to access the public repository
see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html

ChangeSet
  1.2219 06/07/10 18:51:17 patg@stripped +3 -0
  BUG #18764
  
  Removed query_id logic from write_row. See comments for ha_federated.cc

  sql/ha_federated.cc
    1.63 06/07/10 18:51:03 patg@stripped +24 -67
    BUG #18764
    
    Removed all query_id logic in write_row. This logic used to be required because
    fields would have values persisting from a previous insert if a subsequent insert
    did not specify that field. This logic is no longer needed. Also removed 'has_fields'
    variable as this variable can be satisfied with checking table->s->fields.
    
    IMPORTANT
    ---------
    Do not merge the changes in ha_federated.cc to 5.1, except the changes in 
    t/federated.test and r/federated.result. In 5.1, Monty cleaned up the logic 
    by using 
    
    my_bitmap_map *old_map= dbug_tmp_use_all_columns(table, table->read_set);

  mysql-test/t/federated.test
    1.28 06/07/10 18:51:03 patg@stripped +57 -0
    BUG #18764
    
    New test to test that delete_row followed by immediate write_row works

  mysql-test/r/federated.result
    1.32 06/07/10 18:51:03 patg@stripped +38 -0
    BUG #18764
    
    New results 

# This is a BitKeeper patch.  What follows are the unified diffs for the
# set of deltas contained in the patch.  The rest of the patch, the part
# that BitKeeper cares about, is below these diffs.
# User:	patg
# Host:	govinda.patg.net
# Root:	/home/patg/mysql-build/mysql-5.0-bug18764.2

--- 1.31/mysql-test/r/federated.result	2006-06-29 08:35:55 -07:00
+++ 1.32/mysql-test/r/federated.result	2006-07-10 18:51:03 -07:00
@@ -1689,6 +1689,44 @@
 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@stripped: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;

--- 1.27/mysql-test/t/federated.test	2006-06-29 08:35:55 -07:00
+++ 1.28/mysql-test/t/federated.test	2006-07-10 18:51:03 -07:00
@@ -1365,4 +1365,61 @@
 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@stripped:$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;

--- 1.62/sql/ha_federated.cc	2006-06-28 10:11:39 -07:00
+++ 1.63/sql/ha_federated.cc	2006-07-10 18:51:03 -07:00
@@ -1559,10 +1559,6 @@
 
 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,24 +1576,12 @@
   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
   */
   insert_string.append(FEDERATED_INSERT);
@@ -1610,63 +1594,36 @@
   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 @@
     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));
Thread
bk commit into 5.0 tree (patg:1.2219) BUG#18764Patrick Galbraith14 Jul