cleanup: query rewrites for Item_param and Item_splocal

Fix query rewrites in PS code - it was memcpy-ing the same query tail
many times. Instead use the same logic as in SP code, copy query pieces
into the destination buffer.

Extract this logic into a separate class Rewritable_query_parameter
with Item_param and Item_splocal inheriting from it.

Create a helper class Copy_query_with_rewrite that incapsulates
the query rewriting logic, use it in SP and PS.

sql/item.cc

@@ -1579,9 +1579,8 @@ Item_splocal::Item_splocal(const LEX_STRING &sp_var_name,
                            enum_field_types sp_var_type,
                            uint pos_in_q, uint len_in_q)
   :Item_sp_variable(sp_var_name.str, sp_var_name.length),
-   m_var_idx(sp_var_idx),
-   limit_clause_param(FALSE),
-   pos_in_query(pos_in_q), len_in_query(len_in_q)
+   Rewritable_query_parameter(pos_in_q, len_in_q),
+   m_var_idx(sp_var_idx)
 {
   maybe_null= TRUE;
 
@@ -3230,14 +3229,13 @@ default_set_param_func(Item_param *param,
 
 
 Item_param::Item_param(uint pos_in_query_arg) :
+  Rewritable_query_parameter(pos_in_query_arg, 1),
   state(NO_VALUE), inout(IN_PARAM),
   item_result_type(STRING_RESULT),
   /* Don't pretend to be a literal unless value for this item is set. */
   item_type(PARAM_ITEM),
   param_type(MYSQL_TYPE_VARCHAR),
-  pos_in_query(pos_in_query_arg),
   set_param_func(default_set_param_func),
-  limit_clause_param(FALSE),
   m_out_param_info(NULL)
 {
   name= (char*) "?";
@@ -4102,6 +4100,13 @@ void Item_param::make_field(Send_field *field)
   field->type= m_out_param_info->type;
 }
 
+bool Item_param::append_for_log(THD *thd, String *str)
+{
+  StringBuffer<STRING_BUFFER_USUAL_SIZE> buf;
+  const String *val= query_val_str(thd, &buf);
+  return str->append(*val);
+}
+
 /****************************************************************************
   Item_copy
 ****************************************************************************/

sql/item.h

@@ -528,6 +528,70 @@ class Settable_routine_parameter
   { return NULL; }
 };
 
+/**
+  This is used for items in the query that needs to be rewritten
+  before binlogging
+
+  At the moment this applies to Item_param and Item_splocal
+*/
+class Rewritable_query_parameter
+{
+  public:
+  /*
+    Offset inside the query text.
+    Value of 0 means that this object doesn't have to be replaced
+    (for example SP variables in control statements)
+  */
+  uint pos_in_query;
+
+  /*
+    Byte length of parameter name in the statement.  This is not
+    Item::name_length because name_length contains byte length of UTF8-encoded
+    name, but the query string is in the client charset.
+  */
+  uint len_in_query;
+
+  bool limit_clause_param;
+
+  Rewritable_query_parameter(uint pos_in_q= 0, uint len_in_q= 0)
+    : pos_in_query(pos_in_q), len_in_query(len_in_q),
+      limit_clause_param(false)
+  { }
+
+  virtual ~Rewritable_query_parameter() { }
+
+  virtual bool append_for_log(THD *thd, String *str) = 0;
+};
+
+class Copy_query_with_rewrite
+{
+  THD *thd;
+  const char *src;
+  size_t src_len, from;
+  String *dst;
+
+  bool copy_up_to(size_t bytes)
+  {
+    DBUG_ASSERT(bytes >= from);
+    return dst->append(src + from, bytes - from);
+  }
+
+public:
+
+  Copy_query_with_rewrite(THD *t, const char *s, size_t l, String *d)
+    :thd(t), src(s), src_len(l), from(0), dst(d) { }
+
+  bool append(Rewritable_query_parameter *p)
+  {
+    if (copy_up_to(p->pos_in_query) || p->append_for_log(thd, dst))
+      return true;
+    from= p->pos_in_query + p->len_in_query;
+    return false;
+  }
+
+  bool finalize()
+  { return copy_up_to(src_len); }
+};
 
 struct st_dyncall_create_def
 {
@@ -1433,6 +1497,8 @@ class Item {
   }
 
   virtual Item_splocal *get_item_splocal() { return 0; }
+  virtual Rewritable_query_parameter *get_rewritable_query_parameter()
+  { return 0; }
 
   /*
     Return Settable_routine_parameter interface of the Item.  Return 0
@@ -1691,7 +1757,8 @@ inline bool Item_sp_variable::send(Protocol *protocol, String *str)
 *****************************************************************************/
 
 class Item_splocal :public Item_sp_variable,
-                    private Settable_routine_parameter
+                    private Settable_routine_parameter,
+                    public Rewritable_query_parameter
 {
   uint m_var_idx;
 
@@ -1699,33 +1766,6 @@ class Item_splocal :public Item_sp_variable,
   Item_result m_result_type;
   enum_field_types m_field_type;
 public:
-  /*
-    If this variable is a parameter in LIMIT clause.
-    Used only during NAME_CONST substitution, to not append
-    NAME_CONST to the resulting query and thus not break
-    the slave.
-  */
-  bool limit_clause_param;
-  /* 
-    Position of this reference to SP variable in the statement (the
-    statement itself is in sp_instr_stmt::m_query).
-    This is valid only for references to SP variables in statements,
-    excluding DECLARE CURSOR statement. It is used to replace references to SP
-    variables with NAME_CONST calls when putting statements into the binary
-    log.
-    Value of 0 means that this object doesn't corresponding to reference to
-    SP variable in query text.
-  */
-  uint pos_in_query;
-  /*
-    Byte length of SP variable name in the statement (see pos_in_query).
-    The value of this field may differ from the name_length value because
-    name_length contains byte length of UTF8-encoded item name, but
-    the query string (see sp_instr_stmt::m_query) is currently stored with
-    a charset from the SET NAMES statement.
-  */
-  uint len_in_query;
-
   Item_splocal(const LEX_STRING &sp_var_name, uint sp_var_idx,
                enum_field_types sp_var_type,
                uint pos_in_q= 0, uint len_in_q= 0);
@@ -1751,10 +1791,13 @@ class Item_splocal :public Item_sp_variable,
 public:
   Item_splocal *get_item_splocal() { return this; }
 
+  Rewritable_query_parameter *get_rewritable_query_parameter()
+  { return this; }
+
   Settable_routine_parameter *get_settable_routine_parameter()
-  {
-    return this;
-  }
+  { return this; }
+
+  bool append_for_log(THD *thd, String *str);
 };
 
 /*****************************************************************************
@@ -2229,7 +2272,8 @@ class Item_null_result :public Item_null
 /* Item represents one placeholder ('?') of prepared statement */
 
 class Item_param :public Item,
-                  private Settable_routine_parameter
+                  private Settable_routine_parameter,
+                  public Rewritable_query_parameter
 {
   char cnvbuf[MAX_FIELD_WIDTH];
   String cnvstr;
@@ -2294,11 +2338,6 @@ class Item_param :public Item,
     supply for this placeholder in mysql_stmt_execute.
   */
   enum enum_field_types param_type;
-  /*
-    Offset of placeholder inside statement text. Used to create
-    no-placeholders version of this statement for the binary log.
-  */
-  uint pos_in_query;
 
   Item_param(uint pos_in_query_arg);
 
@@ -2364,14 +2403,14 @@ class Item_param :public Item,
     Otherwise return FALSE.
   */
   bool eq(const Item *item, bool binary_cmp) const;
-  /** Item is a argument to a limit clause. */
-  bool limit_clause_param;
   void set_param_type_and_swap_value(Item_param *from);
 
+  Rewritable_query_parameter *get_rewritable_query_parameter()
+  { return this; }
   Settable_routine_parameter *get_settable_routine_parameter()
-  {
-    return this;
-  }
+  { return this; }
+
+  bool append_for_log(THD *thd, String *str);
 
 private:
   virtual bool set_value(THD *thd, sp_rcontext *ctx, Item **it);

sql/sp_head.cc

@@ -149,13 +149,9 @@ sp_get_item_value(THD *thd, Item *item, String *str)
         return NULL;
 
       {
-        char buf_holder[STRING_BUFFER_USUAL_SIZE];
-        String buf(buf_holder, sizeof(buf_holder), result->charset());
+        StringBuffer<STRING_BUFFER_USUAL_SIZE> buf(result->charset());
         CHARSET_INFO *cs= thd->variables.character_set_client;
 
-        /* We must reset length of the buffer, because of String specificity. */
-        buf.length(0);
-
         buf.append('_');
         buf.append(result->charset()->csname);
         if (cs->escape_with_backslash_is_dangerous)
@@ -178,6 +174,28 @@ sp_get_item_value(THD *thd, Item *item, String *str)
 }
 
 
+bool Item_splocal::append_for_log(THD *thd, String *str)
+{
+  if (fix_fields(thd, NULL))
+    return true;
+
+  if (limit_clause_param)
+    return str->append_ulonglong(val_uint());
+
+  if (str->append(STRING_WITH_LEN(" NAME_CONST('")) ||
+      str->append(&m_name) ||
+      str->append(STRING_WITH_LEN("',")))
+    return true;
+
+  StringBuffer<STRING_BUFFER_USUAL_SIZE> str_value_holder(&my_charset_latin1);
+  String *str_value= sp_get_item_value(thd, this_item(), &str_value_holder);
+  if (str_value)
+    return str->append(*str_value) || str->append(')');
+  else
+    return str->append(STRING_WITH_LEN("NULL)"));
+}
+
+
 /**
    Returns a combination of:
    - sp_head::MULTI_RESULTS: added if the 'cmd' is a command that might
@@ -979,19 +997,16 @@ subst_spvars(THD *thd, sp_instr *instr, LEX_STRING *query_str)
   DBUG_ENTER("subst_spvars");
 
   Dynamic_array<Item_splocal*> sp_vars_uses;
-  char *pbuf, *cur, buffer[512];
-  String qbuf(buffer, sizeof(buffer), &my_charset_bin);
-  int prev_pos, res, buf_len;
+  char *pbuf;
+  StringBuffer<512> qbuf;
+  Copy_query_with_rewrite acc(thd, query_str->str, query_str->length, &qbuf);
 
   /* Find all instances of Item_splocal used in this statement */
   for (Item *item= instr->free_list; item; item= item->next)
   {
     Item_splocal *item_spl= item->get_item_splocal();
-    if (item_spl)
-    {
-      if (item_spl->pos_in_query)
-        sp_vars_uses.append(item_spl);
-    }
+    if (item_spl && item_spl->pos_in_query)
+      sp_vars_uses.append(item_spl);
   }
   if (!sp_vars_uses.elements())
     DBUG_RETURN(FALSE);
@@ -999,64 +1014,15 @@ subst_spvars(THD *thd, sp_instr *instr, LEX_STRING *query_str)
   /* Sort SP var refs by their occurences in the query */
   sp_vars_uses.sort(cmp_splocal_locations);
 
-  /*
-    Construct a statement string where SP local var refs are replaced
-    with "NAME_CONST(name, value)"
-  */
-  qbuf.length(0);
-  cur= query_str->str;
-  prev_pos= res= 0;
-  thd->query_name_consts= 0;
+  thd->query_name_consts= sp_vars_uses.elements();
 
   for (Item_splocal **splocal= sp_vars_uses.front(); 
        splocal <= sp_vars_uses.back(); splocal++)
   {
-    Item *val;
-
-    char str_buffer[STRING_BUFFER_USUAL_SIZE];
-    String str_value_holder(str_buffer, sizeof(str_buffer),
-                            &my_charset_latin1);
-    String *str_value;
-
-    /* append the text between sp ref occurences */
-    res|= qbuf.append(cur + prev_pos, (*splocal)->pos_in_query - prev_pos);
-    prev_pos= (*splocal)->pos_in_query + (*splocal)->len_in_query;
-
-    res|= (*splocal)->fix_fields(thd, (Item **) splocal);
-    if (res)
-      break;
-
-    if ((*splocal)->limit_clause_param)
-    {
-      res|= qbuf.append_ulonglong((*splocal)->val_uint());
-      if (res)
-        break;
-      continue;
-    }
-
-    /* append the spvar substitute */
-    res|= qbuf.append(STRING_WITH_LEN(" NAME_CONST('"));
-    res|= qbuf.append((*splocal)->m_name.str, (*splocal)->m_name.length);
-    res|= qbuf.append(STRING_WITH_LEN("',"));
-
-    if (res)
-      break;
-
-    val= (*splocal)->this_item();
-    DBUG_PRINT("info", ("print 0x%lx", (long) val));
-    str_value= sp_get_item_value(thd, val, &str_value_holder);
-    if (str_value)
-      res|= qbuf.append(*str_value);
-    else
-      res|= qbuf.append(STRING_WITH_LEN("NULL"));
-    res|= qbuf.append(')');
-    if (res)
-      break;
-
-    thd->query_name_consts++;
+    if (acc.append(*splocal))
+      DBUG_RETURN(TRUE);
   }
-  if (res ||
-      qbuf.append(cur + prev_pos, query_str->length - prev_pos))
+  if (acc.finalize())
     DBUG_RETURN(TRUE);
 
   /*
@@ -1071,8 +1037,8 @@ subst_spvars(THD *thd, sp_instr *instr, LEX_STRING *query_str)
             <db_name>     Name of current database
             <flags>       Flags struct
   */
-  buf_len= (qbuf.length() + 1 + QUERY_CACHE_DB_LENGTH_SIZE + thd->db_length +
-            QUERY_CACHE_FLAGS_SIZE + 1);
+  int buf_len= (qbuf.length() + 1 + QUERY_CACHE_DB_LENGTH_SIZE +
+                thd->db_length + QUERY_CACHE_FLAGS_SIZE + 1);
   if ((pbuf= (char *) alloc_root(thd->mem_root, buf_len)))
   {
     char *ptr= pbuf + qbuf.length();

sql/sql_prepare.cc

@@ -873,14 +873,9 @@ static bool insert_params_with_log(Prepared_statement *stmt, uchar *null_array,
   THD  *thd= stmt->thd;
   Item_param **begin= stmt->param_array;
   Item_param **end= begin + stmt->param_count;
-  uint32 length= 0;
-  String str;
-  const String *res;
+  Copy_query_with_rewrite acc(thd, stmt->query(), stmt->query_length(), query);
   DBUG_ENTER("insert_params_with_log");
 
-  if (query->copy(stmt->query(), stmt->query_length(), default_charset_info))
-    DBUG_RETURN(1);
-
   for (Item_param **it= begin; it < end; ++it)
   {
     Item_param *param= *it;
@@ -913,15 +908,16 @@ static bool insert_params_with_log(Prepared_statement *stmt, uchar *null_array,
     */
     else if (! is_param_long_data_type(param))
       DBUG_RETURN(1);
-    res= param->query_val_str(thd, &str);
-    if (param->convert_str_value(thd))
-      DBUG_RETURN(1);                           /* out of memory */
 
-    if (query->replace(param->pos_in_query+length, 1, *res))
+    if (acc.append(param))
       DBUG_RETURN(1);
 
-    length+= res->length()-1;
+    if (param->convert_str_value(thd))
+      DBUG_RETURN(1);                           /* out of memory */
   }
+  if (acc.finalize())
+    DBUG_RETURN(1);
+
   DBUG_RETURN(0);
 }
 
@@ -1050,23 +1046,15 @@ static bool emb_insert_params(Prepared_statement *stmt, String *expanded_query)
 }
 
 
-static bool emb_insert_params_with_log(Prepared_statement *stmt,
-                                       String *query)
+static bool emb_insert_params_with_log(Prepared_statement *stmt, String *query)
 {
   THD *thd= stmt->thd;
   Item_param **it= stmt->param_array;
   Item_param **end= it + stmt->param_count;
   MYSQL_BIND *client_param= thd->client_params;
-
-  String str;
-  const String *res;
-  uint32 length= 0;
-
+  Copy_query_with_rewrite acc(thd, stmt->query(), stmt->query_length(), query);
   DBUG_ENTER("emb_insert_params_with_log");
 
-  if (query->copy(stmt->query(), stmt->query_length(), default_charset_info))
-    DBUG_RETURN(1);
-
   for (; it < end; ++it, ++client_param)
   {
     Item_param *param= *it;
@@ -1087,15 +1075,15 @@ static bool emb_insert_params_with_log(Prepared_statement *stmt,
           DBUG_RETURN(1);
       }
     }
-    res= param->query_val_str(thd, &str);
-    if (param->convert_str_value(thd))
-      DBUG_RETURN(1);                           /* out of memory */
-
-    if (query->replace(param->pos_in_query+length, 1, *res))
+    if (acc.append(param))
       DBUG_RETURN(1);
 
-    length+= res->length()-1;
+    if (param->convert_str_value(thd))
+      DBUG_RETURN(1);                           /* out of memory */
   }
+  if (acc.finalize())
+    DBUG_RETURN(1);
+
   DBUG_RETURN(0);
 }
 
@@ -1232,16 +1220,11 @@ static bool insert_params_from_vars_with_log(Prepared_statement *stmt,
   user_var_entry *entry;
   LEX_STRING *varname;
   List_iterator<LEX_STRING> var_it(varnames);
-  String buf;
-  const String *val;
-  uint32 length= 0;
   THD *thd= stmt->thd;
+  Copy_query_with_rewrite acc(thd, stmt->query(), stmt->query_length(), query);
 
   DBUG_ENTER("insert_params_from_vars_with_log");
 
-  if (query->copy(stmt->query(), stmt->query_length(), default_charset_info))
-    DBUG_RETURN(1);
-
   for (Item_param **it= begin; it < end; ++it)
   {
     Item_param *param= *it;
@@ -1257,15 +1240,16 @@ static bool insert_params_from_vars_with_log(Prepared_statement *stmt,
     setup_one_conversion_function(thd, param, param->param_type);
     if (param->set_from_user_var(thd, entry))
       DBUG_RETURN(1);
-    val= param->query_val_str(thd, &buf);
 
-    if (param->convert_str_value(thd))
-      DBUG_RETURN(1);                           /* out of memory */
+    if (acc.append(param))
+      DBUG_RETURN(1);
 
-    if (query->replace(param->pos_in_query+length, 1, *val))
+    if (param->convert_str_value(thd))
       DBUG_RETURN(1);
-    length+= val->length()-1;
   }
+  if (acc.finalize())
+    DBUG_RETURN(1);
+
   DBUG_RETURN(0);
 }
 
@@ -3244,9 +3228,15 @@ void Prepared_statement::setup_set_params()
     Decide if we have to expand the query (because we must write it to logs or
     because we want to look it up in the query cache) or not.
   */
-  if ((mysql_bin_log.is_open() && is_update_query(lex->sql_command)) ||
-      opt_log || thd->variables.sql_log_slow ||
-      query_cache_is_cacheable_query(lex))
+  bool replace_params_with_values= false;
+  // binlog
+  replace_params_with_values|= mysql_bin_log.is_open() && is_update_query(lex->sql_command);
+  // general or slow log
+  replace_params_with_values|= opt_log || thd->variables.sql_log_slow;
+  // query cache
+  replace_params_with_values|= query_cache_is_cacheable_query(lex);
+
+  if (replace_params_with_values)
   {
     set_params_from_vars= insert_params_from_vars_with_log;
 #ifndef EMBEDDED_LIBRARY