bk commit into 5.0 tree (patg:1.1805)

[Patrick Galbraith]

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.1805 05/03/09 00:41:25 patg@krsna. +4 -0
  WL# 2094 Federated Storage Engine
  
  Bug fixes Kostja sent a patch for, which pointed out problems which he 
  found with upon pushing change that had problems with non-blocking read
  and caused federated to crash. Also, new table creation error handling
  code and associated tests/results.

  BitKeeper/etc/logging_ok
    1.296 05/03/09 00:41:24 patg@krsna. +1 -0
    Logging to logging@stripped accepted

  sql/ha_federated.cc
    1.19 05/03/09 00:31:15 patg@krsna. +111 -9
    Fixes to:
    
    - error handling 
    - order in which table name is appended to rnd_init query
    - returning an error if no rows on rnd_init
    - more arguments to my_error on call with ER_CANT_CREATE_TABLE
    - removal of ptrhead_mutex_destroy from error: block, which would cause
      its call _before_ the mutex is init'ed 

  mysql-test/t/federated.test
    1.6 05/03/09 00:31:15 patg@krsna. +85 -5
    Added tests for checking to see if error handling code in handler deals 
    with 
    - malformed URLs during creation.
    - using different table names on local and 
    foreign tables
    - bad username/password/connection values (can't connect) during creation.

  mysql-test/r/federated.result
    1.7 05/03/09 00:31:15 patg@krsna. +71 -2
    new test results for error handling tests

# 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:	krsna.
# Root:	/home/patg/mysql-5.0

--- 1.295/BitKeeper/etc/logging_ok	Sat Mar  5 06:12:19 2005
+++ 1.296/BitKeeper/etc/logging_ok	Wed Mar  9 00:41:24 2005
@@ -179,6 +179,7 @@
 nick@stripped
 nick@stripped
 papa@stripped
+patg@krsna.
 patg@stripped
 patg@stripped
 patg@stripped

--- 1.6/mysql-test/r/federated.result	Wed Feb 23 00:38:23 2005
+++ 1.7/mysql-test/r/federated.result	Wed Mar  9 00:31:15 2005
@@ -8,6 +8,77 @@
 DROP DATABASE IF EXISTS federated;
 CREATE DATABASE federated;
 CREATE TABLE federated.t1 (
+`id` int(20) NOT NULL,
+`name` varchar(32) NOT NULL default ''
+    )
+DEFAULT CHARSET=latin1;
+DROP DATABASE IF EXISTS federated;
+CREATE DATABASE federated;
+CREATE TABLE federated.t1 (
+`id` int(20) NOT NULL,
+`name` varchar(32) NOT NULL default ''
+    )
+ENGINE="FEDERATED" DEFAULT CHARSET=latin1
+COMMENT='mysql://root@stripped:@/too/many/items/federated/t1';
+ERROR HY000: Can't create table 'this connection string is not in the correct format!
+' (errno: 0)
+CREATE TABLE federated.t1 (
+`id` int(20) NOT NULL,
+`name` varchar(32) NOT NULL default ''
+    )
+ENGINE="FEDERATED" DEFAULT CHARSET=latin1
+COMMENT='mysql://root@stripped';
+ERROR HY000: Can't create table 'this connection string is not in the correct format!
+' (errno: 0)
+CREATE TABLE federated.t1 (
+`id` int(20) NOT NULL,
+`name` varchar(32) NOT NULL default ''
+    )
+ENGINE="FEDERATED" DEFAULT CHARSET=latin1
+COMMENT='mysql://root@stripped:SLAVE_PORT/federated/t3';
+ERROR HY000: Error running query on master: foreign table 't3' does not exist!
+CREATE TABLE federated.t1 (
+`id` int(20) NOT NULL,
+`name` varchar(32) NOT NULL default ''
+    )
+ENGINE="FEDERATED" DEFAULT CHARSET=latin1
+COMMENT='mysql://user:pass@stripped:SLAVE_PORT/federated/t1';
+ERROR 08S01: Error connecting to master: unable to connect to database 'federated' on
host '127.0.0.1 as user 'user' !
+DROP TABLE IF EXISTS federated.t1;
+Warnings:
+Note	1051	Unknown table 't1'
+CREATE TABLE federated.t1 (
+`id` int(20) NOT NULL,
+`name` varchar(32) NOT NULL default ''
+    )
+ENGINE="FEDERATED" DEFAULT CHARSET=latin1
+COMMENT='mysql://root@stripped:SLAVE_PORT/federated/t1';
+INSERT INTO federated.t1 (id, name) VALUES (1, 'foo');
+INSERT INTO federated.t1 (id, name) VALUES (2, 'fee');
+SELECT * FROM federated.t1;
+id	name
+1	foo
+2	fee
+DELETE FROM federated.t1;
+DROP TABLE federated.t1;
+DROP TABLE IF EXISTS federated.t2;
+Warnings:
+Note	1051	Unknown table 't2'
+CREATE TABLE federated.t2 (
+`id` int(20) NOT NULL,
+`name` varchar(32) NOT NULL default ''
+    )
+ENGINE="FEDERATED" DEFAULT CHARSET=latin1
+COMMENT='mysql://root@stripped:SLAVE_PORT/federated/t1';
+INSERT INTO federated.t2 (id, name) VALUES (1, 'foo');
+INSERT INTO federated.t2 (id, name) VALUES (2, 'fee');
+SELECT * FROM federated.t2;
+id	name
+1	foo
+2	fee
+DROP TABLE federated.t2;
+DROP TABLE IF EXISTS federated.t1;
+CREATE TABLE federated.t1 (
 `id` int(20) NOT NULL auto_increment,
 `name` varchar(32) NOT NULL default '',
 `other` int(20) NOT NULL default '0',
@@ -16,8 +87,6 @@
 KEY `name` (`name`),
 KEY `other_key` (`other`))
 DEFAULT CHARSET=latin1;
-DROP DATABASE IF EXISTS federated;
-CREATE DATABASE federated;
 CREATE TABLE federated.t1 (
 `id` int(20) NOT NULL auto_increment,
 `name` varchar(32) NOT NULL default '',

--- 1.5/mysql-test/t/federated.test	Wed Feb 23 00:38:23 2005
+++ 1.6/mysql-test/t/federated.test	Wed Mar  9 00:31:15 2005
@@ -14,7 +14,92 @@
 --enable_warnings
 CREATE DATABASE federated;
 
+CREATE TABLE federated.t1 (
+    `id` int(20) NOT NULL,
+    `name` varchar(32) NOT NULL default ''
+    )
+  DEFAULT CHARSET=latin1;
+
+connection master;
+--disable_warnings
+DROP DATABASE IF EXISTS federated;
+--enable_warnings
+CREATE DATABASE federated;
+
+# test too many items (malformed) in the comment string url
+--error 1005
+eval CREATE TABLE federated.t1 (
+    `id` int(20) NOT NULL,
+    `name` varchar(32) NOT NULL default ''
+    )
+  ENGINE="FEDERATED" DEFAULT CHARSET=latin1
+  COMMENT='mysql://root@stripped:@/too/many/items/federated/t1';
+
+# test not enough items (malformed) in the comment string url
+--error 1005
+eval CREATE TABLE federated.t1 (
+    `id` int(20) NOT NULL,
+    `name` varchar(32) NOT NULL default ''
+    )
+  ENGINE="FEDERATED" DEFAULT CHARSET=latin1
+  COMMENT='mysql://root@stripped';
+
+# test non-existant table
+--replace_result $SLAVE_MYPORT SLAVE_PORT
+--error 1219 
+eval CREATE TABLE federated.t1 (
+    `id` int(20) NOT NULL,
+    `name` varchar(32) NOT NULL default ''
+    )
+  ENGINE="FEDERATED" DEFAULT CHARSET=latin1
+  COMMENT='mysql://root@stripped:$SLAVE_MYPORT/federated/t3';
+
+# test bad user/password 
+--replace_result $SLAVE_MYPORT SLAVE_PORT
+--error 1218
+eval CREATE TABLE federated.t1 (
+    `id` int(20) NOT NULL,
+    `name` varchar(32) NOT NULL default ''
+    )
+  ENGINE="FEDERATED" DEFAULT CHARSET=latin1
+  COMMENT='mysql://user:pass@stripped:$SLAVE_MYPORT/federated/t1';
+
+DROP TABLE IF EXISTS federated.t1;
+# correct connection, same named tables
+--replace_result $SLAVE_MYPORT SLAVE_PORT
+eval CREATE TABLE federated.t1 (
+    `id` int(20) NOT NULL,
+    `name` varchar(32) NOT NULL default ''
+    )
+  ENGINE="FEDERATED" DEFAULT CHARSET=latin1
+  COMMENT='mysql://root@stripped:$SLAVE_MYPORT/federated/t1';
+
+INSERT INTO federated.t1 (id, name) VALUES (1, 'foo');
+INSERT INTO federated.t1 (id, name) VALUES (2, 'fee');
+
+SELECT * FROM federated.t1;
+DELETE FROM federated.t1;
+DROP TABLE federated.t1;
+
+# correct connection, differently named tables
+DROP TABLE IF EXISTS federated.t2;
+--replace_result $SLAVE_MYPORT SLAVE_PORT
+eval CREATE TABLE federated.t2 (
+    `id` int(20) NOT NULL,
+    `name` varchar(32) NOT NULL default ''
+    )
+  ENGINE="FEDERATED" DEFAULT CHARSET=latin1
+  COMMENT='mysql://root@stripped:$SLAVE_MYPORT/federated/t1';
+
+INSERT INTO federated.t2 (id, name) VALUES (1, 'foo');
+INSERT INTO federated.t2 (id, name) VALUES (2, 'fee');
+
+SELECT * FROM federated.t2;
+DROP TABLE federated.t2;
+
+connection slave;
 # I wanted to use timestamp, but results will fail if so!!!
+DROP TABLE IF EXISTS federated.t1;
 CREATE TABLE federated.t1 (
     `id` int(20) NOT NULL auto_increment,
     `name` varchar(32) NOT NULL default '',
@@ -26,11 +111,6 @@
   DEFAULT CHARSET=latin1;
 
 connection master;
---disable_warnings
-DROP DATABASE IF EXISTS federated;
---enable_warnings
-CREATE DATABASE federated;
-
 --replace_result $SLAVE_MYPORT SLAVE_PORT
 eval CREATE TABLE federated.t1 (
     `id` int(20) NOT NULL auto_increment,

--- 1.18/sql/ha_federated.cc	Wed Feb 23 00:38:23 2005
+++ 1.19/sql/ha_federated.cc	Wed Mar  9 00:31:15 2005
@@ -414,6 +414,88 @@
   return FALSE;
 }
 
+
+/*
+ Check (in create) whether the tables exists, and that it can be connected to
+
+  SYNOPSIS
+    check_connection()
+      share               pointer to FEDERATED share
+
+  DESCRIPTION
+    This method first checks that the connection information that parse url
+    has populated into the share will be sufficient to connect to the foreign
+    table, and if so, does the foreign table exist.
+*/
+
+static int check_connection(FEDERATED_SHARE *share)
+{
+  MYSQL *mysql;
+  MYSQL_RES *result=0;
+  uint error_code;
+  char query_buffer[IO_SIZE];
+  char error_buffer[IO_SIZE];
+  String query(query_buffer, sizeof(query_buffer), &my_charset_bin);
+  DBUG_ENTER("ha_federated::check_connection");
+  query.length(0);
+
+  mysql= mysql_init(0);
+  /* check if we can connect */
+  if (!mysql_real_connect(mysql,
+                          share->hostname,
+                          share->username,
+                          share->password,
+                          share->database,
+                          share->port,
+                          share->socket, 0))
+  {
+    my_sprintf(error_buffer,
+               (error_buffer,
+                "unable to connect to database '%s' on host '%s as user '%s' !",
+               share->database, share->hostname, share->username));
+    error_code= ER_CONNECT_TO_MASTER;
+    goto error;
+  }
+  else
+  {
+    /* if we can connect, then make sure the table exists */
+    query.append("SHOW TABLES LIKE '");
+    query.append(share->table_base_name);
+    query.append("'");
+
+    error_code= ER_QUERY_ON_MASTER;
+    if (mysql_real_query(mysql, query.ptr(), query.length()))
+      goto error;
+
+    result= mysql_store_result(mysql);
+    if (! result)
+      goto error;
+
+    /* if ! mysql_num_rows, the table doesn't exist, send error */
+    if (! mysql_num_rows(result))
+    {
+      my_sprintf(error_buffer,
+                 (error_buffer, "foreign table '%s' does not exist!",
+                  share->table_base_name));
+      goto error;
+    }
+    mysql_free_result(result);
+    result= 0;
+    mysql_close(mysql);
+
+  }
+  DBUG_RETURN(0);
+
+error:
+    if (result)
+      mysql_free_result(result);
+    mysql_close(mysql);
+    my_error(error_code, MYF(0), error_buffer);
+    DBUG_RETURN(error_code);
+
+}
+
+
 /*
   Parse connection info from table->s->comment
 
@@ -521,6 +603,8 @@
         }
         else
           goto error;
+
+        share->table_base_name_length= strlen(share->table_base_name);
       }
       else
         goto error;
@@ -545,6 +629,15 @@
                   share->scheme, share->username, share->password,
                   share->hostname, share->port, share->database,
                   share->table_base_name));
+
+      /* If creation, check first if we can connect and that the table exists */
+      if (table_create_flag)
+      {
+        if (check_connection(share))
+          goto error;
+        /* free share->schema even if no error, since this is a create */
+        my_free((gptr) share->scheme, MYF(0));
+      }
     }
     else
       goto error;
@@ -557,10 +650,12 @@
 error:
     my_error(error_num, MYF(0),
              "this connection string is not in the correct format!\n");
+    my_free((gptr) share->scheme, MYF(0));
     DBUG_RETURN(1);
 
 }
 
+
 /*
   Convert MySQL result set row to handler internal format
 
@@ -779,14 +874,13 @@
   {
     query.set_charset(system_charset_info);
     query.append("SELECT * FROM ");
-    query.append(table_base_name);
 
     if (!(share= (FEDERATED_SHARE *)
           my_multi_malloc(MYF(MY_WME | MY_ZEROFILL),
                           &share, sizeof(*share),
                           &tmp_table_name, table_name_length + 1,
-                          &tmp_table_base_name, table_base_name_length + 1,
-                          &select_query, query.length() + 1, NullS)))
+                          &select_query, query.length() +
+                          strlen(table->s->comment) + 1,  NullS)))
     {
       pthread_mutex_unlock(&federated_mutex);
       return NULL;
@@ -795,14 +889,12 @@
     if (parse_url(share, table, 0))
       goto error;
 
+    query.append(share->table_base_name);
     share->use_count= 0;
     share->table_name_length= table_name_length;
     share->table_name= tmp_table_name;
-    share->table_base_name_length= table_base_name_length;
-    share->table_base_name= tmp_table_base_name;
     share->select_query= select_query;
     strmov(share->table_name, table_name);
-    strmov(share->table_base_name, table_base_name);
     strmov(share->select_query, query.ptr());
     DBUG_PRINT("ha_federated::get_share",
                ("share->select_query %s", share->select_query));
@@ -820,7 +912,6 @@
   pthread_mutex_unlock(&federated_mutex);
   if (share->scheme)
     my_free((gptr) share->scheme, MYF(0));
-  VOID(pthread_mutex_destroy(&share->mutex));
   my_free((gptr) share, MYF(0));
 
   return NULL;
@@ -1576,6 +1667,16 @@
   MYSQL_ROW row;
   DBUG_ENTER("ha_federated::rnd_next");
 
+  if (result == 0)
+  {
+    /*
+      Return value of rnd_init is not always checked (see records.cc),
+      so we can get here _even_ if there is _no_ pre-fetched result-set!
+      TODO: fix it.
+      */
+    DBUG_RETURN(1);
+  }
+ 
   /* Fetch a row, insert it back in a row format. */
   current_position= result->data_cursor;
   DBUG_PRINT("ha_federated::rnd_next",
@@ -1805,12 +1906,13 @@
 {
   FEDERATED_SHARE tmp;
   DBUG_ENTER("ha_federated::create");
+
   if (parse_url(&tmp, table_arg, 1))
   {
-    my_error(ER_CANT_CREATE_TABLE, MYF(0));
+    my_error(ER_CANT_CREATE_TABLE, MYF(0), name, 1);
     DBUG_RETURN(ER_CANT_CREATE_TABLE);
   }
-  my_free((gptr) tmp.scheme, MYF(0));
+
   DBUG_RETURN(0);
 }
 #endif /* HAVE_FEDERATED_DB */