← Back to team overview

maria-developers team mailing list archive

  1. maria-developers team
  2. Mailing list archive
  3. Message #00973

Re: Rev 2731: Fix for in file:///Users/hakan/work/monty_program/maria-debug/

  Thread Previous • Date Previous • • Thread Next 
Hi!
This patch is big progress over previous one. Thank you! But still there are things which should be fixed. See my comments in the code.
What I have not found in the patch is correct description of the option (in --help text) as we discussed in on IRC. 

15 сент. 2009, в 08:03, Hakan Kuecuekyilmaz написал(а):

[skip]

=== modified file 'client/mysqlslap.c'
--- a/client/mysqlslap.c	2009-04-25 10:05:32 +0000
+++ b/client/mysqlslap.c	2009-09-15 05:00:24 +0000
@@ -423,6 +423,10 @@
  stats *sptr;
  conclusions conclusion;
  unsigned long long client_limit;
+  DYNAMIC_STRING table_string;
+  statement *ddl_with_engine;
+
+  init_dynamic_string(&table_string, "", 1024, 1024);


In general any non-literal constant like above (1024) is bad idea.

But you do not need this code at all. Engine should be added in:
if (engine_stmt && engine_stmt->option && ptr->type == CREATE_TABLE_TYPE) 
...

So the question is why it is not work or why you need this code at all?



  head_sptr= (stats *)my_malloc(sizeof(stats) * iterations,
                                MYF(MY_ZEROFILL|MY_FAE|MY_WME));
@@ -448,8 +452,39 @@

    /* First we create */
    if (create_statements)
- create_schema(mysql, create_schema_string, create_statements, eptr); 
-
+    {
+      /*
+        In case of user supplied DDL and engine, we add the engine
+        type to the DDL here.
+       */
+      if (create_string && eptr)
+      {
+        dynstr_append(&table_string, create_statements->string);
+        dynstr_append(&table_string, " Engine = ");
+        dynstr_append(&table_string, eptr->string);
+
+        if (eptr->option)
+        {
+          dynstr_append(&table_string, " ");
+          dynstr_append(&table_string, eptr->option);
+        }
+
+        ddl_with_engine= (statement *) my_malloc(sizeof(statement),
+ MYF(MY_ZEROFILL| MY_FAE|MY_WME)); + ddl_with_engine->string= (char *) my_malloc(table_string.length + 1, + MYF(MY_ZEROFILL| MY_FAE|MY_WME)); 
+        ddl_with_engine->length= table_string.length + 1;
+        ddl_with_engine->type= create_statements->type;
+        strmov(ddl_with_engine->string, table_string.str);
+        ddl_with_engine->next= create_statements->next;
+
+        dynstr_free(&table_string);
+
+ create_schema(mysql, create_schema_string, ddl_with_engine, eptr); 
+      }
+      else
+ create_schema(mysql, create_schema_string, create_statements, eptr); 
+    }
    /*
If we generated GUID we need to build a list of them from creation that 
      we can later use.
@@ -773,7 +808,7 @@
static statement *
build_table_string(void)
{
-  char       buf[HUGE_STRING_LENGTH];
+  char       buf[HUGE_STRING_LENGTH]= "";
Why you need this initialization everywhere if you overwrite it late in snprintf & Co ? 


  unsigned int        col_count;
  statement *ptr;
  DYNAMIC_STRING table_string;
@@ -900,7 +935,7 @@
static statement *
build_update_string(void)
{
-  char       buf[HUGE_STRING_LENGTH];
+  char       buf[HUGE_STRING_LENGTH]= "";
  unsigned int        col_count;
  statement *ptr;
  DYNAMIC_STRING update_string;
@@ -958,6 +993,7 @@
    ptr->type= UPDATE_TYPE_REQUIRES_PREFIX ;
  else
    ptr->type= UPDATE_TYPE;
+
  strmov(ptr->string, update_string.str);
  dynstr_free(&update_string);
  DBUG_RETURN(ptr);
@@ -973,8 +1009,8 @@
static statement *
build_insert_string(void)
{
-  char       buf[HUGE_STRING_LENGTH];
-  unsigned int        col_count;
+  char buf[HUGE_STRING_LENGTH]= "";
+  unsigned int col_count;
  statement *ptr;
  DYNAMIC_STRING insert_string;
  DBUG_ENTER("build_insert_string");
@@ -1064,8 +1100,8 @@
static statement *
build_select_string(my_bool key)
{
-  char       buf[HUGE_STRING_LENGTH];
-  unsigned int        col_count;
+  char buf[HUGE_STRING_LENGTH]= "";
+  unsigned int col_count;
  statement *ptr;
  static DYNAMIC_STRING query_string;
  DBUG_ENTER("build_select_string");
@@ -1117,6 +1153,7 @@
    ptr->type= SELECT_TYPE_REQUIRES_PREFIX;
  else
    ptr->type= SELECT_TYPE;
+
  strmov(ptr->string, query_string.str);
  dynstr_free(&query_string);
  DBUG_RETURN(ptr);
@@ -1175,8 +1212,6 @@
      exit(1);
    }

-
-
  if (auto_generate_sql && num_of_query && auto_actual_queries)
  {
      fprintf(stderr,
@@ -1217,6 +1252,7 @@
    num_int_cols= atoi(str->string);
    if (str->option)
      num_int_cols_index= atoi(str->option);
+
    option_cleanup(str);
  }

@@ -1229,6 +1265,7 @@
      num_char_cols_index= atoi(str->option);
    else
      num_char_cols_index= 0;
+
    option_cleanup(str);
  }

@@ -1463,6 +1500,7 @@

  if (tty_password)
    opt_password= get_tty_password(NullS);
+
  DBUG_RETURN(0);
}

@@ -1477,6 +1515,7 @@

  if (verbose >= 3)
    printf("%.*s;\n", len, query);
+
  return mysql_real_query(mysql, query, len);
}

@@ -1556,7 +1595,7 @@
create_schema(MYSQL *mysql, const char *db, statement *stmt,
              option_string *engine_stmt)
{
-  char query[HUGE_STRING_LENGTH];
+  char query[HUGE_STRING_LENGTH]= "";
  statement *ptr;
  statement *after_create;
  int len;
@@ -1592,18 +1631,6 @@
    }
  }

-  if (engine_stmt)
-  {
- len= snprintf(query, HUGE_STRING_LENGTH, "set storage_engine=` %s`", 
-                  engine_stmt->string);
-    if (run_query(mysql, query, len))
-    {
- fprintf(stderr,"%s: Cannot set default engine: %s\n", my_progname, 
-              mysql_error(mysql));
-      exit(1);
-    }
-  }
-
it would better to rollback previous patch or merge this one with previous (we do not need incorrect patches pile up one over another, I mean clear history) 


  count= 0;
  after_create= stmt;

@@ -1615,10 +1642,23 @@
if (engine_stmt && engine_stmt->option && ptr->type == CREATE_TABLE_TYPE) 
    {
-      char buffer[HUGE_STRING_LENGTH];
-
-      snprintf(buffer, HUGE_STRING_LENGTH, "%s %s", ptr->string,
-               engine_stmt->option);
+      char buffer[HUGE_STRING_LENGTH]= "";
+
+      snprintf(buffer, HUGE_STRING_LENGTH, "%s Engine = %s %s",
+ ptr->string, engine_stmt->string, engine_stmt- >option); 
+      if (run_query(mysql, buffer, strlen(buffer)))
+      {
+        fprintf(stderr,"%s: Cannot run query %.*s ERROR : %s\n",
+ my_progname, (uint)ptr->length, ptr->string, mysql_error(mysql)); 
+        exit(1);
+      }
+    }
+ else if (engine_stmt && engine_stmt->string && ptr->type == CREATE_TABLE_TYPE) 
+    {
+      char buffer[HUGE_STRING_LENGTH]= "";
+
+      snprintf(buffer, HUGE_STRING_LENGTH, "%s Engine = %s",
+               ptr->string, engine_stmt->string);
      if (run_query(mysql, buffer, strlen(buffer)))
      {
        fprintf(stderr,"%s: Cannot run query %.*s ERROR : %s\n",
@@ -1650,8 +1690,9 @@
static int
drop_schema(MYSQL *mysql, const char *db)
{
-  char query[HUGE_STRING_LENGTH];
+  char query[HUGE_STRING_LENGTH]= "";
  int len;
+
  DBUG_ENTER("drop_schema");
len= snprintf(query, HUGE_STRING_LENGTH, "DROP SCHEMA IF EXISTS ` %s`", db); 

@@ -1842,7 +1883,7 @@
        int length;
        unsigned int key_val;
        char *key;
-        char buffer[HUGE_STRING_LENGTH];
+        char buffer[HUGE_STRING_LENGTH]= "";

        /*
          This should only happen if some sort of new engine was
@@ -1940,17 +1981,22 @@
  uint count= 0; /* We know that there is always one */

  for (tmp= *sptr= (option_string *)my_malloc(sizeof(option_string),
- MYF(MY_ZEROFILL|MY_FAE| MY_WME)); + MYF(MY_ZEROFILL| MY_FAE|MY_WME)); 
       (retstr= strchr(ptr, delm));
       tmp->next=  (option_string *)my_malloc(sizeof(option_string),
- MYF(MY_ZEROFILL|MY_FAE| MY_WME)), + MYF(MY_ZEROFILL| MY_FAE|MY_WME)), 
       tmp= tmp->next)
  {
-    char buffer[HUGE_STRING_LENGTH];
+    char buffer[HUGE_STRING_LENGTH]= "";
    char *buffer_ptr;

    count++;
    strncpy(buffer, ptr, (size_t)(retstr - ptr));
+    /*
+ Handle --engine=memory:max_row=200 cases, or more general speaking + --engine=<storage_engine>:<options>. -- engine=<storage_engine:option 
+       will be translated to Engine = storage_engine option
+     */
    if ((buffer_ptr= strchr(buffer, ':')))
    {
      char *option_ptr;
@@ -1971,13 +2017,15 @@
      tmp->length= (size_t)(retstr - ptr);
    }

+    /* Skip delimiter delm */
    ptr+= retstr - ptr + 1;
    if (isspace(*ptr))
      ptr++;
+
    count++;
  }

-  if (ptr != origin+length)
+  if (ptr != origin + length)
  {
    char *origin_ptr;

@@ -1986,7 +2034,8 @@
      char *option_ptr;

      tmp->length= (size_t)(origin_ptr - ptr);
-      tmp->string= my_strndup(origin, tmp->length, MYF(MY_FAE));
+      // tmp->string= my_strndup(origin, tmp->length, MYF(MY_FAE));
+      tmp->string= my_strndup(ptr, tmp->length, MYF(MY_FAE));

      option_ptr= (char *)ptr + 1 + tmp->length;

@@ -2036,7 +2085,7 @@
  if (ptr != script+length)
  {
    tmp->string= my_strndup(ptr, (uint)((script + length) - ptr),
-                                       MYF(MY_FAE));
+                            MYF(MY_FAE));
    tmp->length= (size_t)((script + length) - ptr);
    count++;
  }
@@ -2092,8 +2141,9 @@
void
print_conclusions_csv(conclusions *con)
{
-  char buffer[HUGE_STRING_LENGTH];
+  char buffer[HUGE_STRING_LENGTH]= "";
const char *ptr= auto_generate_sql_type ? auto_generate_sql_type : "query"; 
+
  snprintf(buffer, HUGE_STRING_LENGTH,
           "%s,%s,%ld.%03ld,%ld.%03ld,%ld.%03ld,%d,%llu\n",
con->engine ? con->engine : "", /* Storage engine we ran against */

Follow ups

References

  Thread Previous • Date Previous • • Thread Next