From 0a48c5b5c230fe27f30c44038a37610f807897a1 Mon Sep 17 00:00:00 2001 From: "patg@krsna.patg.net" <> Date: Sun, 6 Feb 2005 09:40:07 -0800 Subject: [PATCH 1/4] WL# 2094 This patch contains all that my previous patch (1.1814) contained, with the addition of using cli_fetch_lengths for handling binary data (Bar noted this on the review of 1.1814, Guilhem suggested using cli_fetch_lenghts by making available via removal of static in method definition and declaration in mysql.h, but Konstantin had some reservations, but he said to commit the patch using this anyway, and I suppose this can be discussed. I abandoned 1.1814 because Monty made a couple fixes to my code as well as formatting changes, and I thought it would just be easier to hand-edit my changes into a fresh clone and then make a patch. The reason for using cli_fetch_lengths is so that I can correctly get the length of the field I am setting into the field. I was previously using 'strlen' but Bar pointed out this won't correctly get the length of binary data and is also less effecient. Upon testing, it was in fact verified that binary data in a blob table was being inserted correctly, but not being retrieved correctly, all due to not having the correct value for the field: (*field)->store(row[x], strlen(row[x]), &my_charset_bin); was changed to: (*field)->store(row[x], lengths[x], &my_charset_bin); lengths being a unsigned long pointer to the values of the field lengths from a MYSQL_ROW. Since the server doesn't have the function "mysql_fetch_lengths" available, I tried to use "result->lengths", but this isn't set, so I finally successfully used cli_fetch_lenghts, which does give the correct lengths, and now the binary data gets retrieved correctly. I've also run the code through indent-ex and am using Brian's vimrc to ensure correct formatting! This code passes the entire test suite, without any errors or warning on both my workstation and build.mysql.com --- include/mysql.h | 2 + mysql-test/r/federated.result | 26 +- mysql-test/t/federated.test | 33 +- sql-common/client.c | 2 +- sql/ha_federated.cc | 698 +++++++++++++++++++--------------- sql/ha_federated.h | 3 +- 6 files changed, 432 insertions(+), 332 deletions(-) diff --git a/include/mysql.h b/include/mysql.h index b87b865608e..e37cf710817 100644 --- a/include/mysql.h +++ b/include/mysql.h @@ -499,6 +499,8 @@ MYSQL_FIELD_OFFSET STDCALL mysql_field_seek(MYSQL_RES *result, MYSQL_FIELD_OFFSET offset); MYSQL_ROW STDCALL mysql_fetch_row(MYSQL_RES *result); unsigned long * STDCALL mysql_fetch_lengths(MYSQL_RES *result); +void STDCALL cli_fetch_lengths(ulong *to, MYSQL_ROW column, + unsigned int field_count); MYSQL_FIELD * STDCALL mysql_fetch_field(MYSQL_RES *result); MYSQL_RES * STDCALL mysql_list_fields(MYSQL *mysql, const char *table, const char *wild); diff --git a/mysql-test/r/federated.result b/mysql-test/r/federated.result index 4ed0b45a4c5..d1ad9921865 100644 --- a/mysql-test/r/federated.result +++ b/mysql-test/r/federated.result @@ -644,14 +644,19 @@ select * from federated.t1 where fileguts = 'jimbob'; id code fileguts creation_date entered_time 3 DEUEUEUEUEUEUEUEUEU jimbob 2004-04-04 04:04:04 2004-04-04 04:04:04 drop table if exists federated.t1; -CREATE TABLE federated.t1 ( `id` int(20) NOT NULL auto_increment, `country_id` int(20) NOT NULL DEFAULT 0, `name` varchar(32), `other` varchar(20), PRIMARY KEY (`id`), key (country_id)); +CREATE TABLE federated.t1 (a BLOB); drop table if exists federated.t1; -CREATE TABLE federated.t1 ( `id` int(20) NOT NULL auto_increment, `country_id` int(20) NOT NULL DEFAULT 0, `name` varchar(32), `other` varchar(20), PRIMARY KEY (`id`), key (country_id) ) ENGINE="FEDERATED" DEFAULT CHARSET=latin1 COMMENT='mysql://root@127.0.0.1:9308/federated/t1'; -insert into federated.t1 (name, country_id, other) values ('Kumar', 1, 11111); -insert into federated.t1 (name, country_id, other) values ('Lenz', 2, 22222); -insert into federated.t1 (name, country_id, other) values ('Marizio', 3, 33333); -insert into federated.t1 (name, country_id, other) values ('Monty', 4, 33333); -insert into federated.t1 (name, country_id, other) values ('Sanja', 5, 33333); +CREATE TABLE federated.t1 (a BLOB) ENGINE="FEDERATED" COMMENT='mysql://root@127.0.0.1:9308/federated/t1'; +INSERT INTO federated.t1 VALUES (0x00); +INSERT INTO federated.t1 VALUES (0x0001); +INSERT INTO federated.t1 VALUES (0x0100); +SELECT HEX(a) FROM federated.t1; +HEX(a) +00 +0001 +0100 +drop table if exists federated.t1; +CREATE TABLE federated.t1 ( `id` int(20) NOT NULL auto_increment, `country_id` int(20) NOT NULL DEFAULT 0, `name` varchar(32), `other` varchar(20), PRIMARY KEY (`id`), key (country_id)); drop table if exists federated.countries; Warnings: Note 1051 Unknown table 'countries' @@ -661,6 +666,13 @@ insert into federated.countries (country) values ('Germany'); insert into federated.countries (country) values ('Italy'); insert into federated.countries (country) values ('Finland'); insert into federated.countries (country) values ('Ukraine'); +drop table if exists federated.t1; +CREATE TABLE federated.t1 ( `id` int(20) NOT NULL auto_increment, `country_id` int(20) NOT NULL DEFAULT 0, `name` varchar(32), `other` varchar(20), PRIMARY KEY (`id`), key (country_id) ) ENGINE="FEDERATED" DEFAULT CHARSET=latin1 COMMENT='mysql://root@127.0.0.1:9308/federated/t1'; +insert into federated.t1 (name, country_id, other) values ('Kumar', 1, 11111); +insert into federated.t1 (name, country_id, other) values ('Lenz', 2, 22222); +insert into federated.t1 (name, country_id, other) values ('Marizio', 3, 33333); +insert into federated.t1 (name, country_id, other) values ('Monty', 4, 33333); +insert into federated.t1 (name, country_id, other) values ('Sanja', 5, 33333); select federated.t1.*, federated.countries.country from federated.t1 left join federated.countries on federated.t1.country_id = federated.countries.id; id country_id name other country 1 1 Kumar 11111 India diff --git a/mysql-test/t/federated.test b/mysql-test/t/federated.test index 413509a0a6a..c634badf04e 100644 --- a/mysql-test/t/federated.test +++ b/mysql-test/t/federated.test @@ -522,8 +522,22 @@ insert into federated.t1 (code, fileguts, creation_date) values ('ASDFWERQWETWET insert into federated.t1 (code, fileguts, creation_date) values ('DEUEUEUEUEUEUEUEUEU', '*()w*09*$()*#)(*09*^90*d)(*s()d8g)(s*ned)(*)(s*d)(*hn(d*)(*sbn)D((#$*(#*%%&#&^$#&#&#&#&^&#*&*#$*&^*(&#(&Q*&&(*!&!(*&*(#&*(%&#*###[[', '2004-04-04 04:04:04'); insert into federated.t1 (code, fileguts, creation_date) values ('DEUEUEUEUEUEUEUEUEU', 'jimbob', '2004-04-04 04:04:04'); select * from federated.t1; -select * from federated.t1 where fileguts = 'jimbob'; # test blob indexes +select * from federated.t1 where fileguts = 'jimbob'; + +# test blob with binary +connection slave; +drop table if exists federated.t1; +CREATE TABLE federated.t1 (a BLOB); + +connection master; +drop table if exists federated.t1; +CREATE TABLE federated.t1 (a BLOB) ENGINE="FEDERATED" COMMENT='mysql://root@127.0.0.1:9308/federated/t1'; +INSERT INTO federated.t1 VALUES (0x00); +INSERT INTO federated.t1 VALUES (0x0001); +INSERT INTO federated.t1 VALUES (0x0100); +SELECT HEX(a) FROM federated.t1; + # TODO # @@ -559,15 +573,6 @@ drop table if exists federated.t1; CREATE TABLE federated.t1 ( `id` int(20) NOT NULL auto_increment, `country_id` int(20) NOT NULL DEFAULT 0, `name` varchar(32), `other` varchar(20), PRIMARY KEY (`id`), key (country_id)); connection master; -drop table if exists federated.t1; -CREATE TABLE federated.t1 ( `id` int(20) NOT NULL auto_increment, `country_id` int(20) NOT NULL DEFAULT 0, `name` varchar(32), `other` varchar(20), PRIMARY KEY (`id`), key (country_id) ) ENGINE="FEDERATED" DEFAULT CHARSET=latin1 COMMENT='mysql://root@127.0.0.1:9308/federated/t1'; -insert into federated.t1 (name, country_id, other) values ('Kumar', 1, 11111); -insert into federated.t1 (name, country_id, other) values ('Lenz', 2, 22222); -insert into federated.t1 (name, country_id, other) values ('Marizio', 3, 33333); -insert into federated.t1 (name, country_id, other) values ('Monty', 4, 33333); -insert into federated.t1 (name, country_id, other) values ('Sanja', 5, 33333); - - drop table if exists federated.countries; CREATE TABLE federated.countries ( `id` int(20) NOT NULL auto_increment, `country` varchar(32), primary key (id)); insert into federated.countries (country) values ('India'); @@ -576,6 +581,14 @@ insert into federated.countries (country) values ('Italy'); insert into federated.countries (country) values ('Finland'); insert into federated.countries (country) values ('Ukraine'); +drop table if exists federated.t1; +CREATE TABLE federated.t1 ( `id` int(20) NOT NULL auto_increment, `country_id` int(20) NOT NULL DEFAULT 0, `name` varchar(32), `other` varchar(20), PRIMARY KEY (`id`), key (country_id) ) ENGINE="FEDERATED" DEFAULT CHARSET=latin1 COMMENT='mysql://root@127.0.0.1:9308/federated/t1'; +insert into federated.t1 (name, country_id, other) values ('Kumar', 1, 11111); +insert into federated.t1 (name, country_id, other) values ('Lenz', 2, 22222); +insert into federated.t1 (name, country_id, other) values ('Marizio', 3, 33333); +insert into federated.t1 (name, country_id, other) values ('Monty', 4, 33333); +insert into federated.t1 (name, country_id, other) values ('Sanja', 5, 33333); + select federated.t1.*, federated.countries.country from federated.t1 left join federated.countries on federated.t1.country_id = federated.countries.id; drop table federated.countries; diff --git a/sql-common/client.c b/sql-common/client.c index 9dcf6b3e32c..3d760c73eea 100644 --- a/sql-common/client.c +++ b/sql-common/client.c @@ -1121,7 +1121,7 @@ void mysql_read_default_options(struct st_mysql_options *options, else the lengths are calculated from the offset between pointers. **************************************************************************/ -static void cli_fetch_lengths(ulong *to, MYSQL_ROW column, +void cli_fetch_lengths(ulong *to, MYSQL_ROW column, unsigned int field_count) { ulong *prev_length; diff --git a/sql/ha_federated.cc b/sql/ha_federated.cc index 695c71677c0..eed145203c3 100644 --- a/sql/ha_federated.cc +++ b/sql/ha_federated.cc @@ -343,10 +343,10 @@ ./sql/ha_federated.cc ./sql/ha_federated.h -*/ +*/ #ifdef __GNUC__ -#pragma implementation // gcc: Class implementation +#pragma implementation // gcc: Class implementation #endif #include @@ -355,18 +355,21 @@ #include "ha_federated.h" #define MAX_REMOTE_SIZE IO_SIZE /* Variables for federated share methods */ -static HASH federated_open_tables; // Hash used to track open tables -pthread_mutex_t federated_mutex; // This is the mutex we use to init the hash -static int federated_init= 0; // Variable for checking the init state of hash +static HASH federated_open_tables; // Hash used to track open + // tables +pthread_mutex_t federated_mutex; // This is the mutex we use to + // init the hash +static int federated_init= 0; // Variable for checking the + // init state of hash /* Function we use in the creation of our hash to get key. */ -static byte* federated_get_key(FEDERATED_SHARE *share,uint *length, - my_bool not_used __attribute__((unused))) +static byte *federated_get_key(FEDERATED_SHARE *share, uint *length, + my_bool not_used __attribute__ ((unused))) { *length= share->table_name_length; - return (byte*) share->table_name; + return (byte *) share->table_name; } /* @@ -374,8 +377,9 @@ static byte* federated_get_key(FEDERATED_SHARE *share,uint *length, SYNOPSIS parse_url() - share pointer to FEDERATED share - table pointer to current TABLE class + share pointer to FEDERATED share + table pointer to current TABLE class + table_create_flag determines what error to throw DESCRIPTION populates the share with information about the connection @@ -385,10 +389,10 @@ static byte* federated_get_key(FEDERATED_SHARE *share,uint *length, This string MUST be in the format of any of these: -scheme://username:password@hostname:port/database/table -scheme://username@hostname/database/table -scheme://username@hostname:port/database/table -scheme://username:password@hostname/database/table + scheme://username:password@hostname:port/database/table + scheme://username@hostname/database/table + scheme://username@hostname:port/database/table + scheme://username:password@hostname/database/table An Example: @@ -401,33 +405,34 @@ scheme://username:password@hostname/database/table RETURN VALUE 0 success - -1 failure, wrong string format + 1 failure, wrong string format */ -static int parse_url(FEDERATED_SHARE *share, TABLE *table, uint table_create_flag) +static int parse_url(FEDERATED_SHARE *share, TABLE *table, + uint table_create_flag) { DBUG_ENTER("ha_federated::parse_url"); // This either get set or will remain the same. share->port= 0; - uint error_num= table_create_flag ? ER_CANT_CREATE_TABLE : ER_CONNECT_TO_MASTER ; + uint error_num= table_create_flag ? ER_CANT_CREATE_TABLE : + ER_CONNECT_TO_MASTER; share->scheme= my_strdup(table->s->comment, MYF(0)); - if ((share->username= strstr(share->scheme, "://"))) { - share->scheme[share->username - share->scheme] = '\0'; + share->scheme[share->username - share->scheme]= '\0'; if (strcmp(share->scheme, "mysql") != 0) { DBUG_PRINT("ha_federated::parse_url", ("The federated handler currently only supports connecting\ to a MySQL database!!!\n")); my_error(error_num, MYF(0), - "ERROR: federated handler only supports remote 'mysql://' database"); - DBUG_RETURN(-1); + "ERROR: federated handler only supports remote 'mysql://' database"); + DBUG_RETURN(1); } - share->username+= 3; + share->username += 3; if ((share->hostname= strchr(share->username, '@'))) { @@ -445,14 +450,14 @@ static int parse_url(FEDERATED_SHARE *share, TABLE *table, uint table_create_fla DBUG_PRINT("ha_federated::parse_url", ("this connection string is not in the correct format!!!\n")); my_error(error_num, MYF(0), - "this connection string is not in the correct format!!!\n"); - DBUG_RETURN(-1); + "this connection string is not in the correct format!!!\n"); + DBUG_RETURN(1); } /* - Found that if the string is: -user:@hostname:port/database/table -Then password is a null string, so set to NULL - */ + Found that if the string is: + user:@hostname:port/database/table + Then password is a null string, so set to NULL + */ if ((share->password[0] == '\0')) share->password= NULL; } @@ -465,8 +470,8 @@ Then password is a null string, so set to NULL DBUG_PRINT("ha_federated::parse_url", ("this connection string is not in the correct format!!!\n")); my_error(error_num, MYF(0), - "this connection string is not in the correct format!!!\n"); - DBUG_RETURN(-1); + "this connection string is not in the correct format!!!\n"); + DBUG_RETURN(1); } if ((share->database= strchr(share->hostname, '/'))) @@ -494,8 +499,8 @@ Then password is a null string, so set to NULL DBUG_PRINT("ha_federated::parse_url", ("this connection string is not in the correct format!!!\n")); my_error(error_num, MYF(0), - "this connection string is not in the correct format!!!\n"); - DBUG_RETURN(-1); + "this connection string is not in the correct format!!!\n"); + DBUG_RETURN(1); } } else @@ -503,8 +508,8 @@ Then password is a null string, so set to NULL DBUG_PRINT("ha_federated::parse_url", ("this connection string is not in the correct format!!!\n")); my_error(error_num, MYF(0), - "this connection string is not in the correct format!!!\n"); - DBUG_RETURN(-1); + "this connection string is not in the correct format!!!\n"); + DBUG_RETURN(1); } // make sure there's not an extra / if ((strchr(share->table_base_name, '/'))) @@ -512,34 +517,40 @@ Then password is a null string, so set to NULL DBUG_PRINT("ha_federated::parse_url", ("this connection string is not in the correct format!!!\n")); my_error(error_num, MYF(0), - "this connection string is not in the correct format!!!\n"); - DBUG_RETURN(-1); + "this connection string is not in the correct format!!!\n"); + DBUG_RETURN(1); } if (share->hostname[0] == '\0') share->hostname= NULL; - DBUG_PRINT("ha_federated::parse_url", - ("scheme %s username %s password %s \ - hostname %s port %d database %s tablename %s\n", - share->scheme, share->username, share->password, share->hostname, - share->port, share->database, share->table_base_name)); + if (!share->port) + { + if (strcmp(share->hostname, "localhost") == 0) + share->socket= my_strdup("/tmp/mysql.sock", MYF(0)); + else + share->port= 3306; + } + + DBUG_PRINT("ha_federated::parse_url", + ("scheme %s username %s password %s \ + hostname %s port %d database %s tablename %s\n", share->scheme, share->username, share->password, share->hostname, share->port, share->database, share->table_base_name)); } else { DBUG_PRINT("ha_federated::parse_url", - ("this connection string is not in the correct format!!!\n")); + ("this connection string is not in the correct format!!!\n")); my_error(error_num, MYF(0), - "this connection string is not in the correct format!!!\n"); - DBUG_RETURN(-1); + "this connection string is not in the correct format!!!\n"); + DBUG_RETURN(1); } } else - { + { DBUG_PRINT("ha_federated::parse_url", - ("this connection string is not in the correct format!!!\n")); + ("this connection string is not in the correct format!!!\n")); my_error(error_num, MYF(0), - "this connection string is not in the correct format!!!\n"); - DBUG_RETURN(-1); + "this connection string is not in the correct format!!!\n"); + DBUG_RETURN(1); } DBUG_RETURN(0); } @@ -564,24 +575,36 @@ Then password is a null string, so set to NULL */ uint ha_federated::convert_row_to_internal_format(byte *record, MYSQL_ROW row) { - unsigned long len; - int x= 0; + unsigned long *lengths; + unsigned int num_fields; + unsigned int x= 0; + DBUG_ENTER("ha_federated::convert_row_to_internal_format"); - // Question this - memset(record, 0, table->s->null_bytes); + num_fields= mysql_num_fields(result); + lengths= (unsigned long*) my_malloc(num_fields * sizeof(unsigned long), + MYF(0)); + cli_fetch_lengths((unsigned long*) (lengths), row, num_fields); - for (Field **field=table->field; *field ; field++, x++) + memset(record, 0, table->s->null_bytes); + + for (Field ** field= table->field; *field; field++, x++) { - if (!row[x]) + if (!row[x]) + { (*field)->set_null(); + } else /* - changed system_charset_info to default_charset_info because - testing revealed that german text was not being retrieved properly + changed system_charset_info to default_charset_info because + testing revealed that german text was not being retrieved properly */ - (*field)->store(row[x], strlen(row[x]), &my_charset_bin); + DBUG_PRINT("ha_federated::convert_row_to_internal_format", + ("row[%d] %s length %lu", x, row[x], lengths[x])); + (*field)->store(row[x], lengths[x], &my_charset_bin); } + my_free((gptr) lengths, MYF(0)); + lengths= 0; DBUG_RETURN(0); } @@ -595,29 +618,30 @@ bool ha_federated::create_where_from_key(String *to, KEY *key_info, String tmp; DBUG_ENTER("ha_federated::create_where_from_key"); - for (key_part= key_info->key_part ; (int) key_length > 0 ; key_part++) + for (key_part= key_info->key_part; (int) key_length > 0; key_part++) { Field *field= key_part->field; needs_quotes= field->needs_quotes(); //bool needs_quotes= type_quote(field->type()); - DBUG_PRINT("ha_federated::create_where_from_key", ("key name %s type %d", field->field_name, field->type())); + DBUG_PRINT("ha_federated::create_where_from_key", + ("key name %s type %d", field->field_name, field->type())); uint length= key_part->length; - if (second_loop++ && to->append(" AND ",5)) + if (second_loop++ && to->append(" AND ", 5)) DBUG_RETURN(1); - if (to->append('`') || to->append(field->field_name) || - to->append("` ",2)) - DBUG_RETURN(1); // Out of memory + if (to->append('`') || to->append(field->field_name) || to->append("` ", 2)) + DBUG_RETURN(1); // Out of memory if (key_part->null_bit) { if (*key++) { - if (to->append("IS NULL",7)) - DBUG_PRINT("ha_federated::create_where_from_key", ("NULL type %s", to->c_ptr_quick())); - DBUG_RETURN(1); - key_length-= key_part->store_length; - key+= key_part->store_length-1; + if (to->append("IS NULL", 7)) + DBUG_PRINT("ha_federated::create_where_from_key", + ("NULL type %s", to->c_ptr_quick())); + DBUG_RETURN(1); + key_length -= key_part->store_length; + key += key_part->store_length - 1; continue; } key_length--; @@ -630,101 +654,81 @@ bool ha_federated::create_where_from_key(String *to, KEY *key_info, { /* This is can be threated as a hex string */ Field_bit *field= (Field_bit *) (key_part->field); - char buff[64+2], *ptr; - byte *end= (byte *)(key) + length; + char buff[64 + 2], *ptr; + byte *end= (byte *) (key) + length; - buff[0]='0'; - buff[1]='x'; - for (ptr= buff+2 ; key < end ; key++) + buff[0]= '0'; + buff[1]= 'x'; + for (ptr= buff + 2; key < end; key++) { - uint tmp= (uint) (uchar) *key; - *ptr++=_dig_vec_upper[tmp >> 4]; - *ptr++=_dig_vec_upper[tmp & 15]; + uint tmp= (uint) (uchar) * key; + *ptr++= _dig_vec_upper[tmp >> 4]; + *ptr++= _dig_vec_upper[tmp & 15]; } - if (to->append(buff, (uint) (ptr-buff))) + if (to->append(buff, (uint) (ptr - buff))) DBUG_RETURN(1); - DBUG_PRINT("ha_federated::create_where_from_key", ("bit type %s", to->c_ptr_quick())); - key_length-= length; + DBUG_PRINT("ha_federated::create_where_from_key", + ("bit type %s", to->c_ptr_quick())); + key_length -= length; continue; } if (key_part->key_part_flag & HA_BLOB_PART) { uint blob_length= uint2korr(key); - key+= HA_KEY_BLOB_LENGTH; - key_length-= HA_KEY_BLOB_LENGTH; + key += HA_KEY_BLOB_LENGTH; + key_length -= HA_KEY_BLOB_LENGTH; tmp.set_quick((char*) key, blob_length, &my_charset_bin); if (append_escaped(to, &tmp)) DBUG_RETURN(1); - DBUG_PRINT("ha_federated::create_where_from_key", ("blob type %s", to->c_ptr_quick())); + DBUG_PRINT("ha_federated::create_where_from_key", + ("blob type %s", to->c_ptr_quick())); length= key_part->length; } else if (key_part->key_part_flag & HA_VAR_LENGTH_PART) { length= uint2korr(key); - key+= HA_KEY_BLOB_LENGTH; + key += HA_KEY_BLOB_LENGTH; tmp.set_quick((char*) key, length, &my_charset_bin); if (append_escaped(to, &tmp)) DBUG_RETURN(1); - DBUG_PRINT("ha_federated::create_where_from_key", ("varchar type %s", to->c_ptr_quick())); + DBUG_PRINT("ha_federated::create_where_from_key", + ("varchar type %s", to->c_ptr_quick())); } else { - DBUG_PRINT("ha_federated::create_where_from_key", ("else block, unknown type so far")); + DBUG_PRINT("ha_federated::create_where_from_key", + ("else block, unknown type so far")); char buff[MAX_FIELD_WIDTH]; String str(buff, sizeof(buff), field->charset()), *res; - res= field->val_str(&str, (char *)(key)); + res= field->val_str(&str, (char*) (key)); if (field->result_type() == STRING_RESULT) { if (append_escaped(to, res)) DBUG_RETURN(1); - res= field->val_str(&str, (char *)(key)); + res= field->val_str(&str, (char*) (key)); - DBUG_PRINT("ha_federated::create_where_from_key", ("else block, string type", to->c_ptr_quick())); + DBUG_PRINT("ha_federated::create_where_from_key", + ("else block, string type", to->c_ptr_quick())); } else if (to->append(res->ptr(), res->length())) DBUG_RETURN(1); } if (needs_quotes && to->append("'")) DBUG_RETURN(1); - DBUG_PRINT("ha_federated::create_where_from_key", ("final value for 'to' %s", to->c_ptr_quick())); - key+= length; - key_length-= length; + DBUG_PRINT("ha_federated::create_where_from_key", + ("final value for 'to' %s", to->c_ptr_quick())); + key += length; + key_length -= length; DBUG_RETURN(0); } DBUG_RETURN(1); } -int load_conn_info(FEDERATED_SHARE *share, TABLE *table) -{ - DBUG_ENTER("ha_federated::load_conn_info"); - int retcode; - - retcode= parse_url(share, table, 0); - - if (retcode < 0) - { - DBUG_PRINT("ha_federated::load_conn_info", - ("retcode %d, setting defaults", retcode)); - /* sanity checks to make sure all needed pieces are present */ - if (!share->port) - { - if (strcmp(share->hostname, "localhost") == 0) - share->socket= my_strdup("/tmp/mysql.sock",MYF(0)); - else - share->port= 3306; - } - } - DBUG_PRINT("ha_federated::load_conn_info", - ("returned from retcode %d", retcode)); - - DBUG_RETURN(retcode); -} - /* Example of simple lock controls. The "share" it creates is structure we will pass to each federated handler. Do you have to have one of these? Well, you @@ -742,13 +746,13 @@ static FEDERATED_SHARE *get_share(const char *table_name, TABLE *table) // share->table_name has the file location - we want the actual table's // name! - table_base_name= (char *)table->s->table_name; - DBUG_PRINT("ha_federated::get_share",("table_name %s", table_base_name)); + table_base_name= (char*) table->s->table_name; + DBUG_PRINT("ha_federated::get_share", ("table_name %s", table_base_name)); /* - So why does this exist? There is no way currently to init a storage engine. - Innodb and BDB both have modifications to the server to allow them to - do this. Since you will not want to do this, this is probably the next - best method. + So why does this exist? There is no way currently to init a storage engine. + Innodb and BDB both have modifications to the server to allow them to + do this. Since you will not want to do this, this is probably the next + best method. */ if (!federated_init) { @@ -757,9 +761,9 @@ static FEDERATED_SHARE *get_share(const char *table_name, TABLE *table) if (!federated_init) { federated_init++; - VOID(pthread_mutex_init(&federated_mutex,MY_MUTEX_INIT_FAST)); - (void) hash_init(&federated_open_tables,system_charset_info,32,0,0, - (hash_get_key) federated_get_key,0,0); + VOID(pthread_mutex_init(&federated_mutex, MY_MUTEX_INIT_FAST)); + (void) hash_init(&federated_open_tables, system_charset_info, 32, 0, 0, + (hash_get_key) federated_get_key, 0, 0); } pthread_mutex_unlock(&LOCK_mysql_create_db); } @@ -767,41 +771,43 @@ static FEDERATED_SHARE *get_share(const char *table_name, TABLE *table) table_name_length= (uint) strlen(table_name); table_base_name_length= (uint) strlen(table_base_name); - if (!(share= (FEDERATED_SHARE*) hash_search(&federated_open_tables, - (byte*) table_name, - table_name_length))) + if (!(share= (FEDERATED_SHARE *) hash_search(&federated_open_tables, + (byte *) table_name, + table_name_length))) { 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))) + &tmp_table_name, table_name_length + 1, + &tmp_table_base_name, table_base_name_length + 1, + &select_query, query.length() + 1, NullS))) { pthread_mutex_unlock(&federated_mutex); return NULL; } - load_conn_info(share, table); + if (parse_url(share, table, 0)) + goto error; + 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.c_ptr_quick()); - DBUG_PRINT("ha_federated::get_share",("share->select_query %s", share->select_query)); - if (my_hash_insert(&federated_open_tables, (byte*) share)) + 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)); + if (my_hash_insert(&federated_open_tables, (byte *) share)) goto error; thr_lock_init(&share->lock); - pthread_mutex_init(&share->mutex,MY_MUTEX_INIT_FAST); + pthread_mutex_init(&share->mutex, MY_MUTEX_INIT_FAST); } share->use_count++; pthread_mutex_unlock(&federated_mutex); @@ -813,6 +819,9 @@ error2: pthread_mutex_destroy(&share->mutex); error: pthread_mutex_unlock(&federated_mutex); + hash_delete(&federated_open_tables, (byte *) share); + if (share->scheme) + my_free((gptr) share->scheme, MYF(0)); my_free((gptr) share, MYF(0)); return NULL; @@ -827,9 +836,14 @@ error: static int free_share(FEDERATED_SHARE *share) { pthread_mutex_lock(&federated_mutex); + + if (share->scheme) + my_free((gptr) share->scheme, MYF(0)); + if (!--share->use_count) { - hash_delete(&federated_open_tables, (byte*) share); + hash_delete(&federated_open_tables, (byte *) share); + hash_free(&federated_open_tables); thr_lock_delete(&share->lock); pthread_mutex_destroy(&share->mutex); my_free((gptr) share, MYF(0)); @@ -847,7 +861,13 @@ static int free_share(FEDERATED_SHARE *share) in handler.cc. */ const char **ha_federated::bas_ext() const -{ static const char *ext[]= { NullS }; return ext; } +{ + static const char *ext[]= + { + NullS + }; + return ext; +} /* @@ -867,23 +887,20 @@ int ha_federated::open(const char *name, int mode, uint test_if_locked) if (!(share= get_share(name, table))) DBUG_RETURN(1); - thr_lock_data_init(&share->lock,&lock,NULL); + thr_lock_data_init(&share->lock, &lock, NULL); /* Connect to remote database mysql_real_connect() */ - mysql= mysql_init(0); - DBUG_PRINT("ha_federated::open",("hostname %s", share->hostname)); - DBUG_PRINT("ha_federated::open",("username %s", share->username)); - DBUG_PRINT("ha_federated::open",("password %s", share->password)); - DBUG_PRINT("ha_federated::open",("database %s", share->database)); - DBUG_PRINT("ha_federated::open",("port %d", share->port)); + mysql= mysql_init(0); + DBUG_PRINT("ha_federated::open", ("hostname %s", share->hostname)); + DBUG_PRINT("ha_federated::open", ("username %s", share->username)); + DBUG_PRINT("ha_federated::open", ("password %s", share->password)); + DBUG_PRINT("ha_federated::open", ("database %s", share->database)); + DBUG_PRINT("ha_federated::open", ("port %d", share->port)); if (!mysql_real_connect(mysql, - share->hostname, - share->username, - share->password, - share->database, - share->port, - NULL, - 0)) + share->hostname, + share->username, + share->password, + share->database, share->port, NULL, 0)) { my_error(ER_CONNECT_TO_MASTER, MYF(0), mysql_error(mysql)); DBUG_RETURN(ER_CONNECT_TO_MASTER); @@ -905,6 +922,15 @@ int ha_federated::open(const char *name, int mode, uint test_if_locked) int ha_federated::close(void) { DBUG_ENTER("ha_federated::close"); + + // free the result set + if (result) + { + DBUG_PRINT("ha_federated::close", + ("mysql_free_result result at address %lx", result)); + mysql_free_result(result); + result= 0; + } /* Disconnect from mysql */ mysql_close(mysql); DBUG_RETURN(free_share(share)); @@ -929,10 +955,12 @@ int ha_federated::close(void) 1 if NULL 0 otherwise */ -inline uint field_in_record_is_null ( - TABLE* table, /* in: MySQL table object */ - Field* field, /* in: MySQL field object */ - char* record) /* in: a row in MySQL format */ +inline uint field_in_record_is_null(TABLE * table, /* in: MySQL table + object */ + Field * field, /* in: MySQL field + object */ + char *record) /* in: a row in MySQL + format */ { int null_offset; DBUG_ENTER("ha_federated::field_in_record_is_null"); @@ -963,11 +991,11 @@ inline uint field_in_record_is_null ( */ int ha_federated::write_row(byte * buf) { - int x= 0, num_fields= 0; + uint x= 0, num_fields= 0; Field **field; ulong current_query_id= 1; ulong tmp_query_id= 1; - int all_fields_have_same_query_id= 1; + uint all_fields_have_same_query_id= 1; char insert_buffer[IO_SIZE]; char values_buffer[IO_SIZE], insert_field_value_buffer[IO_SIZE]; @@ -980,40 +1008,41 @@ int ha_federated::write_row(byte * buf) values_string.length(0); // The actual value of the field, to be added to the values_string String insert_field_value_string(insert_field_value_buffer, - sizeof(insert_field_value_buffer), &my_charset_bin); + sizeof(insert_field_value_buffer), + &my_charset_bin); insert_field_value_string.length(0); DBUG_ENTER("ha_federated::write_row"); /* - I want to use this and the next line, but the repository needs to be - updated to do so + I want to use this and the next line, but the repository needs to be + updated to do so */ - statistic_increment(table->in_use->status_var.ha_write_count,&LOCK_status); + 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 remote will not be appended unless they match - this query id + get the current query id - the fields that we add to the insert + statement to send to the remote will not be appended unless they match + this query id */ current_query_id= table->in_use->query_id; DBUG_PRINT("ha_federated::write_row", ("current query id %d", current_query_id)); // start off our string - insert_string.append("INSERT INTO "); + insert_string.append("INSERT INTO "); insert_string.append(share->table_base_name); // start both our field and field values strings insert_string.append(" ("); values_string.append(" VALUES ("); /* - 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) + 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++, x++) + for (field= table->field; *field; field++, x++) { if (x > 0 && tmp_query_id != (*field)->query_id) all_fields_have_same_query_id= 0; @@ -1021,18 +1050,18 @@ int ha_federated::write_row(byte * buf) 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 + loop through the field pointer array, add any fields to both the values + list and the fields list that match the current query id */ - for (field= table->field; *field ; field++, x++) + for (field= table->field; *field; field++, x++) { DBUG_PRINT("ha_federated::write_row", ("field type %d", (*field)->type())); // 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)->query_id && (*field)->query_id == current_query_id) + || all_fields_have_same_query_id) { num_fields++; - + if ((*field)->is_null()) { DBUG_PRINT("ha_federated::write_row", @@ -1060,30 +1089,28 @@ int ha_federated::write_row(byte * buf) // append commas between both fields and fieldnames insert_string.append(','); values_string.append(','); - DBUG_PRINT("ha_federated::write_row", - ("insert_string %s values_string %s insert_field_value_string %s", - insert_string.c_ptr_quick(), values_string.c_ptr_quick(), - insert_field_value_string.c_ptr_quick())); + DBUG_PRINT("ha_federated::write_row", + ("insert_string %s values_string %s insert_field_value_string %s", + insert_string.c_ptr_quick(), values_string.c_ptr_quick(), + insert_field_value_string.c_ptr_quick())); } } /* - chop of the trailing comma, or if there were no fields, a '(' - So, "INSERT INTO foo (" becomes "INSERT INTO foo " - or, with fields, "INSERT INTO foo (field1, field2," becomes + chop of the trailing comma, or if there were no fields, a '(' + So, "INSERT INTO foo (" becomes "INSERT INTO foo " + or, with fields, "INSERT INTO foo (field1, field2," becomes "INSERT INTO foo (field1, field2" */ insert_string.chop(); - /* - if there were no fields, we don't want to add a closing paren - AND, we don't want to chop off the last char '(' - insert will be "INSERT INTO t1 VALUES ();" + if there were no fields, we don't want to add a closing paren + AND, we don't want to chop off the last char '(' + insert will be "INSERT INTO t1 VALUES ();" */ - DBUG_PRINT("ha_federated::write_row",("x %d num fields %d", - x, num_fields)); + DBUG_PRINT("ha_federated::write_row", ("x %d num fields %d", x, num_fields)); if (num_fields > 0) { // chops off leading commas @@ -1092,18 +1119,17 @@ int ha_federated::write_row(byte * buf) } // we always want to append this, even if there aren't any fields values_string.append(')'); - + // add the values insert_string.append(values_string); - DBUG_PRINT("ha_federated::write_row",("insert query %s", - insert_string.c_ptr_quick())); + DBUG_PRINT("ha_federated::write_row", ("insert query %s", + insert_string.c_ptr_quick())); - if (mysql_real_query(mysql, insert_string.c_ptr_quick(), - insert_string.length())) + if (mysql_real_query(mysql, insert_string.ptr(), insert_string.length())) { - my_error(ER_QUERY_ON_MASTER,MYF(0),mysql_error(mysql)); - DBUG_RETURN(ER_QUERY_ON_MASTER); + my_error(ER_QUERY_ON_MASTER, MYF(0), mysql_error(mysql)); + DBUG_RETURN(ER_QUERY_ON_MASTER); } DBUG_RETURN(0); @@ -1125,22 +1151,21 @@ int ha_federated::write_row(byte * buf) Called from sql_select.cc, sql_acl.cc, sql_update.cc, and sql_insert.cc. */ -int ha_federated::update_row( - const byte * old_data, - byte * new_data - ) +int ha_federated::update_row(const byte * old_data, byte * new_data) { - int x= 0; + uint x= 0; uint has_a_primary_key= 0; - int primary_key_field_num; + uint primary_key_field_num; char old_field_value_buffer[IO_SIZE], new_field_value_buffer[IO_SIZE]; char update_buffer[IO_SIZE], where_buffer[IO_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); + String old_field_value(old_field_value_buffer, sizeof(old_field_value_buffer), + &my_charset_bin); old_field_value.length(0); // stores the new value of the field - String new_field_value(new_field_value_buffer, sizeof(new_field_value_buffer), &my_charset_bin); + String new_field_value(new_field_value_buffer, sizeof(new_field_value_buffer), + &my_charset_bin); new_field_value.length(0); // stores the update query String update_string(update_buffer, sizeof(update_buffer), &my_charset_bin); @@ -1153,10 +1178,10 @@ int ha_federated::update_row( has_a_primary_key= table->s->primary_key == 0 ? 1 : 0; - primary_key_field_num= has_a_primary_key ? - table->key_info[table->s->primary_key].key_part->fieldnr -1 : -1; + primary_key_field_num= has_a_primary_key ? + table->key_info[table->s->primary_key].key_part->fieldnr - 1 : -1; if (has_a_primary_key) - DBUG_PRINT("ha_federated::update_row", ("has a primary key")); + DBUG_PRINT("ha_federated::update_row", ("has a primary key")); update_string.append("UPDATE "); update_string.append(share->table_base_name); @@ -1171,19 +1196,19 @@ int ha_federated::update_row( used to create SET field=value and old data is used to create WHERE field=oldvalue */ - - for (Field **field= table->field ; *field ; field++, x++) + + for (Field ** field= table->field; *field; field++, x++) { /* - In all of these tests for 'has_a_primary_key', what I'm trying to - accomplish is to only use the primary key in the WHERE clause if the - table has a primary key, as opposed to a table without a primary key - in which case we have to use all the fields to create a WHERE clause - using the old/current values, as well as adding a LIMIT statement + In all of these tests for 'has_a_primary_key', what I'm trying to + accomplish is to only use the primary key in the WHERE clause if the + table has a primary key, as opposed to a table without a primary key + in which case we have to use all the fields to create a WHERE clause + using the old/current values, as well as adding a LIMIT statement */ - if (has_a_primary_key) + if (has_a_primary_key) { - if (x == primary_key_field_num) + if (x == primary_key_field_num) where_string.append((*field)->field_name); } else @@ -1200,61 +1225,59 @@ int ha_federated::update_row( (*field)->val_str(&new_field_value); (*field)->quote_data(&new_field_value); - if ( has_a_primary_key ) + if (has_a_primary_key) { if (x == primary_key_field_num) where_string.append("="); } - else - if (! field_in_record_is_null(table, *field, (char*) old_data)) - where_string.append("="); + else if (!field_in_record_is_null(table, *field, (char*) old_data)) + where_string.append("="); } - if ( has_a_primary_key) + if (has_a_primary_key) { if (x == primary_key_field_num) { (*field)->val_str(&old_field_value, - (char *)(old_data + (*field)->offset())); + (char*) (old_data + (*field)->offset())); (*field)->quote_data(&old_field_value); where_string.append(old_field_value); } } else { - if (field_in_record_is_null(table, *field, (char*) old_data)) - where_string.append(" IS NULL "); - else - { - (*field)->val_str(&old_field_value, - (char *)(old_data + (*field)->offset())); - (*field)->quote_data(&old_field_value); - where_string.append(old_field_value); - } + if (field_in_record_is_null(table, *field, (char*) old_data)) + where_string.append(" IS NULL "); + else + { + (*field)->val_str(&old_field_value, + (char*) (old_data + (*field)->offset())); + (*field)->quote_data(&old_field_value); + where_string.append(old_field_value); + } } update_string.append(new_field_value); new_field_value.length(0); - if ((uint) x+1 < table->s->fields) + if (x + 1 < table->s->fields) { update_string.append(", "); - if (! has_a_primary_key) + if (!has_a_primary_key) where_string.append(" AND "); } old_field_value.length(0); } update_string.append(" WHERE "); - update_string.append(where_string.c_ptr_quick()); - if (! has_a_primary_key) + update_string.append(where_string.ptr()); + if (!has_a_primary_key) update_string.append(" LIMIT 1"); DBUG_PRINT("ha_federated::update_row", ("Final update query: %s", update_string.c_ptr_quick())); - if (mysql_real_query(mysql, update_string.c_ptr_quick(), - update_string.length())) + if (mysql_real_query(mysql, update_string.ptr(), update_string.length())) { - my_error(ER_QUERY_ON_MASTER,MYF(0),mysql_error(mysql)); - DBUG_RETURN(ER_QUERY_ON_MASTER); + my_error(ER_QUERY_ON_MASTER, MYF(0), mysql_error(mysql)); + DBUG_RETURN(ER_QUERY_ON_MASTER); } @@ -1277,7 +1300,7 @@ int ha_federated::update_row( */ int ha_federated::delete_row(const byte * buf) { - int x= 0; + uint x= 0; char delete_buffer[IO_SIZE]; char data_buffer[IO_SIZE]; @@ -1292,7 +1315,7 @@ int ha_federated::delete_row(const byte * buf) delete_string.append(share->table_base_name); delete_string.append(" WHERE "); - for (Field **field= table->field; *field; field++, x++) + for (Field ** field= table->field; *field; field++, x++) { delete_string.append((*field)->field_name); @@ -1307,22 +1330,21 @@ int ha_federated::delete_row(const byte * buf) (*field)->val_str(&data_string); (*field)->quote_data(&data_string); } - + delete_string.append(data_string); data_string.length(0); - if ((uint) x+1 < table->s->fields) + if (x + 1 < table->s->fields) delete_string.append(" AND "); } delete_string.append(" LIMIT 1"); DBUG_PRINT("ha_federated::delete_row", ("Delete sql: %s", delete_string.c_ptr_quick())); - if ( mysql_real_query(mysql, delete_string.c_ptr_quick(), - delete_string.length())) + if (mysql_real_query(mysql, delete_string.ptr(), delete_string.length())) { - my_error(ER_QUERY_ON_MASTER,MYF(0),mysql_error(mysql)); - DBUG_RETURN(ER_QUERY_ON_MASTER); + my_error(ER_QUERY_ON_MASTER, MYF(0), mysql_error(mysql)); + DBUG_RETURN(ER_QUERY_ON_MASTER); } DBUG_RETURN(0); @@ -1336,9 +1358,9 @@ int ha_federated::delete_row(const byte * buf) a WHERE clause on a non-primary key index, simply calls index_read_idx. */ int ha_federated::index_read(byte * buf, const byte * key, - uint key_len __attribute__((unused)), - enum ha_rkey_function find_flag - __attribute__((unused))) + uint key_len __attribute__ ((unused)), + enum ha_rkey_function find_flag + __attribute__ ((unused))) { DBUG_ENTER("ha_federated::index_read"); DBUG_RETURN(index_read_idx(buf, active_index, key, key_len, find_flag)); @@ -1354,9 +1376,9 @@ int ha_federated::index_read(byte * buf, const byte * key, uses a PRIMARY KEY index. */ int ha_federated::index_read_idx(byte * buf, uint index, const byte * key, - uint key_len __attribute__((unused)), - enum ha_rkey_function find_flag - __attribute__((unused))) + uint key_len __attribute__ ((unused)), + enum ha_rkey_function find_flag + __attribute__ ((unused))) { char index_value[IO_SIZE]; char key_value[IO_SIZE]; @@ -1370,27 +1392,34 @@ int ha_federated::index_read_idx(byte * buf, uint index, const byte * key, sql_query.length(0); DBUG_ENTER("ha_federated::index_read_idx"); - statistic_increment(table->in_use->status_var.ha_read_key_count,&LOCK_status); + statistic_increment(table->in_use->status_var.ha_read_key_count, + &LOCK_status); sql_query.append(share->select_query); sql_query.append(" WHERE "); - keylen= strlen((char *)(key)); + keylen= strlen((char*) (key)); create_where_from_key(&index_string, &table->key_info[index], key, keylen); sql_query.append(index_string); DBUG_PRINT("ha_federated::index_read_idx", - ("current key %d key value %s index_string value %s length %d", index, (char *)(key),index_string.c_ptr_quick(), - index_string.length())); + ("current key %d key value %s index_string value %s length %d", + index, (char*) (key), index_string.c_ptr_quick(), + index_string.length())); DBUG_PRINT("ha_federated::index_read_idx", - ("current position %d sql_query %s", current_position, - sql_query.c_ptr_quick())); + ("current position %d sql_query %s", current_position, + sql_query.c_ptr_quick())); - if (mysql_real_query(mysql, sql_query.c_ptr_quick(), sql_query.length())) + if (result) { - my_error(ER_QUERY_ON_MASTER,MYF(0),mysql_error(mysql)); - DBUG_RETURN(ER_QUERY_ON_MASTER); + mysql_free_result(result); + result= 0; + } + if (mysql_real_query(mysql, sql_query.ptr(), sql_query.length())) + { + my_error(ER_QUERY_ON_MASTER, MYF(0), mysql_error(mysql)); + DBUG_RETURN(ER_QUERY_ON_MASTER); } result= mysql_store_result(mysql); @@ -1403,7 +1432,7 @@ int ha_federated::index_read_idx(byte * buf, uint index, const byte * key, if (mysql_errno(mysql)) { table->status= STATUS_NOT_FOUND; - DBUG_RETURN(mysql_errno(mysql)); + DBUG_RETURN(mysql_errno(mysql)); } DBUG_RETURN(rnd_next(buf)); @@ -1449,23 +1478,54 @@ int ha_federated::rnd_init(bool scan) DBUG_ENTER("ha_federated::rnd_init"); int num_fields, rows; - DBUG_PRINT("ha_federated::rnd_init", - ("share->select_query %s", share->select_query)); - if (mysql_real_query(mysql, share->select_query, strlen(share->select_query))) + /* + This 'scan' flag is incredibly important for this handler to work properly, + especially with updates that are called with indexes, because what happens + without this is index_read_idx gets called, does a query using the + index in a where clause, calls mysql_store_result, which then rnd_init + (from sql_update.cc) is called after this, which would do a + "select * from table" then a mysql_store_result, wiping out the result + set from index_read_idx's query, which causes the subsequent update_row + to update the wrong row! + */ + scan_flag= scan; + if (scan) { - my_error(ER_QUERY_ON_MASTER,MYF(0),mysql_error(mysql)); - DBUG_RETURN(ER_QUERY_ON_MASTER); - } - result= mysql_store_result(mysql); + DBUG_PRINT("ha_federated::rnd_init", + ("share->select_query %s", share->select_query)); + if (result) + { + DBUG_PRINT("ha_federated::rnd_init", + ("mysql_free_result address %lx", result)); + mysql_free_result(result); + result= 0; + } - if (mysql_errno(mysql)) - DBUG_RETURN(mysql_errno(mysql)); + if (mysql_real_query + (mysql, share->select_query, strlen(share->select_query))) + { + my_error(ER_QUERY_ON_MASTER, MYF(0), mysql_error(mysql)); + DBUG_RETURN(ER_QUERY_ON_MASTER); + } + result= mysql_store_result(mysql); + + if (mysql_errno(mysql)) + DBUG_RETURN(mysql_errno(mysql)); + } DBUG_RETURN(0); } int ha_federated::rnd_end() { DBUG_ENTER("ha_federated::rnd_end"); + if (result) + { + DBUG_PRINT("ha_federated::index_end", + ("mysql_free_result address %lx", result)); + mysql_free_result(result); + result= 0; + } + mysql_free_result(result); DBUG_RETURN(index_end()); } @@ -1493,10 +1553,12 @@ int ha_federated::rnd_next(byte *buf) // Fetch a row, insert it back in a row format. current_position= result->data_cursor; - if (! (row= mysql_fetch_row(result))) + DBUG_PRINT("ha_federated::rnd_next", + ("current position %d", current_position)); + if (!(row= mysql_fetch_row(result))) DBUG_RETURN(HA_ERR_END_OF_FILE); - - DBUG_RETURN(convert_row_to_internal_format(buf,row)); + + DBUG_RETURN(convert_row_to_internal_format(buf, row)); } @@ -1517,7 +1579,7 @@ void ha_federated::position(const byte *record) { DBUG_ENTER("ha_federated::position"); //ha_store_ptr Add seek storage - *(MYSQL_ROW_OFFSET *)ref=current_position; // ref is always aligned + *(MYSQL_ROW_OFFSET *) ref= current_position; // ref is always aligned DBUG_VOID_RETURN; } @@ -1535,11 +1597,24 @@ void ha_federated::position(const byte *record) int ha_federated::rnd_pos(byte * buf, byte *pos) { DBUG_ENTER("ha_federated::rnd_pos"); - statistic_increment(table->in_use->status_var.ha_read_rnd_count,&LOCK_status); - memcpy_fixed(¤t_position, pos, sizeof(MYSQL_ROW_OFFSET)); // pos is not aligned - result->current_row= 0; - result->data_cursor= current_position; - DBUG_RETURN(rnd_next(buf)); + /* + we do not need to do any of this if there has been a scan performed already, or + if this is an update and index_read_idx already has a result set in which to build + it's update query from + */ + if (scan_flag) + { + statistic_increment(table->in_use->status_var.ha_read_rnd_count, + &LOCK_status); + memcpy_fixed(¤t_position, pos, sizeof(MYSQL_ROW_OFFSET)); // pos + // is + // not + // aligned + result->current_row= 0; + result->data_cursor= current_position; + DBUG_RETURN(rnd_next(buf)); + } + DBUG_RETURN(0); } @@ -1590,7 +1665,7 @@ int ha_federated::rnd_pos(byte * buf, byte *pos) void ha_federated::info(uint flag) { DBUG_ENTER("ha_federated::info"); - records= 10000; // Fake! + records= 10000; // Fake! DBUG_VOID_RETURN; } @@ -1618,9 +1693,10 @@ int ha_federated::delete_all_rows() query.append("TRUNCATE "); query.append(share->table_base_name); - if (mysql_real_query(mysql, query.c_ptr_quick(), query.length())) { - my_error(ER_QUERY_ON_MASTER,MYF(0),mysql_error(mysql)); - DBUG_RETURN(ER_QUERY_ON_MASTER); + if (mysql_real_query(mysql, query.ptr(), query.length())) + { + my_error(ER_QUERY_ON_MASTER, MYF(0), mysql_error(mysql)); + DBUG_RETURN(ER_QUERY_ON_MASTER); } DBUG_RETURN(HA_ERR_WRONG_COMMAND); @@ -1657,32 +1733,31 @@ int ha_federated::delete_all_rows() Called from lock.cc by get_lock_data(). */ THR_LOCK_DATA **ha_federated::store_lock(THD *thd, - THR_LOCK_DATA **to, - enum thr_lock_type lock_type) + THR_LOCK_DATA **to, + enum thr_lock_type lock_type) { - if (lock_type != TL_IGNORE && lock.type == TL_UNLOCK) + if (lock_type != TL_IGNORE && lock.type == TL_UNLOCK) { /* - Here is where we get into the guts of a row level lock. - If TL_UNLOCK is set - If we are not doing a LOCK TABLE or DISCARD/IMPORT - TABLESPACE, then allow multiple writers + Here is where we get into the guts of a row level lock. + If TL_UNLOCK is set + If we are not doing a LOCK TABLE or DISCARD/IMPORT + TABLESPACE, then allow multiple writers */ if ((lock_type >= TL_WRITE_CONCURRENT_INSERT && - lock_type <= TL_WRITE) && !thd->in_lock_tables - && !thd->tablespace_op) + lock_type <= TL_WRITE) && !thd->in_lock_tables && !thd->tablespace_op) lock_type= TL_WRITE_ALLOW_WRITE; /* - In queries of type INSERT INTO t1 SELECT ... FROM t2 ... - MySQL would use the lock TL_READ_NO_INSERT on t2, and that - would conflict with TL_WRITE_ALLOW_WRITE, blocking all inserts - to t2. Convert the lock to a normal read lock to allow - concurrent inserts to t2. + In queries of type INSERT INTO t1 SELECT ... FROM t2 ... + MySQL would use the lock TL_READ_NO_INSERT on t2, and that + would conflict with TL_WRITE_ALLOW_WRITE, blocking all inserts + to t2. Convert the lock to a normal read lock to allow + concurrent inserts to t2. */ - if (lock_type == TL_READ_NO_INSERT && !thd->in_lock_tables) + if (lock_type == TL_READ_NO_INSERT && !thd->in_lock_tables) lock_type= TL_READ; lock.type= lock_type; @@ -1699,19 +1774,16 @@ THR_LOCK_DATA **ha_federated::store_lock(THD *thd, create tables if they do not exist. */ int ha_federated::create(const char *name, TABLE *table_arg, - HA_CREATE_INFO *create_info) + HA_CREATE_INFO *create_info) { - int retcode; FEDERATED_SHARE tmp; DBUG_ENTER("ha_federated::create"); - retcode= parse_url(&tmp, table_arg, 1); - if (retcode < 0) + if (parse_url(&tmp, table_arg, 1)) { - DBUG_PRINT("ha_federated::create", - ("ERROR: on table creation for %s called parse_url, retcode %d", - create_info->data_file_name, retcode)); + my_error(ER_CANT_CREATE_TABLE, MYF(0)); DBUG_RETURN(ER_CANT_CREATE_TABLE); } + my_free((gptr) tmp.scheme, MYF(0)); DBUG_RETURN(0); } -#endif /* HAVE_FEDERATED_DB */ +#endif /* HAVE_FEDERATED_DB */ diff --git a/sql/ha_federated.h b/sql/ha_federated.h index 56f5e6de4b7..6870a0902e8 100755 --- a/sql/ha_federated.h +++ b/sql/ha_federated.h @@ -62,6 +62,7 @@ class ha_federated: public handler FEDERATED_SHARE *share; /* Shared lock info */ MYSQL *mysql; MYSQL_RES *result; + bool scan_flag; uint ref_length; uint fetch_num; // stores the fetch num MYSQL_ROW_OFFSET current_position; // Current position used by ::position() @@ -76,7 +77,7 @@ private: public: ha_federated(TABLE *table): handler(table), - mysql(0), + mysql(0), result(0), scan_flag(0), ref_length(sizeof(MYSQL_ROW_OFFSET)), current_position(0) { } From 005deef102b04234f8613d4bf62a987f852f87b4 Mon Sep 17 00:00:00 2001 From: "patg@krsna.patg.net" <> Date: Wed, 16 Feb 2005 23:07:10 -0800 Subject: [PATCH 2/4] WL# 2094 Federated Storage Handler This changeset/patch is on top of changesets 1.1814 and 1.1846 (for bugs 8033 and 8065) and now fixes bug 8535. These changes have been built and tested successfully on build.mysql.com handler.cc: Added hooks for federated_db_init() and federated_db_end(), as done with ha_archive_db does, per suggestion by Ingo in code review of patch 1.1846. ha_federated.h: declaration of federated_db_init() and federated_db_end() ha_federated.cc: - Fixed some indentation problems from indent-ex (mainly to do with cases where "variablename += value" - Added federated_db_init() and federated_db_end(), as done with archive, which also handler more elegantly one of the memory leaks from bug 8033 where the federated_mutex was not freed - Removed extrenous debug messages in parse_url() - Fixed bug 8535, caused by NULL being quoted in write_row. This used to work (incorrectly) but a recent change was made in the server that exposed this --- sql/ha_federated.cc | 209 +++++++++++++++++++++++--------------------- sql/ha_federated.h | 3 + sql/handler.cc | 14 +++ 3 files changed, 125 insertions(+), 101 deletions(-) diff --git a/sql/ha_federated.cc b/sql/ha_federated.cc index eed145203c3..500dcda3e21 100644 --- a/sql/ha_federated.cc +++ b/sql/ha_federated.cc @@ -369,7 +369,50 @@ static byte *federated_get_key(FEDERATED_SHARE *share, uint *length, my_bool not_used __attribute__ ((unused))) { *length= share->table_name_length; - return (byte *) share->table_name; + return (byte*)share->table_name; +} + +/* + Initialize the federated handler. + + SYNOPSIS + federated_db_init() + void + + RETURN + FALSE OK + TRUE Error +*/ + +bool federated_db_init() +{ + federated_init= 1; + VOID(pthread_mutex_init(&federated_mutex, MY_MUTEX_INIT_FAST)); + return (hash_init(&federated_open_tables, system_charset_info, 32, 0, 0, + (hash_get_key) federated_get_key, 0, 0)); +} + + +/* + Release the federated handler. + + SYNOPSIS + federated_db_end() + void + + RETURN + FALSE OK +*/ + +bool federated_db_end() +{ + if (federated_init) + { + hash_free(&federated_open_tables); + VOID(pthread_mutex_destroy(&federated_mutex)); + } + federated_init= 0; + return FALSE; } /* @@ -425,14 +468,11 @@ static int parse_url(FEDERATED_SHARE *share, TABLE *table, share->scheme[share->username - share->scheme]= '\0'; if (strcmp(share->scheme, "mysql") != 0) { - DBUG_PRINT("ha_federated::parse_url", - ("The federated handler currently only supports connecting\ - to a MySQL database!!!\n")); my_error(error_num, MYF(0), "ERROR: federated handler only supports remote 'mysql://' database"); DBUG_RETURN(1); } - share->username += 3; + share->username+= 3; if ((share->hostname= strchr(share->username, '@'))) { @@ -447,8 +487,6 @@ static int parse_url(FEDERATED_SHARE *share, TABLE *table, // make sure there isn't an extra / or @ if ((strchr(share->password, '/') || strchr(share->hostname, '@'))) { - DBUG_PRINT("ha_federated::parse_url", - ("this connection string is not in the correct format!!!\n")); my_error(error_num, MYF(0), "this connection string is not in the correct format!!!\n"); DBUG_RETURN(1); @@ -467,8 +505,6 @@ static int parse_url(FEDERATED_SHARE *share, TABLE *table, // make sure there isn't an extra / or @ if ((strchr(share->username, '/')) || (strchr(share->hostname, '@'))) { - DBUG_PRINT("ha_federated::parse_url", - ("this connection string is not in the correct format!!!\n")); my_error(error_num, MYF(0), "this connection string is not in the correct format!!!\n"); DBUG_RETURN(1); @@ -496,8 +532,6 @@ static int parse_url(FEDERATED_SHARE *share, TABLE *table, } else { - DBUG_PRINT("ha_federated::parse_url", - ("this connection string is not in the correct format!!!\n")); my_error(error_num, MYF(0), "this connection string is not in the correct format!!!\n"); DBUG_RETURN(1); @@ -505,8 +539,6 @@ static int parse_url(FEDERATED_SHARE *share, TABLE *table, } else { - DBUG_PRINT("ha_federated::parse_url", - ("this connection string is not in the correct format!!!\n")); my_error(error_num, MYF(0), "this connection string is not in the correct format!!!\n"); DBUG_RETURN(1); @@ -514,8 +546,6 @@ static int parse_url(FEDERATED_SHARE *share, TABLE *table, // make sure there's not an extra / if ((strchr(share->table_base_name, '/'))) { - DBUG_PRINT("ha_federated::parse_url", - ("this connection string is not in the correct format!!!\n")); my_error(error_num, MYF(0), "this connection string is not in the correct format!!!\n"); DBUG_RETURN(1); @@ -533,12 +563,13 @@ static int parse_url(FEDERATED_SHARE *share, TABLE *table, DBUG_PRINT("ha_federated::parse_url", ("scheme %s username %s password %s \ - hostname %s port %d database %s tablename %s\n", share->scheme, share->username, share->password, share->hostname, share->port, share->database, share->table_base_name)); + hostname %s port %d database %s tablename %s\n", + share->scheme, share->username, share->password, + share->hostname, share->port, share->database, + share->table_base_name)); } else { - DBUG_PRINT("ha_federated::parse_url", - ("this connection string is not in the correct format!!!\n")); my_error(error_num, MYF(0), "this connection string is not in the correct format!!!\n"); DBUG_RETURN(1); @@ -546,8 +577,6 @@ static int parse_url(FEDERATED_SHARE *share, TABLE *table, } else { - DBUG_PRINT("ha_federated::parse_url", - ("this connection string is not in the correct format!!!\n")); my_error(error_num, MYF(0), "this connection string is not in the correct format!!!\n"); DBUG_RETURN(1); @@ -573,35 +602,27 @@ static int parse_url(FEDERATED_SHARE *share, TABLE *table, RETURN VALUE 0 After fields have had field values stored from record */ -uint ha_federated::convert_row_to_internal_format(byte *record, MYSQL_ROW row) +uint ha_federated::convert_row_to_internal_format(byte* record, MYSQL_ROW row) { - unsigned long *lengths; - unsigned int num_fields; - unsigned int x= 0; + ulong *lengths; + uint num_fields; + uint x= 0; DBUG_ENTER("ha_federated::convert_row_to_internal_format"); num_fields= mysql_num_fields(result); - lengths= (unsigned long*) my_malloc(num_fields * sizeof(unsigned long), + lengths= (ulong*) my_malloc(num_fields * sizeof(ulong), MYF(0)); - cli_fetch_lengths((unsigned long*) (lengths), row, num_fields); + cli_fetch_lengths((ulong*) (lengths), row, num_fields); memset(record, 0, table->s->null_bytes); - for (Field ** field= table->field; *field; field++, x++) + for (Field **field= table->field; *field; field++, x++) { if (!row[x]) - { (*field)->set_null(); - } else - /* - changed system_charset_info to default_charset_info because - testing revealed that german text was not being retrieved properly - */ - DBUG_PRINT("ha_federated::convert_row_to_internal_format", - ("row[%d] %s length %lu", x, row[x], lengths[x])); - (*field)->store(row[x], lengths[x], &my_charset_bin); + (*field)->store(row[x], lengths[x], &my_charset_bin); } my_free((gptr) lengths, MYF(0)); lengths= 0; @@ -637,11 +658,12 @@ bool ha_federated::create_where_from_key(String *to, KEY *key_info, if (*key++) { if (to->append("IS NULL", 7)) - DBUG_PRINT("ha_federated::create_where_from_key", - ("NULL type %s", to->c_ptr_quick())); - DBUG_RETURN(1); - key_length -= key_part->store_length; - key += key_part->store_length - 1; + DBUG_RETURN(1); + + DBUG_PRINT("ha_federated::create_where_from_key", + ("NULL type %s", to->c_ptr_quick())); + key_length-= key_part->store_length; + key+= key_part->store_length - 1; continue; } key_length--; @@ -655,29 +677,29 @@ bool ha_federated::create_where_from_key(String *to, KEY *key_info, /* This is can be threated as a hex string */ Field_bit *field= (Field_bit *) (key_part->field); char buff[64 + 2], *ptr; - byte *end= (byte *) (key) + length; + byte *end= (byte*)(key)+length; buff[0]= '0'; buff[1]= 'x'; for (ptr= buff + 2; key < end; key++) { - uint tmp= (uint) (uchar) * key; + uint tmp= (uint)(uchar) *key; *ptr++= _dig_vec_upper[tmp >> 4]; *ptr++= _dig_vec_upper[tmp & 15]; } - if (to->append(buff, (uint) (ptr - buff))) + if (to->append(buff, (uint)(ptr - buff))) DBUG_RETURN(1); DBUG_PRINT("ha_federated::create_where_from_key", ("bit type %s", to->c_ptr_quick())); - key_length -= length; + key_length-= length; continue; } if (key_part->key_part_flag & HA_BLOB_PART) { uint blob_length= uint2korr(key); - key += HA_KEY_BLOB_LENGTH; - key_length -= HA_KEY_BLOB_LENGTH; + key+= HA_KEY_BLOB_LENGTH; + key_length-= HA_KEY_BLOB_LENGTH; tmp.set_quick((char*) key, blob_length, &my_charset_bin); if (append_escaped(to, &tmp)) @@ -690,7 +712,7 @@ bool ha_federated::create_where_from_key(String *to, KEY *key_info, else if (key_part->key_part_flag & HA_VAR_LENGTH_PART) { length= uint2korr(key); - key += HA_KEY_BLOB_LENGTH; + key+= HA_KEY_BLOB_LENGTH; tmp.set_quick((char*) key, length, &my_charset_bin); if (append_escaped(to, &tmp)) DBUG_RETURN(1); @@ -722,8 +744,8 @@ bool ha_federated::create_where_from_key(String *to, KEY *key_info, DBUG_RETURN(1); DBUG_PRINT("ha_federated::create_where_from_key", ("final value for 'to' %s", to->c_ptr_quick())); - key += length; - key_length -= length; + key+= length; + key_length-= length; DBUG_RETURN(0); } DBUG_RETURN(1); @@ -744,9 +766,11 @@ static FEDERATED_SHARE *get_share(const char *table_name, TABLE *table) uint table_name_length, table_base_name_length; char *tmp_table_name, *tmp_table_base_name, *table_base_name, *select_query; - // share->table_name has the file location - we want the actual table's - // name! - table_base_name= (char*) table->s->table_name; + /* + share->table_name has the file location - we want the actual table's + name! + */ + table_base_name= (char*)table->s->table_name; DBUG_PRINT("ha_federated::get_share", ("table_name %s", table_base_name)); /* So why does this exist? There is no way currently to init a storage engine. @@ -754,25 +778,12 @@ static FEDERATED_SHARE *get_share(const char *table_name, TABLE *table) do this. Since you will not want to do this, this is probably the next best method. */ - if (!federated_init) - { - /* Hijack a mutex for init'ing the storage engine */ - pthread_mutex_lock(&LOCK_mysql_create_db); - if (!federated_init) - { - federated_init++; - VOID(pthread_mutex_init(&federated_mutex, MY_MUTEX_INIT_FAST)); - (void) hash_init(&federated_open_tables, system_charset_info, 32, 0, 0, - (hash_get_key) federated_get_key, 0, 0); - } - pthread_mutex_unlock(&LOCK_mysql_create_db); - } pthread_mutex_lock(&federated_mutex); table_name_length= (uint) strlen(table_name); table_base_name_length= (uint) strlen(table_base_name); if (!(share= (FEDERATED_SHARE *) hash_search(&federated_open_tables, - (byte *) table_name, + (byte*) table_name, table_name_length))) { query.set_charset(system_charset_info); @@ -804,7 +815,7 @@ static FEDERATED_SHARE *get_share(const char *table_name, TABLE *table) strmov(share->select_query, query.ptr()); DBUG_PRINT("ha_federated::get_share", ("share->select_query %s", share->select_query)); - if (my_hash_insert(&federated_open_tables, (byte *) share)) + if (my_hash_insert(&federated_open_tables, (byte*) share)) goto error; thr_lock_init(&share->lock); pthread_mutex_init(&share->mutex, MY_MUTEX_INIT_FAST); @@ -819,7 +830,6 @@ error2: pthread_mutex_destroy(&share->mutex); error: pthread_mutex_unlock(&federated_mutex); - hash_delete(&federated_open_tables, (byte *) share); if (share->scheme) my_free((gptr) share->scheme, MYF(0)); my_free((gptr) share, MYF(0)); @@ -837,15 +847,14 @@ static int free_share(FEDERATED_SHARE *share) { pthread_mutex_lock(&federated_mutex); - if (share->scheme) - my_free((gptr) share->scheme, MYF(0)); - if (!--share->use_count) { - hash_delete(&federated_open_tables, (byte *) share); - hash_free(&federated_open_tables); + if (share->scheme) + my_free((gptr) share->scheme, MYF(0)); + + hash_delete(&federated_open_tables, (byte*)share); thr_lock_delete(&share->lock); - pthread_mutex_destroy(&share->mutex); + VOID(pthread_mutex_destroy(&share->mutex)); my_free((gptr) share, MYF(0)); } pthread_mutex_unlock(&federated_mutex); @@ -900,7 +909,9 @@ int ha_federated::open(const char *name, int mode, uint test_if_locked) share->hostname, share->username, share->password, - share->database, share->port, NULL, 0)) + share->database, + share->port, + share->socket, 0)) { my_error(ER_CONNECT_TO_MASTER, MYF(0), mysql_error(mysql)); DBUG_RETURN(ER_CONNECT_TO_MASTER); @@ -955,12 +966,9 @@ int ha_federated::close(void) 1 if NULL 0 otherwise */ -inline uint field_in_record_is_null(TABLE * table, /* in: MySQL table - object */ - Field * field, /* in: MySQL field - object */ - char *record) /* in: a row in MySQL - format */ +inline uint field_in_record_is_null(TABLE *table, /* in: MySQL table object */ + Field *field, /* in: MySQL field object */ + char *record) /* in: row in MySQL format */ { int null_offset; DBUG_ENTER("ha_federated::field_in_record_is_null"); @@ -968,7 +976,7 @@ inline uint field_in_record_is_null(TABLE * table, /* in: MySQL table if (!field->null_ptr) DBUG_RETURN(0); - null_offset= (uint) ((char*) field->null_ptr - (char*) table->record[0]); + null_offset= (uint) ((char*)field->null_ptr - (char*)table->record[0]); if (record[null_offset] & field->null_bit) DBUG_RETURN(1); @@ -989,7 +997,7 @@ inline uint field_in_record_is_null(TABLE * table, /* in: MySQL table Called from item_sum.cc, item_sum.cc, sql_acl.cc, sql_insert.cc, sql_insert.cc, sql_select.cc, sql_table.cc, sql_udf.cc, and sql_update.cc. */ -int ha_federated::write_row(byte * buf) +int ha_federated::write_row(byte* buf) { uint x= 0, num_fields= 0; Field **field; @@ -1075,13 +1083,12 @@ int ha_federated::write_row(byte * buf) ("current query id %d field is not null query ID %d", current_query_id, (*field)->query_id)); (*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); - // quote these fields if they require it - - (*field)->quote_data(&insert_field_value_string); // append the value values_string.append(insert_field_value_string); insert_field_value_string.length(0); @@ -1197,7 +1204,7 @@ int ha_federated::update_row(const byte * old_data, byte * new_data) field=oldvalue */ - for (Field ** field= table->field; *field; field++, x++) + for (Field **field= table->field; *field; field++, x++) { /* In all of these tests for 'has_a_primary_key', what I'm trying to @@ -1268,8 +1275,8 @@ int ha_federated::update_row(const byte * old_data, byte * new_data) old_field_value.length(0); } update_string.append(" WHERE "); - update_string.append(where_string.ptr()); - if (!has_a_primary_key) + update_string.append(where_string.c_ptr_quick()); + if (! has_a_primary_key) update_string.append(" LIMIT 1"); DBUG_PRINT("ha_federated::update_row", ("Final update query: %s", @@ -1315,7 +1322,7 @@ int ha_federated::delete_row(const byte * buf) delete_string.append(share->table_base_name); delete_string.append(" WHERE "); - for (Field ** field= table->field; *field; field++, x++) + for (Field **field= table->field; *field; field++, x++) { delete_string.append((*field)->field_name); @@ -1357,7 +1364,7 @@ int ha_federated::delete_row(const byte * buf) index. This method, which is called in the case of an SQL statement having a WHERE clause on a non-primary key index, simply calls index_read_idx. */ -int ha_federated::index_read(byte * buf, const byte * key, +int ha_federated::index_read(byte* buf, const byte * key, uint key_len __attribute__ ((unused)), enum ha_rkey_function find_flag __attribute__ ((unused))) @@ -1375,7 +1382,7 @@ int ha_federated::index_read(byte * buf, const byte * key, a regular non-primary key index, OR is called DIRECTLY when the WHERE clause uses a PRIMARY KEY index. */ -int ha_federated::index_read_idx(byte * buf, uint index, const byte * key, +int ha_federated::index_read_idx(byte* buf, uint index, const byte * key, uint key_len __attribute__ ((unused)), enum ha_rkey_function find_flag __attribute__ ((unused))) @@ -1454,7 +1461,7 @@ int ha_federated::index_init(uint keynr) /* Used to read forward through the index. */ -int ha_federated::index_next(byte * buf) +int ha_federated::index_next(byte* buf) { DBUG_ENTER("ha_federated::index_next"); DBUG_RETURN(rnd_next(buf)); @@ -1546,7 +1553,7 @@ int ha_federated::index_end(void) Called from filesort.cc, records.cc, sql_handler.cc, sql_select.cc, sql_table.cc, and sql_update.cc. */ -int ha_federated::rnd_next(byte *buf) +int ha_federated::rnd_next(byte* buf) { MYSQL_ROW row; DBUG_ENTER("ha_federated::rnd_next"); @@ -1594,7 +1601,7 @@ void ha_federated::position(const byte *record) Called from filesort.cc records.cc sql_insert.cc sql_select.cc sql_update.cc. */ -int ha_federated::rnd_pos(byte * buf, byte *pos) +int ha_federated::rnd_pos(byte* buf, byte *pos) { DBUG_ENTER("ha_federated::rnd_pos"); /* @@ -1606,10 +1613,10 @@ int ha_federated::rnd_pos(byte * buf, byte *pos) { statistic_increment(table->in_use->status_var.ha_read_rnd_count, &LOCK_status); - memcpy_fixed(¤t_position, pos, sizeof(MYSQL_ROW_OFFSET)); // pos - // is - // not - // aligned + /* + pos is not aligned + */ + memcpy_fixed(¤t_position, pos, sizeof(MYSQL_ROW_OFFSET)); result->current_row= 0; result->data_cursor= current_position; DBUG_RETURN(rnd_next(buf)); @@ -1786,4 +1793,4 @@ int ha_federated::create(const char *name, TABLE *table_arg, my_free((gptr) tmp.scheme, MYF(0)); DBUG_RETURN(0); } -#endif /* HAVE_FEDERATED_DB */ +#endif /* HAVE_FEDERATED_DB */ diff --git a/sql/ha_federated.h b/sql/ha_federated.h index 6870a0902e8..04bbf2a56f6 100755 --- a/sql/ha_federated.h +++ b/sql/ha_federated.h @@ -174,3 +174,6 @@ public: THR_LOCK_DATA **store_lock(THD *thd, THR_LOCK_DATA **to, enum thr_lock_type lock_type); //required }; + +bool federated_db_init(void); +bool federated_db_end(void); diff --git a/sql/handler.cc b/sql/handler.cc index b4fed363e87..0e92956c366 100644 --- a/sql/handler.cc +++ b/sql/handler.cc @@ -364,6 +364,16 @@ int ha_init() opt_using_transactions=1; } #endif +#ifdef HAVE_FEDERATED_DB + if (have_federated_db == SHOW_OPTION_YES) + { + if (federated_db_init()) + { + have_federated_db= SHOW_OPTION_DISABLED; + error= 1; + } + } +#endif #ifdef HAVE_ARCHIVE_DB if (have_archive_db == SHOW_OPTION_YES) { @@ -405,6 +415,10 @@ int ha_panic(enum ha_panic_function flag) if (have_ndbcluster == SHOW_OPTION_YES) error|=ndbcluster_end(); #endif +#ifdef HAVE_FEDERATED_DB + if (have_federated_db == SHOW_OPTION_YES) + error|= federated_db_end(); +#endif #ifdef HAVE_ARCHIVE_DB if (have_archive_db == SHOW_OPTION_YES) error|= archive_db_end(); From de2fea12c329dbbca26f20a5dcb8cead15a6970a Mon Sep 17 00:00:00 2001 From: "patg@krsna.patg.net" <> Date: Thu, 17 Feb 2005 16:17:21 -0800 Subject: [PATCH 3/4] WL# 2094, This patch is on top of 1.1814, 1.1846, 1.1856, which contain fixes for bugs 8033, 8065, 8535, 8582 This particular changeset contains style changes per code review suggestions and does not contain bug fixes in of itself. ha_federated.h: standardised code style to conform to internals.texi ha_federated.cc: more code standardisation to conform to internals.texi. - casts - declarations - comments - 80 char width also, append using string1.append(string2) and not string1.append(string2.c_ptr_quick()) --- sql/ha_federated.cc | 107 ++++++++++++++++++++++---------------------- sql/ha_federated.h | 12 +++-- 2 files changed, 62 insertions(+), 57 deletions(-) diff --git a/sql/ha_federated.cc b/sql/ha_federated.cc index 500dcda3e21..da62d51c83e 100644 --- a/sql/ha_federated.cc +++ b/sql/ha_federated.cc @@ -369,7 +369,7 @@ static byte *federated_get_key(FEDERATED_SHARE *share, uint *length, my_bool not_used __attribute__ ((unused))) { *length= share->table_name_length; - return (byte*)share->table_name; + return (byte*) share->table_name; } /* @@ -456,7 +456,6 @@ static int parse_url(FEDERATED_SHARE *share, TABLE *table, { DBUG_ENTER("ha_federated::parse_url"); - // This either get set or will remain the same. share->port= 0; uint error_num= table_create_flag ? ER_CANT_CREATE_TABLE : ER_CONNECT_TO_MASTER; @@ -484,7 +483,9 @@ static int parse_url(FEDERATED_SHARE *share, TABLE *table, share->username[share->password - share->username]= '\0'; share->password++; share->username= share->username; - // make sure there isn't an extra / or @ + /* + make sure there isn't an extra / or @ + */ if ((strchr(share->password, '/') || strchr(share->hostname, '@'))) { my_error(error_num, MYF(0), @@ -502,7 +503,9 @@ static int parse_url(FEDERATED_SHARE *share, TABLE *table, else share->username= share->username; - // make sure there isn't an extra / or @ + /* + make sure there isn't an extra / or @ + */ if ((strchr(share->username, '/')) || (strchr(share->hostname, '@'))) { my_error(error_num, MYF(0), @@ -543,7 +546,9 @@ static int parse_url(FEDERATED_SHARE *share, TABLE *table, "this connection string is not in the correct format!!!\n"); DBUG_RETURN(1); } - // make sure there's not an extra / + /* + make sure there's not an extra / + */ if ((strchr(share->table_base_name, '/'))) { my_error(error_num, MYF(0), @@ -602,7 +607,7 @@ static int parse_url(FEDERATED_SHARE *share, TABLE *table, RETURN VALUE 0 After fields have had field values stored from record */ -uint ha_federated::convert_row_to_internal_format(byte* record, MYSQL_ROW row) +uint ha_federated::convert_row_to_internal_format(byte *record, MYSQL_ROW row) { ulong *lengths; uint num_fields; @@ -611,9 +616,8 @@ uint ha_federated::convert_row_to_internal_format(byte* record, MYSQL_ROW row) DBUG_ENTER("ha_federated::convert_row_to_internal_format"); num_fields= mysql_num_fields(result); - lengths= (ulong*) my_malloc(num_fields * sizeof(ulong), - MYF(0)); - cli_fetch_lengths((ulong*) (lengths), row, num_fields); + lengths= (ulong*) my_malloc(num_fields * sizeof(ulong), MYF(0)); + cli_fetch_lengths((ulong*) lengths, row, num_fields); memset(record, 0, table->s->null_bytes); @@ -770,7 +774,7 @@ static FEDERATED_SHARE *get_share(const char *table_name, TABLE *table) share->table_name has the file location - we want the actual table's name! */ - table_base_name= (char*)table->s->table_name; + table_base_name= (char*) table->s->table_name; DBUG_PRINT("ha_federated::get_share", ("table_name %s", table_base_name)); /* So why does this exist? There is no way currently to init a storage engine. @@ -825,13 +829,11 @@ static FEDERATED_SHARE *get_share(const char *table_name, TABLE *table) return share; -error2: - thr_lock_delete(&share->lock); - pthread_mutex_destroy(&share->mutex); error: 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; @@ -852,7 +854,7 @@ static int free_share(FEDERATED_SHARE *share) if (share->scheme) my_free((gptr) share->scheme, MYF(0)); - hash_delete(&federated_open_tables, (byte*)share); + hash_delete(&federated_open_tables, (byte*) share); thr_lock_delete(&share->lock); VOID(pthread_mutex_destroy(&share->mutex)); my_free((gptr) share, MYF(0)); @@ -934,7 +936,7 @@ int ha_federated::close(void) { DBUG_ENTER("ha_federated::close"); - // free the result set + /* free the result set */ if (result) { DBUG_PRINT("ha_federated::close", @@ -966,9 +968,9 @@ int ha_federated::close(void) 1 if NULL 0 otherwise */ -inline uint field_in_record_is_null(TABLE *table, /* in: MySQL table object */ - Field *field, /* in: MySQL field object */ - char *record) /* in: row in MySQL format */ +inline uint field_in_record_is_null(TABLE *table, + Field *field, + char *record) { int null_offset; DBUG_ENTER("ha_federated::field_in_record_is_null"); @@ -997,7 +999,7 @@ inline uint field_in_record_is_null(TABLE *table, /* in: MySQL table object */ Called from item_sum.cc, item_sum.cc, sql_acl.cc, sql_insert.cc, sql_insert.cc, sql_select.cc, sql_table.cc, sql_udf.cc, and sql_update.cc. */ -int ha_federated::write_row(byte* buf) +int ha_federated::write_row(byte *buf) { uint x= 0, num_fields= 0; Field **field; @@ -1008,23 +1010,20 @@ int ha_federated::write_row(byte* buf) char insert_buffer[IO_SIZE]; char values_buffer[IO_SIZE], insert_field_value_buffer[IO_SIZE]; - // The main insert query string + /* The main insert query string */ String insert_string(insert_buffer, sizeof(insert_buffer), &my_charset_bin); insert_string.length(0); - // The string containing the values to be added to the insert + /* The string containing the values to be added to the insert */ String values_string(values_buffer, sizeof(values_buffer), &my_charset_bin); values_string.length(0); - // The actual value of the field, to be added to the values_string + /* The actual value of the field, to be added to the values_string */ String insert_field_value_string(insert_field_value_buffer, sizeof(insert_field_value_buffer), &my_charset_bin); insert_field_value_string.length(0); DBUG_ENTER("ha_federated::write_row"); - /* - I want to use this and the next line, but the repository needs to be - updated to do so - */ + 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(); @@ -1038,10 +1037,10 @@ int ha_federated::write_row(byte* buf) DBUG_PRINT("ha_federated::write_row", ("current query id %d", current_query_id)); - // start off our string + /* start off our string */ insert_string.append("INSERT INTO "); insert_string.append(share->table_base_name); - // start both our field and field values strings + /* start both our field and field values strings */ insert_string.append(" ("); values_string.append(" VALUES ("); @@ -1083,17 +1082,17 @@ int ha_federated::write_row(byte* buf) ("current query id %d field is not null query ID %d", current_query_id, (*field)->query_id)); (*field)->val_str(&insert_field_value_string); - // quote these fields if they require it + /* quote these fields if they require it */ (*field)->quote_data(&insert_field_value_string); } - // append the field name + /* append the field name */ insert_string.append((*field)->field_name); - // append the value + /* append the value */ values_string.append(insert_field_value_string); insert_field_value_string.length(0); - // append commas between both fields and fieldnames + /* append commas between both fields and fieldnames */ insert_string.append(','); values_string.append(','); DBUG_PRINT("ha_federated::write_row", @@ -1120,14 +1119,14 @@ int ha_federated::write_row(byte* buf) DBUG_PRINT("ha_federated::write_row", ("x %d num fields %d", x, num_fields)); if (num_fields > 0) { - // chops off leading commas + /* chops off leading commas */ values_string.chop(); insert_string.append(')'); } - // we always want to append this, even if there aren't any fields + /* we always want to append this, even if there aren't any fields */ values_string.append(')'); - // add the values + /* add the values */ insert_string.append(values_string); DBUG_PRINT("ha_federated::write_row", ("insert query %s", @@ -1158,7 +1157,7 @@ int ha_federated::write_row(byte* buf) Called from sql_select.cc, sql_acl.cc, sql_update.cc, and sql_insert.cc. */ -int ha_federated::update_row(const byte * old_data, byte * new_data) +int ha_federated::update_row(const byte *old_data, byte *new_data) { uint x= 0; uint has_a_primary_key= 0; @@ -1166,18 +1165,20 @@ int ha_federated::update_row(const byte * old_data, byte * new_data) char old_field_value_buffer[IO_SIZE], new_field_value_buffer[IO_SIZE]; char update_buffer[IO_SIZE], where_buffer[IO_SIZE]; - // stores the value to be replaced of the field were are updating + /* + 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); old_field_value.length(0); - // stores the new value of the field + /* stores the new value of the field */ String new_field_value(new_field_value_buffer, sizeof(new_field_value_buffer), &my_charset_bin); new_field_value.length(0); - // stores the update query + /* stores the update query */ String update_string(update_buffer, sizeof(update_buffer), &my_charset_bin); update_string.length(0); - // stores the WHERE clause + /* stores the WHERE clause */ String where_string(where_buffer, sizeof(where_buffer), &my_charset_bin); where_string.length(0); @@ -1275,7 +1276,7 @@ int ha_federated::update_row(const byte * old_data, byte * new_data) old_field_value.length(0); } update_string.append(" WHERE "); - update_string.append(where_string.c_ptr_quick()); + update_string.append(where_string); if (! has_a_primary_key) update_string.append(" LIMIT 1"); @@ -1305,7 +1306,7 @@ int ha_federated::update_row(const byte * old_data, byte * new_data) it is used for removing duplicates while in insert it is used for REPLACE calls. */ -int ha_federated::delete_row(const byte * buf) +int ha_federated::delete_row(const byte *buf) { uint x= 0; char delete_buffer[IO_SIZE]; @@ -1364,7 +1365,7 @@ int ha_federated::delete_row(const byte * buf) index. This method, which is called in the case of an SQL statement having a WHERE clause on a non-primary key index, simply calls index_read_idx. */ -int ha_federated::index_read(byte* buf, const byte * key, +int ha_federated::index_read(byte *buf, const byte *key, uint key_len __attribute__ ((unused)), enum ha_rkey_function find_flag __attribute__ ((unused))) @@ -1382,7 +1383,7 @@ int ha_federated::index_read(byte* buf, const byte * key, a regular non-primary key index, OR is called DIRECTLY when the WHERE clause uses a PRIMARY KEY index. */ -int ha_federated::index_read_idx(byte* buf, uint index, const byte * key, +int ha_federated::index_read_idx(byte *buf, uint index, const byte *key, uint key_len __attribute__ ((unused)), enum ha_rkey_function find_flag __attribute__ ((unused))) @@ -1411,7 +1412,7 @@ int ha_federated::index_read_idx(byte* buf, uint index, const byte * key, DBUG_PRINT("ha_federated::index_read_idx", ("current key %d key value %s index_string value %s length %d", - index, (char*) (key), index_string.c_ptr_quick(), + index, (char*) key, index_string.c_ptr_quick(), index_string.length())); DBUG_PRINT("ha_federated::index_read_idx", @@ -1461,7 +1462,7 @@ int ha_federated::index_init(uint keynr) /* Used to read forward through the index. */ -int ha_federated::index_next(byte* buf) +int ha_federated::index_next(byte *buf) { DBUG_ENTER("ha_federated::index_next"); DBUG_RETURN(rnd_next(buf)); @@ -1553,7 +1554,7 @@ int ha_federated::index_end(void) Called from filesort.cc, records.cc, sql_handler.cc, sql_select.cc, sql_table.cc, and sql_update.cc. */ -int ha_federated::rnd_next(byte* buf) +int ha_federated::rnd_next(byte *buf) { MYSQL_ROW row; DBUG_ENTER("ha_federated::rnd_next"); @@ -1601,7 +1602,7 @@ void ha_federated::position(const byte *record) Called from filesort.cc records.cc sql_insert.cc sql_select.cc sql_update.cc. */ -int ha_federated::rnd_pos(byte* buf, byte *pos) +int ha_federated::rnd_pos(byte *buf, byte *pos) { DBUG_ENTER("ha_federated::rnd_pos"); /* @@ -1613,10 +1614,8 @@ int ha_federated::rnd_pos(byte* buf, byte *pos) { statistic_increment(table->in_use->status_var.ha_read_rnd_count, &LOCK_status); - /* - pos is not aligned - */ - memcpy_fixed(¤t_position, pos, sizeof(MYSQL_ROW_OFFSET)); + memcpy_fixed(¤t_position, pos, sizeof(MYSQL_ROW_OFFSET)); // pos + /* is not aligned */ result->current_row= 0; result->data_cursor= current_position; DBUG_RETURN(rnd_next(buf)); @@ -1668,11 +1667,11 @@ int ha_federated::rnd_pos(byte* buf, byte *pos) sql_update.cc */ -// FIX: later version provide better information to the optimizer +/* FIX: later version provide better information to the optimizer */ void ha_federated::info(uint flag) { DBUG_ENTER("ha_federated::info"); - records= 10000; // Fake! + records= 10000; // fix later DBUG_VOID_RETURN; } diff --git a/sql/ha_federated.h b/sql/ha_federated.h index 04bbf2a56f6..e75fd285338 100755 --- a/sql/ha_federated.h +++ b/sql/ha_federated.h @@ -73,7 +73,8 @@ private: return errorcode otherwise */ uint convert_row_to_internal_format(byte *buf, MYSQL_ROW row); - bool ha_federated::create_where_from_key(String *to, KEY *key_info, const byte *key, uint key_length); + bool ha_federated::create_where_from_key(String *to, KEY *key_info, + const byte *key, uint key_length); public: ha_federated(TABLE *table): handler(table), @@ -127,11 +128,16 @@ public: /* Called in test_quick_select to determine if indexes should be used. */ - virtual double scan_time() { DBUG_PRINT("ha_federated::scan_time", ("rows %d", records)); return (double)(records*2); } + virtual double scan_time() + { + DBUG_PRINT("ha_federated::scan_time", + ("rows %d", records)); return (double)(records*2); + } /* The next method will never be called if you do not implement indexes. */ - virtual double read_time(uint index, uint ranges, ha_rows rows) { return (double) rows / 20.0+1; } + virtual double read_time(uint index, uint ranges, ha_rows rows) + { return (double) rows / 20.0+1; } /* Everything below are methods that we implment in ha_federated.cc. From c33868e70ce38d2aa54bd2e2937bc904b6f434b0 Mon Sep 17 00:00:00 2001 From: "patg@krsna.patg.net" <> Date: Sat, 19 Feb 2005 10:45:19 -0800 Subject: [PATCH 4/4] WL# 2094, Federated Storage Handler. This patch fixes bug #8599, HPUX compile errors. Testing on hp3750 shows these fixes fix the compile problems on HPUX, but I have a problem where when I run the tests, the test shows that the tables default to MyISAM! --- include/mysql.h | 2 +- sql/ha_federated.h | 13 ++++++++----- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/include/mysql.h b/include/mysql.h index e37cf710817..828d38f5ec4 100644 --- a/include/mysql.h +++ b/include/mysql.h @@ -499,7 +499,7 @@ MYSQL_FIELD_OFFSET STDCALL mysql_field_seek(MYSQL_RES *result, MYSQL_FIELD_OFFSET offset); MYSQL_ROW STDCALL mysql_fetch_row(MYSQL_RES *result); unsigned long * STDCALL mysql_fetch_lengths(MYSQL_RES *result); -void STDCALL cli_fetch_lengths(ulong *to, MYSQL_ROW column, +void STDCALL cli_fetch_lengths(unsigned long *to, MYSQL_ROW column, unsigned int field_count); MYSQL_FIELD * STDCALL mysql_fetch_field(MYSQL_RES *result); MYSQL_RES * STDCALL mysql_list_fields(MYSQL *mysql, const char *table, diff --git a/sql/ha_federated.h b/sql/ha_federated.h index e75fd285338..28f28f8aa63 100755 --- a/sql/ha_federated.h +++ b/sql/ha_federated.h @@ -32,13 +32,16 @@ FEDERATED_SHARE is a structure that will be shared amoung all open handlers The example implements the minimum of what you will probably need. */ -//FIX document typedef struct st_federated_share { char *table_name; char *table_base_name; - // the primary select query to be used in rnd_init + /* + the primary select query to be used in rnd_init + */ char *select_query; - // remote host info, parse_url supplies + /* + remote host info, parse_url supplies + */ char *scheme; char *hostname; char *username; @@ -73,8 +76,8 @@ private: return errorcode otherwise */ uint convert_row_to_internal_format(byte *buf, MYSQL_ROW row); - bool ha_federated::create_where_from_key(String *to, KEY *key_info, - const byte *key, uint key_length); + bool create_where_from_key(String *to, KEY *key_info, + const byte *key, uint key_length); public: ha_federated(TABLE *table): handler(table),