diff --git a/include/my_base.h b/include/my_base.h index 05ba38e77eb..d8a0e15ccbe 100644 --- a/include/my_base.h +++ b/include/my_base.h @@ -356,8 +356,9 @@ enum ha_base_keytype { #define HA_ERR_NULL_IN_SPATIAL 158 /* NULLs are not supported in spatial index */ #define HA_ERR_TABLE_DEF_CHANGED 159 /* The table changed in storage engine */ #define HA_ERR_TABLE_NEEDS_UPGRADE 160 /* The table changed in storage engine */ +#define HA_ERR_TABLE_READONLY 161 /* The table is not writable */ -#define HA_ERR_LAST 160 /*Copy last error nr.*/ +#define HA_ERR_LAST 161 /*Copy last error nr.*/ /* Add error numbers before HA_ERR_LAST and change it accordingly. */ #define HA_ERR_ERRORS (HA_ERR_LAST - HA_ERR_FIRST + 1) diff --git a/mysql-test/r/federated.result b/mysql-test/r/federated.result index 709e44579e2..5d17afd8cfc 100644 --- a/mysql-test/r/federated.result +++ b/mysql-test/r/federated.result @@ -1689,6 +1689,44 @@ id c1 c2 9 abc ppc drop table federated.t1, federated.t2; drop table federated.t1, federated.t2; +DROP TABLE IF EXISTS federated.test; +CREATE TABLE federated.test ( +`id` int(11) NOT NULL, +`val1` varchar(255) NOT NULL, +`val2` varchar(255) NOT NULL, +PRIMARY KEY (`id`) +) ENGINE=MyISAM DEFAULT CHARSET=latin1; +DROP TABLE IF EXISTS federated.test_local; +DROP TABLE IF EXISTS federated.test_remote; +CREATE TABLE federated.test_local ( +`id` int(11) NOT NULL, +`val1` varchar(255) NOT NULL, +`val2` varchar(255) NOT NULL, +PRIMARY KEY (`id`) +) ENGINE=MyISAM DEFAULT CHARSET=latin1; +INSERT INTO federated.test_local VALUES (1, 'foo', 'bar'), +(2, 'bar', 'foo'); +CREATE TABLE federated.test_remote ( +`id` int(11) NOT NULL, +`val1` varchar(255) NOT NULL, +`val2` varchar(255) NOT NULL, +PRIMARY KEY (`id`) +) ENGINE=FEDERATED DEFAULT CHARSET=latin1 +CONNECTION='mysql://root@127.0.0.1:SLAVE_PORT/federated/test'; +insert into federated.test_remote select * from federated.test_local; +select * from federated.test_remote; +id val1 val2 +1 foo bar +2 bar foo +delete from federated.test_remote where id in (1,2); +insert into federated.test_remote select * from federated.test_local; +select * from federated.test_remote; +id val1 val2 +2 bar foo +1 foo bar +DROP TABLE federated.test_local; +DROP TABLE federated.test_remote; +DROP TABLE federated.test; drop table if exists federated.t1; create table federated.t1 (a int, b int, c int); drop table if exists federated.t1; @@ -1733,6 +1771,19 @@ id val 2 0 drop table t1; drop table t1; +create table t1 (a longblob not null); +create table t1 +(a longblob not null) engine=federated +connection='mysql://root@127.0.0.1:SLAVE_PORT/test/t1'; +insert into t1 values (repeat('a',5000)); +select length(a) from t1; +length(a) +5000 +select length(a) from t1; +length(a) +5000 +drop table t1; +drop table t1; DROP TABLE IF EXISTS federated.t1; DROP DATABASE IF EXISTS federated; DROP TABLE IF EXISTS federated.t1; diff --git a/mysql-test/r/merge.result b/mysql-test/r/merge.result index b1abe16a091..b8fdd24be74 100644 --- a/mysql-test/r/merge.result +++ b/mysql-test/r/merge.result @@ -774,3 +774,15 @@ create table tm (b bit(1)) engine = merge union = (t1,t2); select * from tm; b drop table tm, t1, t2; +create table t1 (a int) insert_method = last engine = merge; +insert into t1 values (1); +ERROR HY000: Table 't1' is read only +create table t2 (a int) engine = myisam; +alter table t1 union (t2); +insert into t1 values (1); +alter table t1 insert_method = no; +insert into t1 values (1); +ERROR HY000: Table 't1' is read only +drop table t2; +drop table t1; +End of 5.0 tests diff --git a/mysql-test/t/federated.test b/mysql-test/t/federated.test index 773c9121af0..38beab605fd 100644 --- a/mysql-test/t/federated.test +++ b/mysql-test/t/federated.test @@ -1365,6 +1365,62 @@ drop table federated.t1, federated.t2; connection slave; drop table federated.t1, federated.t2; +# +# BUG #18764: Delete conditions causing inconsistencies in Federated tables +# +connection slave; +--disable_warnings +DROP TABLE IF EXISTS federated.test; +--enable_warnings +CREATE TABLE federated.test ( + `id` int(11) NOT NULL, + `val1` varchar(255) NOT NULL, + `val2` varchar(255) NOT NULL, + PRIMARY KEY (`id`) + ) ENGINE=MyISAM DEFAULT CHARSET=latin1; + +connection master; +--disable_warnings +DROP TABLE IF EXISTS federated.test_local; +DROP TABLE IF EXISTS federated.test_remote; +--enable_warnings +CREATE TABLE federated.test_local ( + `id` int(11) NOT NULL, + `val1` varchar(255) NOT NULL, + `val2` varchar(255) NOT NULL, + PRIMARY KEY (`id`) +) ENGINE=MyISAM DEFAULT CHARSET=latin1; + +INSERT INTO federated.test_local VALUES (1, 'foo', 'bar'), +(2, 'bar', 'foo'); + +--replace_result $SLAVE_MYPORT SLAVE_PORT +eval CREATE TABLE federated.test_remote ( + `id` int(11) NOT NULL, + `val1` varchar(255) NOT NULL, + `val2` varchar(255) NOT NULL, + PRIMARY KEY (`id`) +) ENGINE=FEDERATED DEFAULT CHARSET=latin1 +CONNECTION='mysql://root@127.0.0.1:$SLAVE_MYPORT/federated/test'; + +insert into federated.test_remote select * from federated.test_local; + +select * from federated.test_remote; + +delete from federated.test_remote where id in (1,2); + +insert into federated.test_remote select * from federated.test_local; + +select * from federated.test_remote; +--disable_warnings +DROP TABLE federated.test_local; +DROP TABLE federated.test_remote; +--enable_warnings +connection slave; +--disable_warnings +DROP TABLE federated.test; +--enable_warnings + # # Additional test for bug#18437 "Wrong values inserted with a before # update trigger on NDB table". SQL-layer didn't properly inform @@ -1425,4 +1481,22 @@ drop table t1; connection master; drop table t1; +# +# Bug #17608: String literals lost during INSERT query on FEDERATED table +# +connection slave; +create table t1 (a longblob not null); +connection master; +--replace_result $SLAVE_MYPORT SLAVE_PORT +eval create table t1 + (a longblob not null) engine=federated + connection='mysql://root@127.0.0.1:$SLAVE_MYPORT/test/t1'; +insert into t1 values (repeat('a',5000)); +select length(a) from t1; +connection slave; +select length(a) from t1; +drop table t1; +connection master; +drop table t1; + source include/federated_cleanup.inc; diff --git a/mysql-test/t/merge.test b/mysql-test/t/merge.test index 639129e1393..93fbc631680 100644 --- a/mysql-test/t/merge.test +++ b/mysql-test/t/merge.test @@ -389,4 +389,19 @@ create table tm (b bit(1)) engine = merge union = (t1,t2); select * from tm; drop table tm, t1, t2; -# End of 5.0 tests +# +# Bug #17766: The server accepts to create MERGE tables which cannot work +# +create table t1 (a int) insert_method = last engine = merge; +--error ER_OPEN_AS_READONLY +insert into t1 values (1); +create table t2 (a int) engine = myisam; +alter table t1 union (t2); +insert into t1 values (1); +alter table t1 insert_method = no; +--error ER_OPEN_AS_READONLY +insert into t1 values (1); +drop table t2; +drop table t1; + +--echo End of 5.0 tests diff --git a/sql/field.cc b/sql/field.cc index 946351efe36..2cf4a2f83af 100644 --- a/sql/field.cc +++ b/sql/field.cc @@ -1551,104 +1551,6 @@ Field *Field::new_key_field(MEM_ROOT *root, struct st_table *new_table, } -/* - SYNOPSIS - Field::quote_data() - unquoted_string Pointer pointing to the value of a field - - DESCRIPTION - Simple method that passes the field type to the method "type_quote" - To get a true/false value as to whether the value in string1 needs - to be enclosed with quotes. This ensures that values in the final - sql statement to be passed to the remote server will be quoted properly - - RETURN_VALUE - void Immediately - if string doesn't need quote - void Upon prepending/appending quotes on each side of variable - -*/ - -bool Field::quote_data(String *unquoted_string) -{ - char escaped_string[IO_SIZE]; - DBUG_ENTER("Field::quote_data"); - - if (!needs_quotes()) - DBUG_RETURN(0); - - // this is the same call that mysql_real_escape_string() calls - if (escape_string_for_mysql(&my_charset_bin, (char *)escaped_string, - sizeof(escaped_string), unquoted_string->ptr(), - unquoted_string->length()) == (ulong)~0) - DBUG_RETURN(1); - - // reset string, then re-append with quotes and escaped values - unquoted_string->length(0); - if (unquoted_string->append('\'') || - unquoted_string->append((char *)escaped_string) || - unquoted_string->append('\'')) - DBUG_RETURN(1); - DBUG_RETURN(0); -} - - -/* - Quote a field type if needed - - SYNOPSIS - Field::type_quote - - DESCRIPTION - Simple method to give true/false whether a field should be quoted. - Used when constructing INSERT and UPDATE queries to the remote server - see write_row and update_row - - RETURN VALUE - 0 if value is of type NOT needing quotes - 1 if value is of type needing quotes -*/ - -bool Field::needs_quotes(void) -{ - DBUG_ENTER("Field::type_quote"); - - switch (type()) { - //FIX this when kernel is fixed - case MYSQL_TYPE_VARCHAR : - case FIELD_TYPE_STRING : - case FIELD_TYPE_VAR_STRING : - case FIELD_TYPE_YEAR : - case FIELD_TYPE_NEWDATE : - case FIELD_TYPE_TIME : - case FIELD_TYPE_TIMESTAMP : - case FIELD_TYPE_DATE : - case FIELD_TYPE_DATETIME : - case FIELD_TYPE_TINY_BLOB : - case FIELD_TYPE_BLOB : - case FIELD_TYPE_MEDIUM_BLOB : - case FIELD_TYPE_LONG_BLOB : - case FIELD_TYPE_GEOMETRY : - case FIELD_TYPE_BIT: - DBUG_RETURN(1); - - case FIELD_TYPE_DECIMAL : - case FIELD_TYPE_TINY : - case FIELD_TYPE_SHORT : - case FIELD_TYPE_INT24 : - case FIELD_TYPE_LONG : - case FIELD_TYPE_FLOAT : - case FIELD_TYPE_DOUBLE : - case FIELD_TYPE_LONGLONG : - case FIELD_TYPE_NULL : - case FIELD_TYPE_SET : - case FIELD_TYPE_ENUM : - DBUG_RETURN(0); - default: - DBUG_RETURN(0); - } -} - - /**************************************************************************** Field_null, a field that always return NULL ****************************************************************************/ diff --git a/sql/field.h b/sql/field.h index 3b33d3651e3..09638b9a979 100644 --- a/sql/field.h +++ b/sql/field.h @@ -251,8 +251,6 @@ public: ptr= old_ptr; return str; } - bool quote_data(String *unquoted_string); - bool needs_quotes(void); virtual bool send_binary(Protocol *protocol); virtual char *pack(char* to, const char *from, uint max_length=~(uint) 0) { diff --git a/sql/ha_federated.cc b/sql/ha_federated.cc index e2988df1619..2267c2b5d79 100644 --- a/sql/ha_federated.cc +++ b/sql/ha_federated.cc @@ -124,11 +124,6 @@ ha_federated::write_row - - Field::quote_data - Field::quote_data - - ha_federated::reset (UPDATE) @@ -138,20 +133,10 @@ ha_federated::index_init ha_federated::index_read ha_federated::index_read_idx - Field::quote_data ha_federated::rnd_next ha_federated::convert_row_to_internal_format ha_federated::update_row - - Field::quote_data - Field::quote_data - Field::quote_data - Field::quote_data - Field::quote_data - Field::quote_data - - ha_federated::extra ha_federated::extra ha_federated::extra @@ -1151,7 +1136,7 @@ bool ha_federated::create_where_from_key(String *to, Field *field= key_part->field; uint store_length= key_part->store_length; uint part_length= min(store_length, length); - needs_quotes= field->needs_quotes(); + needs_quotes= 1; DBUG_DUMP("key, start of loop", (char *) ptr, length); if (key_part->null_bit) @@ -1559,10 +1544,6 @@ inline uint field_in_record_is_null(TABLE *table, int ha_federated::write_row(byte *buf) { - bool has_fields= FALSE; - uint all_fields_have_same_query_id= 1; - ulong current_query_id= 1; - ulong tmp_query_id= 1; char insert_buffer[FEDERATED_QUERY_BUFFER_SIZE]; char values_buffer[FEDERATED_QUERY_BUFFER_SIZE]; char insert_field_value_buffer[STRING_BUFFER_USUAL_SIZE]; @@ -1580,23 +1561,11 @@ int ha_federated::write_row(byte *buf) insert_string.length(0); insert_field_value_string.length(0); DBUG_ENTER("ha_federated::write_row"); - DBUG_PRINT("info", - ("table charset name %s csname %s", - table->s->table_charset->name, - table->s->table_charset->csname)); statistic_increment(table->in_use->status_var.ha_write_count, &LOCK_status); if (table->timestamp_field_type & TIMESTAMP_AUTO_SET_ON_INSERT) table->timestamp_field->set_time(); - /* - get the current query id - the fields that we add to the insert - statement to send to the foreign will not be appended unless they match - this query id - */ - current_query_id= table->in_use->query_id; - DBUG_PRINT("info", ("current query id %d", current_query_id)); - /* start both our field and field values strings */ @@ -1609,64 +1578,40 @@ int ha_federated::write_row(byte *buf) values_string.append(FEDERATED_VALUES); values_string.append(FEDERATED_OPENPAREN); - /* - Even if one field is different, all_fields_same_query_id can't remain - 0 if it remains 0, then that means no fields were specified in the query - such as in the case of INSERT INTO table VALUES (val1, val2, valN) - - */ - for (field= table->field; *field; field++) - { - if (field > table->field && tmp_query_id != (*field)->query_id) - all_fields_have_same_query_id= 0; - - tmp_query_id= (*field)->query_id; - } /* loop through the field pointer array, add any fields to both the values list and the fields list that match the current query id - - You might ask "Why an index variable (has_fields) ?" My answer is that - we need to count how many fields we actually need */ for (field= table->field; *field; field++) { - /* if there is a query id and if it's equal to the current query id */ - if (((*field)->query_id && (*field)->query_id == current_query_id) - || all_fields_have_same_query_id) + if ((*field)->is_null()) + insert_field_value_string.append(FEDERATED_NULL); + else { - /* - There are some fields. This will be used later to determine - whether to chop off commas and parens. - */ - has_fields= TRUE; + (*field)->val_str(&insert_field_value_string); + values_string.append('\''); + insert_field_value_string.print(&values_string); + values_string.append('\''); - if ((*field)->is_null()) - insert_field_value_string.append(FEDERATED_NULL); - else - { - (*field)->val_str(&insert_field_value_string); - /* quote these fields if they require it */ - (*field)->quote_data(&insert_field_value_string); - } - /* append the field name */ - insert_string.append((*field)->field_name); - - /* append the value */ - values_string.append(insert_field_value_string); insert_field_value_string.length(0); - - /* append commas between both fields and fieldnames */ - /* - unfortunately, we can't use the logic - if *(fields + 1) to make the following - appends conditional because we may not append - if the next field doesn't match the condition: - (((*field)->query_id && (*field)->query_id == current_query_id) - */ - insert_string.append(FEDERATED_COMMA); - values_string.append(FEDERATED_COMMA); } + /* append the field name */ + insert_string.append((*field)->field_name); + + /* append the value */ + values_string.append(insert_field_value_string); + insert_field_value_string.length(0); + + /* append commas between both fields and fieldnames */ + /* + unfortunately, we can't use the logic + if *(fields + 1) to make the following + appends conditional because we may not append + if the next field doesn't match the condition: + (((*field)->query_id && (*field)->query_id == current_query_id) + */ + insert_string.append(FEDERATED_COMMA); + values_string.append(FEDERATED_COMMA); } /* @@ -1678,7 +1623,7 @@ int ha_federated::write_row(byte *buf) AND, we don't want to chop off the last char '(' insert will be "INSERT INTO t1 VALUES ();" */ - if (has_fields) + if (table->s->fields) { /* chops off leading commas */ values_string.length(values_string.length() - strlen(FEDERATED_COMMA)); @@ -1861,8 +1806,9 @@ int ha_federated::update_row(const byte *old_data, byte *new_data) { /* otherwise = */ (*field)->val_str(&field_value); - (*field)->quote_data(&field_value); - update_string.append(field_value); + update_string.append('\''); + field_value.print(&update_string); + update_string.append('\''); field_value.length(0); } @@ -1873,8 +1819,9 @@ int ha_federated::update_row(const byte *old_data, byte *new_data) where_string.append(FEDERATED_EQ); (*field)->val_str(&field_value, (char*) (old_data + (*field)->offset())); - (*field)->quote_data(&field_value); - where_string.append(field_value); + where_string.append('\''); + field_value.print(&where_string); + where_string.append('\''); field_value.length(0); } @@ -1944,17 +1891,17 @@ int ha_federated::delete_row(const byte *buf) if (cur_field->is_null()) { - delete_string.append(FEDERATED_IS); - data_string.append(FEDERATED_NULL); + delete_string.append(FEDERATED_ISNULL); } else { delete_string.append(FEDERATED_EQ); cur_field->val_str(&data_string); - cur_field->quote_data(&data_string); + delete_string.append('\''); + data_string.print(&delete_string); + delete_string.append('\''); } - delete_string.append(data_string); delete_string.append(FEDERATED_AND); } delete_string.length(delete_string.length()-5); // Remove trailing AND diff --git a/sql/ha_myisammrg.cc b/sql/ha_myisammrg.cc index 9780f163634..0b6e05fcbd4 100644 --- a/sql/ha_myisammrg.cc +++ b/sql/ha_myisammrg.cc @@ -132,6 +132,10 @@ int ha_myisammrg::close(void) int ha_myisammrg::write_row(byte * buf) { statistic_increment(table->in_use->status_var.ha_write_count,&LOCK_status); + + if (file->merge_insert_method == MERGE_INSERT_DISABLED || !file->tables) + return (HA_ERR_TABLE_READONLY); + if (table->timestamp_field_type & TIMESTAMP_AUTO_SET_ON_INSERT) table->timestamp_field->set_time(); if (table->next_number_field && buf == table->record[0]) diff --git a/sql/handler.cc b/sql/handler.cc index 448cfc6e1fa..b0051b02d91 100644 --- a/sql/handler.cc +++ b/sql/handler.cc @@ -428,6 +428,7 @@ static int ha_init_errors(void) SETMSG(HA_ERR_NO_CONNECTION, "Could not connect to storage engine"); SETMSG(HA_ERR_TABLE_DEF_CHANGED, ER(ER_TABLE_DEF_CHANGED)); SETMSG(HA_ERR_TABLE_NEEDS_UPGRADE, ER(ER_TABLE_NEEDS_UPGRADE)); + SETMSG(HA_ERR_TABLE_READONLY, ER(ER_OPEN_AS_READONLY)); /* Register the error messages for use with my_error(). */ return my_error_register(errmsgs, HA_ERR_FIRST, HA_ERR_LAST); @@ -1860,6 +1861,9 @@ void handler::print_error(int error, myf errflag) case HA_ERR_TABLE_NEEDS_UPGRADE: textno=ER_TABLE_NEEDS_UPGRADE; break; + case HA_ERR_TABLE_READONLY: + textno= ER_OPEN_AS_READONLY; + break; default: { /* The error was "unknown" to this function.