Hello Sanja,
On 15.09.2009, at 13:21, Oleksandr Byelkin wrote:
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.
I lost that part, I will update the --help text now.
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?
It is needed for the --create --engine case, when the user supplies a
DDL with --create and the engines he wants to test with --engine. I
moved down the init_dynamic_string() to the if () where it is actually
used
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 ?
I don't really need them. They are removed now.
[cut]
- 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)
I am creating a new patch after each review. Is that what you mean and
want me
to do?
[cut]
Thanks,
Hakan
--
Hakan Küçükyılmaz, QA/Benchmark Engineer, Stuttgart/Germany
Monty Program Ab, http://askmonty.org/
Skype: hakank_ Phone: +49 171 1919839
_______________________________________________
Mailing list: https://launchpad.net/~maria-developers
Post to : [email protected]
Unsubscribe : https://launchpad.net/~maria-developers
More help : https://help.launchpad.net/ListHelp
