diff options
| -rw-r--r-- | mysql-test/suite/federated/federated.result | 23 | ||||
| -rw-r--r-- | mysql-test/suite/federated/federated.test | 22 | ||||
| -rw-r--r-- | storage/federated/ha_federated.cc | 202 | ||||
| -rw-r--r-- | storage/federated/ha_federated.h | 9 |
4 files changed, 172 insertions, 84 deletions
diff --git a/mysql-test/suite/federated/federated.result b/mysql-test/suite/federated/federated.result index 57f665995a1..db4ffc38213 100644 --- a/mysql-test/suite/federated/federated.result +++ b/mysql-test/suite/federated/federated.result @@ -2153,6 +2153,29 @@ DROP TABLE t1; End of 5.0 tests create server 's1' foreign data wrapper 'mysql' options (port 3306); drop server 's1'; +# +# Bug #32426: FEDERATED query returns corrupt results for ORDER BY on a TEXT +# +CREATE TABLE federated.t1(a TEXT); +INSERT INTO federated.t1 VALUES('abc'), ('gh'), ('f'), ('ijk'), ('de'); +CREATE TABLE federated.t1(a TEXT) ENGINE=FEDERATED +CONNECTION='mysql://root@127.0.0.1:SLAVE_PORT/federated/t1'; +SELECT * FROM federated.t1 ORDER BY A; +a +abc +de +f +gh +ijk +SELECT * FROM federated.t1 ORDER BY A DESC; +a +ijk +gh +f +de +abc +DROP TABLE federated.t1; +DROP TABLE federated.t1; End of 5.1 tests SET @@GLOBAL.CONCURRENT_INSERT= @OLD_MASTER_CONCURRENT_INSERT; SET @@GLOBAL.CONCURRENT_INSERT= @OLD_SLAVE_CONCURRENT_INSERT; diff --git a/mysql-test/suite/federated/federated.test b/mysql-test/suite/federated/federated.test index 870bbcb8f71..a1d86462c11 100644 --- a/mysql-test/suite/federated/federated.test +++ b/mysql-test/suite/federated/federated.test @@ -1971,6 +1971,28 @@ connection default; create server 's1' foreign data wrapper 'mysql' options (port 3306); drop server 's1'; + +--echo # +--echo # Bug #32426: FEDERATED query returns corrupt results for ORDER BY on a TEXT +--echo # +connection slave; +CREATE TABLE federated.t1(a TEXT); +INSERT INTO federated.t1 VALUES('abc'), ('gh'), ('f'), ('ijk'), ('de'); + +connection master; +--replace_result $SLAVE_MYPORT SLAVE_PORT +eval CREATE TABLE federated.t1(a TEXT) ENGINE=FEDERATED + CONNECTION='mysql://root@127.0.0.1:$SLAVE_MYPORT/federated/t1'; +SELECT * FROM federated.t1 ORDER BY A; +SELECT * FROM federated.t1 ORDER BY A DESC; +DROP TABLE federated.t1; + +connection slave; +DROP TABLE federated.t1; + +connection default; + + --echo End of 5.1 tests SET @@GLOBAL.CONCURRENT_INSERT= @OLD_MASTER_CONCURRENT_INSERT; connection slave; diff --git a/storage/federated/ha_federated.cc b/storage/federated/ha_federated.cc index 8deb8693960..f8dde34b82b 100644 --- a/storage/federated/ha_federated.cc +++ b/storage/federated/ha_federated.cc @@ -1621,11 +1621,10 @@ int ha_federated::open(const char *name, int mode, uint test_if_locked) DBUG_ASSERT(mysql == NULL); - ref_length= (table->s->primary_key != MAX_KEY ? - table->key_info[table->s->primary_key].key_length : - table->s->reclength); + ref_length= sizeof(MYSQL_RES *) + sizeof(MYSQL_ROW_OFFSET); DBUG_PRINT("info", ("ref_length: %u", ref_length)); + my_init_dynamic_array(&results, sizeof(MYSQL_RES *), 4, 4); reset(); DBUG_RETURN(0); @@ -1645,21 +1644,17 @@ int ha_federated::open(const char *name, int mode, uint test_if_locked) int ha_federated::close(void) { - int retval; DBUG_ENTER("ha_federated::close"); - /* free the result set */ - if (stored_result) - { - mysql_free_result(stored_result); - stored_result= 0; - } + free_result(); + + delete_dynamic(&results); + /* Disconnect from mysql */ mysql_close(mysql); mysql= NULL; - retval= free_share(share); - DBUG_RETURN(retval); + DBUG_RETURN(free_share(share)); } /* @@ -2326,8 +2321,7 @@ int ha_federated::index_read(uchar *buf, const uchar *key, { DBUG_ENTER("ha_federated::index_read"); - if (stored_result) - mysql_free_result(stored_result); + free_result(); DBUG_RETURN(index_read_idx_with_result_set(buf, active_index, key, key_len, find_flag, &stored_result)); @@ -2359,7 +2353,8 @@ int ha_federated::index_read_idx(uchar *buf, uint index, const uchar *key, &mysql_result))) DBUG_RETURN(retval); mysql_free_result(mysql_result); - DBUG_RETURN(retval); + results.elements--; + DBUG_RETURN(0); } @@ -2415,18 +2410,20 @@ int ha_federated::index_read_idx_with_result_set(uchar *buf, uint index, retval= ER_QUERY_ON_FOREIGN_DATA_SOURCE; goto error; } - if (!(*result= mysql_store_result(mysql))) + if (!(*result= store_result(mysql))) { retval= HA_ERR_END_OF_FILE; goto error; } - if (!(retval= read_next(buf, *result))) + if ((retval= read_next(buf, *result))) + { + mysql_free_result(*result); + results.elements--; + *result= 0; + table->status= STATUS_NOT_FOUND; DBUG_RETURN(retval); - - mysql_free_result(*result); - *result= 0; - table->status= STATUS_NOT_FOUND; - DBUG_RETURN(retval); + } + DBUG_RETURN(0); error: table->status= STATUS_NOT_FOUND; @@ -2486,12 +2483,6 @@ int ha_federated::read_range_first(const key_range *start_key, create_where_from_key(&sql_query, &table->key_info[active_index], start_key, end_key, 0, eq_range_arg); - - if (stored_result) - { - mysql_free_result(stored_result); - stored_result= 0; - } if (real_query(sql_query.ptr(), sql_query.length())) { retval= ER_QUERY_ON_FOREIGN_DATA_SOURCE; @@ -2499,14 +2490,13 @@ int ha_federated::read_range_first(const key_range *start_key, } sql_query.length(0); - if (!(stored_result= mysql_store_result(mysql))) + if (!(stored_result= store_result(mysql))) { retval= HA_ERR_END_OF_FILE; goto error; } - - retval= read_next(table->record[0], stored_result); - DBUG_RETURN(retval); + + DBUG_RETURN(read_next(table->record[0], stored_result)); error: table->status= STATUS_NOT_FOUND; @@ -2516,10 +2506,8 @@ error: int ha_federated::read_range_next() { - int retval; DBUG_ENTER("ha_federated::read_range_next"); - retval= rnd_next(table->record[0]); - DBUG_RETURN(retval); + DBUG_RETURN(rnd_next(table->record[0])); } @@ -2585,23 +2573,11 @@ int ha_federated::rnd_init(bool scan) if (scan) { - if (stored_result) - { - mysql_free_result(stored_result); - stored_result= 0; - } - - if (real_query(share->select_query, strlen(share->select_query))) - goto error; - - stored_result= mysql_store_result(mysql); - if (!stored_result) - goto error; + if (real_query(share->select_query, strlen(share->select_query)) || + !(stored_result= store_result(mysql))) + DBUG_RETURN(stash_remote_error()); } DBUG_RETURN(0); - -error: - DBUG_RETURN(stash_remote_error()); } @@ -2615,11 +2591,7 @@ int ha_federated::rnd_end() int ha_federated::index_end(void) { DBUG_ENTER("ha_federated::index_end"); - if (stored_result) - { - mysql_free_result(stored_result); - stored_result= 0; - } + free_result(); active_index= MAX_KEY; DBUG_RETURN(0); } @@ -2679,6 +2651,9 @@ int ha_federated::read_next(uchar *buf, MYSQL_RES *result) DBUG_ENTER("ha_federated::read_next"); table->status= STATUS_NOT_FOUND; // For easier return + + /* Save current data cursor position. */ + current_position= result->data_cursor; /* Fetch a row, insert it back in a row format. */ if (!(row= mysql_fetch_row(result))) @@ -2691,24 +2666,38 @@ int ha_federated::read_next(uchar *buf, MYSQL_RES *result) } -/* - store reference to current row so that we can later find it for - a re-read, update or delete. - - In case of federated, a reference is either a primary key or - the whole record. - - Called from filesort.cc, sql_select.cc, sql_delete.cc and sql_update.cc. +/** + @brief Store a reference to current row. + + @details During a query execution we may have different result sets (RS), + e.g. for different ranges. All the RS's used are stored in + memory and placed in @c results dynamic array. At the end of + execution all stored RS's are freed at once in the + @c ha_federated::reset(). + So, in case of federated, a reference to current row is a + stored result address and current data cursor position. + As we keep all RS in memory during a query execution, + we can get any record using the reference any time until + @c ha_federated::reset() is called. + TODO: we don't have to store all RS's rows but only those + we call @c ha_federated::position() for, so we can free memory + where we store other rows in the @c ha_federated::index_end(). + + @param[in] record record data (unused) */ -void ha_federated::position(const uchar *record) +void ha_federated::position(const uchar *record __attribute__ ((unused))) { DBUG_ENTER("ha_federated::position"); - if (table->s->primary_key != MAX_KEY) - key_copy(ref, (uchar *)record, table->key_info + table->s->primary_key, - ref_length); - else - memcpy(ref, record, ref_length); + + DBUG_ASSERT(stored_result); + + position_called= TRUE; + /* Store result set address. */ + memcpy_fixed(ref, &stored_result, sizeof(MYSQL_RES *)); + /* Store data cursor position. */ + memcpy_fixed(ref + sizeof(MYSQL_RES *), ¤t_position, + sizeof(MYSQL_ROW_OFFSET)); DBUG_VOID_RETURN; } @@ -2724,23 +2713,19 @@ void ha_federated::position(const uchar *record) int ha_federated::rnd_pos(uchar *buf, uchar *pos) { - int result; + MYSQL_RES *result; DBUG_ENTER("ha_federated::rnd_pos"); + ha_statistic_increment(&SSV::ha_read_rnd_count); - if (table->s->primary_key != MAX_KEY) - { - /* We have a primary key, so use index_read_idx to find row */ - result= index_read_idx(buf, table->s->primary_key, pos, - ref_length, HA_READ_KEY_EXACT); - } - else - { - /* otherwise, get the old record ref as obtained in ::position */ - memcpy(buf, pos, ref_length); - result= 0; - } - table->status= result ? STATUS_NOT_FOUND : 0; - DBUG_RETURN(result); + + /* Get stored result set. */ + memcpy_fixed(&result, pos, sizeof(MYSQL_RES *)); + DBUG_ASSERT(result); + /* Set data cursor position. */ + memcpy_fixed(&result->data_cursor, pos + sizeof(MYSQL_RES *), + sizeof(MYSQL_ROW_OFFSET)); + /* Read a row. */ + DBUG_RETURN(read_next(buf, result)); } @@ -2943,6 +2928,16 @@ int ha_federated::reset(void) insert_dup_update= FALSE; ignore_duplicates= FALSE; replace_duplicates= FALSE; + + /* Free stored result sets. */ + for (uint i= 0; i < results.elements; i++) + { + MYSQL_RES *result; + get_dynamic(&results, (uchar *) &result, i); + mysql_free_result(result); + } + reset_dynamic(&results); + return 0; } @@ -3206,6 +3201,45 @@ bool ha_federated::get_error_message(int error, String* buf) DBUG_RETURN(FALSE); } + +/** + @brief Store a result set. + + @details Call @c mysql_store_result() to save a result set then + append it to the stored results array. + + @param[in] mysql MySLQ connection structure. + + @return Stored result set (MYSQL_RES object). +*/ + +MYSQL_RES *ha_federated::store_result(MYSQL *mysql) +{ + MYSQL_RES *result= mysql_store_result(mysql); + DBUG_ENTER("ha_federated::store_result"); + if (result) + { + (void) insert_dynamic(&results, (uchar*) &result); + } + position_called= FALSE; + DBUG_RETURN(result); +} + + +void ha_federated::free_result() +{ + DBUG_ENTER("ha_federated::free_result"); + if (stored_result && !position_called) + { + mysql_free_result(stored_result); + stored_result= 0; + if (results.elements > 0) + results.elements--; + } + DBUG_VOID_RETURN; +} + + int ha_federated::external_lock(THD *thd, int lock_type) { int error= 0; diff --git a/storage/federated/ha_federated.h b/storage/federated/ha_federated.h index 2f1c62decca..2e0a19e592f 100644 --- a/storage/federated/ha_federated.h +++ b/storage/federated/ha_federated.h @@ -84,6 +84,11 @@ class ha_federated: public handler FEDERATED_SHARE *share; /* Shared lock info */ MYSQL *mysql; /* MySQL connection */ MYSQL_RES *stored_result; + /** + Array of all stored results we get during a query execution. + */ + DYNAMIC_ARRAY results; + bool position_called; uint fetch_num; // stores the fetch num MYSQL_ROW_OFFSET current_position; // Current position used by ::position() int remote_error_number; @@ -251,6 +256,10 @@ public: THR_LOCK_DATA **store_lock(THD *thd, THR_LOCK_DATA **to, enum thr_lock_type lock_type); //required bool get_error_message(int error, String *buf); + + MYSQL_RES *store_result(MYSQL *mysql); + void free_result(); + int external_lock(THD *thd, int lock_type); int connection_commit(); int connection_rollback(); |