Commit abbf7ad0 authored by unknown's avatar unknown
Browse files

Bug #17608: String literals lost during INSERT query on FEDERATED table 

  The Federated storage engine used Field methods that had arbitrary limits on
  the amount of data they could process, which caused problems with data
  over that limit (4K). By removing those Field methods and just using
  features of the String class, we can avoid this problem.


mysql-test/r/federated.result:
  Add new results
mysql-test/t/federated.test:
  Add new regression test
sql/field.cc:
  Remove unnecessary methods
sql/field.h:
  Remove unnecessary methods
sql/ha_federated.cc:
  Remove use of quote_data, use String::print() to get escaping of strings,
  and don't bother with needs_quotes, just always quote values.
parent a7dddd3b

mysql-test/r/federated.result

+13 −0
Original line number Diff line number Diff line
@@ -1733,6 +1733,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;

mysql-test/t/federated.test

+18 −0
Original line number Diff line number Diff line
@@ -1425,4 +1425,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;

sql/field.cc

+0 −98
Original line number Diff line number Diff line
@@ -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

****************************************************************************/

sql/field.h

+0 −2
Original line number Diff line number Diff line
@@ -251,8 +251,6 @@ class Field 
    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)

  {

sql/ha_federated.cc

+21 −33
Original line number Diff line number Diff line
@@ -124,11 +124,6 @@ 


    ha_federated::write_row



    <for every field/column>

    Field::quote_data

    Field::quote_data

    </for every field/column>



    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



    <quote 3 cols, new and old data>

    Field::quote_data

    Field::quote_data

    Field::quote_data

    Field::quote_data

    Field::quote_data

    Field::quote_data

    </quote 3 cols, new and old 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)
@@ -1641,20 +1626,21 @@ int ha_federated::write_row(byte *buf) 
      */

      has_fields= TRUE;



      /* append the field name */

      insert_string.append((*field)->field_name);



      /* append the field value */

      if ((*field)->is_null())

        insert_field_value_string.append(FEDERATED_NULL);

        values_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);

        values_string.append('\'');

        insert_field_value_string.print(&values_string);

        values_string.append('\'');



      /* append the value */

      values_string.append(insert_field_value_string);

        insert_field_value_string.length(0);

      }



      /* append commas between both fields and fieldnames */

      /*
@@ -1861,8 +1847,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 +1860,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 +1932,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