Drizzle

Merge lp://staging/~diego-fmpwizard/drizzle/bug-fixes into lp://staging/~drizzle-trunk/drizzle/development

  1. bug-fixes
  2. Merge into development
Proposed by fmpwizard
Status: Superseded
Proposed branch: lp://staging/~diego-fmpwizard/drizzle/bug-fixes
Merge into: lp://staging/~drizzle-trunk/drizzle/development
Diff against target: None lines
To merge this branch: bzr merge lp://staging/~diego-fmpwizard/drizzle/bug-fixes
Reviewer Review Type Date Requested Status
fmpwizard (community) Needs Resubmitting
Jay Pipes (community) Needs Fixing
Review via email: mp+11758@code.staging.launchpad.net

This proposal supersedes a proposal from 2009-09-04.

This proposal has been superseded by a proposal from 2009-09-22.

To post a comment you must log in.
Revision history for this message
fmpwizard (diego-fmpwizard) wrote : Posted in a previous version of this proposal

* Fixed bug* #377826 "Failure to detect error in allocating transcation_prealloc_size"

* I converted transaction_prealloc_size to a uint64_t.

That eliminated the strange values that ended up assigned to "transcation_prealloc_size" when you tried to set it to a value of 1024*1024*1024*4 or higher.

* To get a warning on those same cases, I had to add:

throw_bounds_warning(session, true, true, getName(), (int64_t) tmp);

to

set_var.cc:

bool sys_var_session_uint64_t::update(Session *session, set_var *var)

* I also enabled the test suite under big/t/variables-big.test
* Had to modify a test case for
    set @@max_heap_table_size= 4294967296;
  ** Because it now returns a warning about the value being truncated (truncated to 4294966272 ) (This is different than what MySQL does, MySQL accepts a value of 4294967296)

Revision history for this message
Monty Taylor (mordred) wrote : Posted in a previous version of this proposal

Looks great! Merging in to lp:drizzle/mordred

review: Approve
Revision history for this message
Jay Pipes (jaypipes) wrote : Posted in a previous version of this proposal

Pulling into my captain branch now....

Revision history for this message
Monty Taylor (mordred) wrote : Posted in a previous version of this proposal

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Jay Pipes wrote:
> Pulling into my captain branch now....

There are problems in this, actually. Specifically, you need to do:

=== modified file 'drizzled/session.h'
- --- drizzled/session.h 2009-09-06 06:18:00 +0000
+++ drizzled/session.h 2009-09-07 22:36:14 +0000
@@ -164,7 +164,7 @@
   uint32_t query_alloc_block_size;
   uint32_t query_prealloc_size;
   uint32_t trans_alloc_block_size;
- - uint64_t trans_prealloc_size;
+ uint32_t trans_prealloc_size;
   uint64_t group_concat_max_len;
   /* TODO: change this to my_thread_id - but have to fix set_var first */
   uint64_t pseudo_thread_id;

=== modified file 'drizzled/set_var.cc'
- --- drizzled/set_var.cc 2009-09-03 23:21:40 +0000
+++ drizzled/set_var.cc 2009-09-07 22:36:14 +0000
@@ -194,7 +194,7 @@
 static sys_var_session_uint32_t sys_trans_alloc_block_size(&vars,
"transaction_alloc_block_size",

&SV::trans_alloc_block_size,
                                                            false,
fix_trans_mem_root);
- -static sys_var_session_uint64_t sys_trans_prealloc_size(&vars,
"transaction_prealloc_size",
+static sys_var_session_uint32_t sys_trans_prealloc_size(&vars,
"transaction_prealloc_size",

&SV::trans_prealloc_size);

 static sys_var_const_str_ptr sys_secure_file_priv(&vars,
"secure_file_priv",
@@ -470,16 +470,22 @@
   return out;
 }

- -static bool get_unsigned32(Session *, set_var *var)
+static bool get_unsigned32(Session *session, set_var *var)
 {
   if (var->value->unsigned_flag)
- - var->save_result.uint32_t_value= (uint32_t) var->value->val_int();
+ var->save_result.uint32_t_value=
+ static_cast<uint32_t>(var->value->val_int());
   else
   {
     int64_t v= var->value->val_int();
- - var->save_result.uint32_t_value= (uint32_t) ((v < 0) ? 0 : v);
+ if (v > UINT32_MAX)
+ throw_bounds_warning(session, true, true,
var->var->getName().c_str(), v);
+
+ var->save_result.uint32_t_value=
+ static_cast<uint32_t>((v > UINT32_MAX) ? UINT32_MAX : (v < 0) ? 0
: v);
+
   }
- - return 0;
+ return false;
 }

 static bool get_unsigned64(Session *, set_var *var)

Otherwise performance on solaris DIES.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkqugjUACgkQ2Jv7/VK1RgHNQQCgnp40a/QO7Ewo9qV9UlWXv7jv
SNAAnRWAqOgMFdfhiIxkj1HTMQoUGyut
=2Ipj
-----END PGP SIGNATURE-----

Revision history for this message
Jay Pipes (jaypipes) wrote : Posted in a previous version of this proposal

After merging and applying that fix, I get this on Solaris and Linux 64:

main.variables [ fail ] --- ../tests/r/variables.result 2009-09-14 20:17:16.000000000 +0300
+++ ../tests/r/variables.reject 2009-09-14 21:36:31.000000000 +0300
@@ -544,20 +544,14 @@
 set @@global.error_count=1;
 ERROR HY000: Variable 'error_count' is a read only variable
 set @@max_heap_table_size= 4294967296;
-Warnings:
-Error 1292 Truncated incorrect max_heap_table_size value: '4294967296'
 select @@max_heap_table_size > 0;
 @@max_heap_table_size > 0
 1
 set global max_heap_table_size= 4294967296;
-Warnings:
-Error 1292 Truncated incorrect max_heap_table_size value: '4294967296'
 select @@max_heap_table_size > 0;
 @@max_heap_table_size > 0
 1
 set @@max_heap_table_size= 4294967296;
-Warnings:
-Error 1292 Truncated incorrect max_heap_table_size value: '4294967296'
 select @@max_heap_table_size > 0;
 @@max_heap_table_size > 0
 1

drizzletest: Result content mismatch

Revision history for this message
Jay Pipes (jaypipes) wrote :

Going into lp:~jaypipes/drizzle/captain-20090914-01 branch. Tests running now...

review: Approve
Revision history for this message
Jay Pipes (jaypipes) wrote :

:( Still getting error. Only occurs on MacOSX 64-bit:

main.variables [ fail ] --- ../tests/r/variables.result 2009-09-14 21:48:49.000000000 +0300
+++ ../tests/r/variables.reject 2009-09-15 18:08:59.000000000 +0300
@@ -544,14 +544,20 @@
 set @@global.error_count=1;
 ERROR HY000: Variable 'error_count' is a read only variable
 set @@max_heap_table_size= 4294967296;
+Warnings:
+Error 1292 Truncated incorrect max_heap_table_size value: '4294967296'
 select @@max_heap_table_size > 0;
 @@max_heap_table_size > 0
 1
 set global max_heap_table_size= 4294967296;
+Warnings:
+Error 1292 Truncated incorrect max_heap_table_size value: '4294967296'
 select @@max_heap_table_size > 0;
 @@max_heap_table_size > 0
 1
 set @@max_heap_table_size= 4294967296;
+Warnings:
+Error 1292 Truncated incorrect max_heap_table_size value: '4294967296'
 select @@max_heap_table_size > 0;
 @@max_heap_table_size > 0
 1

review: Needs Fixing
lp://staging/~diego-fmpwizard/drizzle/bug-fixes updated
1132. By Diego Medina <email address hidden>

Fixes lp bug#432210
* Only call com_rehash(NULL, NULL) if we are connected to a server and if the option to rehash is enabled.

1133. By Diego Medina <email address hidden>

Fixed a crash that came after fixing lp bug#432210 if you call use <db_name> twice on a
disconnected session.

Revision history for this message
fmpwizard (diego-fmpwizard) wrote :

> :( Still getting error. Only occurs on MacOSX 64-bit:

Hi Jay,

I logged in into hades to see what the problem was and I found out that drizzle was being built as 32-bit, instead of 64-bit

I tried to build it as a 64 bit but libdrizzle had to be recompiled as 64-bit as well (I could do this, but wanted to check first)

To force a 64-bit build on hades, you would need to run

$ ./config/autorun.sh && MACOSX_DEPLOYMENT_TARGET=10.5 \
CPPFLAGS="-arch x86_64" \
CFLAGS=" -arch x86_64 " \
CCFLAGS=" -arch x86_64 " \
CXXFLAGS=" -arch x86_64 " \
LDFLAGS=" -arch x86_64 " \
./configure \
--with-debug \
--with-lib-prefix=/opt/local \
--prefix=/Applications/mysql/enterprise/drizzle-local && make -jx

So, I see two solutions:
1) enable the "hack" on the test, so that it detects if drizzle is 32 or 64bit and enable warnings accordingly.
2) force hades to build 64 bit and we tell people that mac 32bit is not going to have all test passing. :(

I'd go for 1), unless anyone sees a better solution.

Thanks

Diego

>
> main.variables [ fail ] --- ../tests/r/variables.result
> 2009-09-14 21:48:49.000000000 +0300
> +++ ../tests/r/variables.reject 2009-09-15 18:08:59.000000000 +0300
> @@ -544,14 +544,20 @@
> set @@global.error_count=1;
> ERROR HY000: Variable 'error_count' is a read only variable
> set @@max_heap_table_size= 4294967296;
> +Warnings:
> +Error 1292 Truncated incorrect max_heap_table_size value: '4294967296'
> select @@max_heap_table_size > 0;
> @@max_heap_table_size > 0
> 1
> set global max_heap_table_size= 4294967296;
> +Warnings:
> +Error 1292 Truncated incorrect max_heap_table_size value: '4294967296'
> select @@max_heap_table_size > 0;
> @@max_heap_table_size > 0
> 1
> set @@max_heap_table_size= 4294967296;
> +Warnings:
> +Error 1292 Truncated incorrect max_heap_table_size value: '4294967296'
> select @@max_heap_table_size > 0;
> @@max_heap_table_size > 0
> 1

review: Needs Resubmitting
lp://staging/~diego-fmpwizard/drizzle/bug-fixes updated
1134. By Diego Medina <email address hidden>

* Fixed LP bug#435619
** Protect error_message(code) from retrieving a negative index which causes a crash

* When a new session starts, initialize query_length (this was another
crash on the logging_* plugins)(timing dependant)

1135. By Diego Medina <email address hidden>

Enabled a hack to account for 32-bit vs 64-bit tests results

1136. By Diego Medina <email address hidden>

finally fixed the 64-bit vs 32-bit test case

1137. By Diego Medina <email address hidden>

* Fixes bug#438852
** variables test fails as of build 1144 on linux 32 bit machines
** Some linux systems use i686 to indicate they are 32-bit

1138. By Diego Medina <email address hidden>

* On certain UPDATE and DELETE statements, drizzled failed an assert() in
   Diagnostic_area.
** Fixed by resetting m_status after "delete select"
* Fixed LP bug#439719
* Added test case

1139. By Diego Medina <email address hidden>

resolved small merge issue

1140. By Diego Medina <email address hidden>

uncommented a call to reset_diagnostics_area()

Unmerged revisions

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'drizzled/session.cc'
2--- drizzled/session.cc 2009-08-16 14:14:39 +0000
3+++ drizzled/session.cc 2009-09-03 04:00:45 +0000
4@@ -223,6 +223,7 @@
5 thread_id= 0;
6 file_id = 0;
7 query_id= 0;
8+ query= NULL;
9 warn_id= 0;
10 memset(ha_data, 0, sizeof(ha_data));
11 replication_data= 0;
12
13=== modified file 'drizzled/session.h'
14--- drizzled/session.h 2009-08-17 21:30:20 +0000
15+++ drizzled/session.h 2009-09-15 02:21:15 +0000
16@@ -783,7 +783,10 @@
17 /** Returns the length of the current query text */
18 inline size_t getQueryLength() const
19 {
20- return strlen(query);
21+ if (query != NULL)
22+ return strlen(query);
23+ else
24+ return 0;
25 }
26
27 /** Accessor method returning the server's ID. */
28
29=== modified file 'drizzled/set_var.cc'
30--- drizzled/set_var.cc 2009-08-13 21:55:13 +0000
31+++ drizzled/set_var.cc 2009-09-15 02:21:15 +0000
32@@ -196,7 +196,7 @@
33 false, fix_trans_mem_root);
34 static sys_var_session_uint32_t sys_trans_prealloc_size(&vars, "transaction_prealloc_size",
35 &SV::trans_prealloc_size,
36- false, fix_trans_mem_root);
37+ false, fix_session_mem_root);
38
39 static sys_var_const_str_ptr sys_secure_file_priv(&vars, "secure_file_priv",
40 &opt_secure_file_priv);
41@@ -471,16 +471,21 @@
42 return out;
43 }
44
45-static bool get_unsigned32(Session *, set_var *var)
46+static bool get_unsigned32(Session *session, set_var *var)
47 {
48 if (var->value->unsigned_flag)
49- var->save_result.uint32_t_value= (uint32_t) var->value->val_int();
50+ var->save_result.uint32_t_value=
51+ static_cast<uint32_t>(var->value->val_int());
52 else
53 {
54 int64_t v= var->value->val_int();
55- var->save_result.uint32_t_value= (uint32_t) ((v < 0) ? 0 : v);
56+ if (v > UINT32_MAX)
57+ throw_bounds_warning(session, true, true,var->var->getName().c_str(), v);
58+
59+ var->save_result.uint32_t_value=
60+ static_cast<uint32_t>((v > UINT32_MAX) ? UINT32_MAX : (v < 0) ? 0 : v);
61 }
62- return 0;
63+ return false;
64 }
65
66 static bool get_unsigned64(Session *, set_var *var)
67@@ -743,7 +748,10 @@
68 uint64_t tmp= var->save_result.uint64_t_value;
69
70 if (tmp > max_system_variables.*offset)
71+ {
72+ throw_bounds_warning(session, true, true, getName(), (int64_t) tmp);
73 tmp= max_system_variables.*offset;
74+ }
75
76 if (option_limits)
77 tmp= fix_unsigned(session, tmp, option_limits);
78
79=== modified file 'tests/suite/big/r/variables-big.result'
80--- tests/suite/big/r/variables-big.result 2009-07-31 15:28:05 +0000
81+++ tests/suite/big/r/variables-big.result 2009-08-27 14:05:30 +0000
82@@ -10,3 +10,15 @@
83 show processlist;
84 Id User Host db Command Time State Info
85 # root 127.0.0.1 test Query 0 NULL show processlist
86+set session transaction_prealloc_size=1024*1024*1024*4;
87+Warnings:
88+Error 1292 Truncated incorrect transaction_prealloc_size value: '4294967296'
89+show processlist;
90+Id User Host db Command Time State Info
91+# root 127.0.0.1 test Query 0 NULL show processlist
92+set session transaction_prealloc_size=1024*1024*1024*5;
93+Warnings:
94+Error 1292 Truncated incorrect transaction_prealloc_size value: '5368709120'
95+show processlist;
96+Id User Host db Command Time State Info
97+# root 127.0.0.1 test Query 0 NULL show processlist
98
99=== modified file 'tests/suite/big/t/variables-big.test'
100--- tests/suite/big/t/variables-big.test 2009-07-31 15:28:05 +0000
101+++ tests/suite/big/t/variables-big.test 2009-08-27 14:05:30 +0000
102@@ -18,7 +18,9 @@
103
104 # Drizzle bug #377826
105 # please uncomment after the bug is fixed
106-#set session transaction_prealloc_size=1024*1024*1024*4;
107-#show processlist;
108-#set session transaction_prealloc_size=1024*1024*1024*5;
109-#show processlist;
110+set session transaction_prealloc_size=1024*1024*1024*4;
111+--replace_column 1 #
112+show processlist;
113+set session transaction_prealloc_size=1024*1024*1024*5;
114+--replace_column 1 #
115+show processlist;
116
117=== modified file 'tests/t/variables.test'
118--- tests/t/variables.test 2009-06-16 00:46:22 +0000
119+++ tests/t/variables.test 2009-09-15 02:21:15 +0000
120@@ -407,14 +407,12 @@
121 #
122 # Bug #10351: Setting ulong variable to > MAX_ULONG fails on 32-bit platform
123 #
124-
125 set @@max_heap_table_size= 4294967296;
126 select @@max_heap_table_size > 0;
127 set global max_heap_table_size= 4294967296;
128 select @@max_heap_table_size > 0;
129 set @@max_heap_table_size= 4294967296;
130 select @@max_heap_table_size > 0;
131-
132 #
133 # Bug #11775 Variable character_set_system does not exist (sometimes)
134 #