#At file:///home/ram/mysql/mysql-6.0-bugteam/
2767 Ramil Kalimullin 2008-08-15
Fix for bug #32426: "FEDERATED query returns corrupt results for
ORDER BY on a TEXT field"
Problem: storing a reference to a row which is used in fielsort
we miss BLOB/TEXT data.
Fix: store a reference as current result set address and data
cursor position.
modified:
mysql-test/r/federated.result
mysql-test/t/federated.test
storage/federated/ha_federated.cc
storage/federated/ha_federated.h
per-file messages:
storage/federated/ha_federated.cc
Fix for bug #32426: "FEDERATED query returns corrupt results for
ORDER BY on a TEXT field"
- place all used during a query execution result sets in the "results"
dynamic array, don't free them during query execution if position()
was called.
- use as a reference <result set address, data cursor position>.
- free result sets used at once at the very end of a query execution,
see ::reset().
- code cleanup.
=== modified file 'mysql-test/r/federated.result'
--- a/mysql-test/r/federated.result 2008-05-01 10:31:23 +0000
+++ b/mysql-test/r/federated.result 2008-08-15 12:18:06 +0000
@@ -2121,6 +2121,27 @@ DROP TABLE t1;
create server 's1' foreign data wrapper 'mysql' options (port 3306);
drop server 's1';
End of 5.1 tests
+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@stripped: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 6.0 tests
DROP TABLE IF EXISTS federated.t1;
DROP DATABASE IF EXISTS federated;
DROP TABLE IF EXISTS federated.t1;
=== modified file 'mysql-test/t/federated.test'
--- a/mysql-test/t/federated.test 2008-05-01 10:31:23 +0000
+++ b/mysql-test/t/federated.test 2008-08-15 12:18:06 +0000
@@ -1864,6 +1864,28 @@ create server 's1' foreign data wrapper
drop server 's1';
--echo End of 5.1 tests
+
+
+#
+# Bug #32426: FEDERATED query returns corrupt results for ORDER BY on a TEXT
+#
+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@stripped:$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;
+
+--echo End of 6.0 tests
+
source include/federated_cleanup.inc;
connection default;
=== modified file 'storage/federated/ha_federated.cc'
--- a/storage/federated/ha_federated.cc 2008-07-18 12:23:25 +0000
+++ b/storage/federated/ha_federated.cc 2008-08-15 12:18:06 +0000
@@ -1618,11 +1618,10 @@ int ha_federated::open(const char *name,
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);
@@ -1642,21 +1641,17 @@ int ha_federated::open(const char *name,
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));
}
/*
@@ -2323,8 +2318,7 @@ int ha_federated::index_read(uchar *buf,
{
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));
@@ -2356,7 +2350,8 @@ int ha_federated::index_read_idx(uchar *
&mysql_result)))
DBUG_RETURN(retval);
mysql_free_result(mysql_result);
- DBUG_RETURN(retval);
+ results.elements--;
+ DBUG_RETURN(0);
}
@@ -2412,18 +2407,20 @@ int ha_federated::index_read_idx_with_re
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;
@@ -2483,12 +2480,6 @@ int ha_federated::read_range_first(const
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;
@@ -2496,14 +2487,13 @@ int ha_federated::read_range_first(const
}
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;
@@ -2513,10 +2503,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]));
}
@@ -2582,23 +2570,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());
}
@@ -2612,11 +2588,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);
}
@@ -2676,6 +2648,9 @@ int ha_federated::read_next(uchar *buf,
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)))
@@ -2688,24 +2663,38 @@ int ha_federated::read_next(uchar *buf,
}
-/*
- 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;
}
@@ -2721,23 +2710,19 @@ void ha_federated::position(const uchar
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));
}
@@ -2942,6 +2927,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;
}
@@ -3205,6 +3200,45 @@ bool ha_federated::get_error_message(int
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;
=== modified file 'storage/federated/ha_federated.h'
--- a/storage/federated/ha_federated.h 2008-05-29 15:33:33 +0000
+++ b/storage/federated/ha_federated.h 2008-08-15 12:18:06 +0000
@@ -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();
| Thread |
|---|
| • bzr commit into mysql-6.0-bugteam branch (ramil:2767) Bug#32426 | Ramil Kalimullin | 15 Aug |
