From a6c1d56e083bcb8ab2c32f8d89865ef46c0eb70e Mon Sep 17 00:00:00 2001 From: Sergey Petrunya Date: Mon, 22 Nov 2010 19:34:03 +0300 Subject: MWL#121-125 DS-MRR improvements - Address Monty's review feedback, part 1 - Fix buildbot failure --- sql/key.cc | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) (limited to 'sql/key.cc') diff --git a/sql/key.cc b/sql/key.cc index 89423e5280e..708f309fbaf 100644 --- a/sql/key.cc +++ b/sql/key.cc @@ -548,3 +548,53 @@ next_loop: } while (key_info); /* no more keys to test */ DBUG_RETURN(0); } + + +/* + Compare two key tuples. + + @brief + Compare two key tuples, i.e. two key values in KeyTupleFormat. + + @param part KEY_PART_INFO with key description + @param key1 First key to compare + @param key2 Second key to compare + @param tuple_length Length of key1 (and key2, they are the same) in bytes. + + @return + @retval 0 key1 == key2 + @retval -1 key1 < key2 + @retval +1 key1 > key2 +*/ + +int key_tuple_cmp(KEY_PART_INFO *part, uchar *key1, uchar *key2, + uint tuple_length) +{ + uchar *key1_end= key1 + tuple_length; + int len; + int res; + LINT_INIT(len); + for (;key1 < key1_end; key1 += len, key2 += len, part++) + { + len= part->store_length; + if (part->null_bit) + { + if (*key1) // key1 == NULL + { + if (!*key2) // key1(NULL) < key2(notNULL) + return -1; + continue; + } + else if (*key2) // key1(notNULL) > key2 (NULL) + return 1; + /* Step over the NULL bytes for key_cmp() call */ + key1++; + key2++; + } + if ((res= part->field->key_cmp(key1, key2))) + return res; + } + return 0; +} + + -- cgit v1.2.1 From fad2ec74999a7ff513c9421cae2c84b2408b7e92 Mon Sep 17 00:00:00 2001 From: Sergey Petrunya Date: Tue, 23 Nov 2010 18:08:11 +0300 Subject: MWL#121-125 DS-MRR improvements - Address Monty's review feedback, part 3 --- sql/key.cc | 1 + 1 file changed, 1 insertion(+) (limited to 'sql/key.cc') diff --git a/sql/key.cc b/sql/key.cc index 708f309fbaf..b7f52a5833b 100644 --- a/sql/key.cc +++ b/sql/key.cc @@ -590,6 +590,7 @@ int key_tuple_cmp(KEY_PART_INFO *part, uchar *key1, uchar *key2, /* Step over the NULL bytes for key_cmp() call */ key1++; key2++; + len--; } if ((res= part->field->key_cmp(key1, key2))) return res; -- cgit v1.2.1 From 7c56b08216d5ff709d10e4ca662d0215cd823c09 Mon Sep 17 00:00:00 2001 From: Michael Widenius Date: Sat, 27 Nov 2010 17:29:52 +0200 Subject: Added TRASH() to table->record[0] to find out if we access not initialzed data. - Changed Cached_item_field not copy data for fields with NULL value - In key_copy() and key_restore() don't copy data for fields with NULL value Fixed code to avoid valgrind warnings - Use c_ptr_safe instead of c_ptr() Removed "QQ" from comments (QQ was ment to be used for internal comments that should be removed before pushing) Fixed wrong alias used (from previous patch) sql/event_db_repository.cc: Update testing if event table is valid (to avoid valgrind errors) sql/ha_partition.cc: m_ordered_scan_ongoing was not initialized Reset null bits in record to avoid valgrind errors sql/handler.h: Added flag if storage engine will write row verbatim and the row contains varchar or null fields (in which case we must clear the row to avoid valgrind warnings) sql/item_buff.cc: Changed Cached_item_field not copy data for fields with NULL value (Optimization and avoids valgrind warnings) sql/item_func.cc: c_ptr() -> c_ptr_safe() sql/key.cc: In key_copy() and key_restore() don't copy data for fields with NULL value sql/opt_range.cc: c_ptr() -> c_ptr_safe() sql/sql_base.cc: Added TRASH() to table->record[0] to find out if we access not initialzed data. Initialize null_bytes to: - Get consistent tests - Ensure we don't get valgrind warnings for null fields (as we may only update a couple of bits in a byte) sql/sql_class.cc: Removed "QQ" from comments sql/sql_insert.cc: Initialize row to default values if we are using valgrind and row will be copied verbatim to disk in storage engine. sql/sql_load.cc: QQ -> TODO sql/sql_parse.cc: Removed old not used code marked QQ and withing "#ifdef REMOVED" sql/sql_select.cc: QQ -> TODO Initialize some variables that was used uninitialized Added DBUG_ASSERT() to find out if thd was not properly initialized for sub queries sql/sql_test.cc: Fixed format for printing to DBUG file Fixed wrong alias used (from previous patch) sql/sql_trigger.h: QQ -> TODO sql/table.cc: QQ -> TODO storage/maria/ha_maria.cc: Mark table with HA_RECORD_MUST_BE_CLEAN_ON_WRITE, if row is written verbatim to disk and contains varchar or null fields. storage/maria/ma_open.c: Added flags if table has varchar or null fields storage/maria/maria_def.h: Added flags if table has varchar or null fields storage/myisam/ha_myisam.cc: Mark table with HA_RECORD_MUST_BE_CLEAN_ON_WRITE, if row is written verbatim to disk and contains varchar or null fields. storage/myisam/mi_open.c: Fixed memory overrun bug when using fulltext keys storage/xtradb/row/row0sel.c: Removed initialization of null bits. (not needed anymore) --- sql/key.cc | 33 ++++++++++++++++++++++++++------- 1 file changed, 26 insertions(+), 7 deletions(-) (limited to 'sql/key.cc') diff --git a/sql/key.cc b/sql/key.cc index 89423e5280e..a1793e9e5f4 100644 --- a/sql/key.cc +++ b/sql/key.cc @@ -113,13 +113,24 @@ void key_copy(uchar *to_key, uchar *from_record, KEY *key_info, if (key_length == 0) key_length= key_info->key_length; - for (key_part= key_info->key_part; (int) key_length > 0; key_part++) + for (key_part= key_info->key_part; + (int) key_length > 0; + key_part++, to_key+= length, key_length-= length) { if (key_part->null_bit) { *to_key++= test(from_record[key_part->null_offset] & key_part->null_bit); key_length--; + if (to_key[-1]) + { + /* + Don't copy data for null values + The -1 below is to subtract the null byte which is already handled + */ + length= min(key_length, (uint) key_part->store_length-1); + continue; + } } if (key_part->key_part_flag & HA_BLOB_PART || key_part->key_part_flag & HA_VAR_LENGTH_PART) @@ -138,8 +149,6 @@ void key_copy(uchar *to_key, uchar *from_record, KEY *key_info, if (bytes < length) cs->cset->fill(cs, (char*) to_key + bytes, length - bytes, ' '); } - to_key+= length; - key_length-= length; } } @@ -166,16 +175,28 @@ void key_restore(uchar *to_record, uchar *from_key, KEY *key_info, { key_length= key_info->key_length; } - for (key_part= key_info->key_part ; (int) key_length > 0 ; key_part++) + for (key_part= key_info->key_part ; + (int) key_length > 0 ; + key_part++, from_key+= length, key_length-= length) { uchar used_uneven_bits= 0; if (key_part->null_bit) { - if (*from_key++) + bool null_value; + if ((null_value= *from_key++)) to_record[key_part->null_offset]|= key_part->null_bit; else to_record[key_part->null_offset]&= ~key_part->null_bit; key_length--; + if (null_value) + { + /* + Don't copy data for null bytes + The -1 below is to subtract the null byte which is already handled + */ + length= min(key_length, (uint) key_part->store_length-1); + continue; + } } if (key_part->type == HA_KEYTYPE_BIT) { @@ -229,8 +250,6 @@ void key_restore(uchar *to_record, uchar *from_key, KEY *key_info, memcpy(to_record + key_part->offset, from_key + used_uneven_bits , (size_t) length - used_uneven_bits); } - from_key+= length; - key_length-= length; } } -- cgit v1.2.1 From a095346a9d32ca583d211d07075e805881fdb4e5 Mon Sep 17 00:00:00 2001 From: Igor Babaev Date: Wed, 22 Dec 2010 00:37:35 -0800 Subject: Fixed LP bug #670380. Lifted the limitation that hash join could not be used over varchar fields with non-binary collation. --- sql/key.cc | 251 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 251 insertions(+) (limited to 'sql/key.cc') diff --git a/sql/key.cc b/sql/key.cc index a1793e9e5f4..08502cfe6cc 100644 --- a/sql/key.cc +++ b/sql/key.cc @@ -567,3 +567,254 @@ next_loop: } while (key_info); /* no more keys to test */ DBUG_RETURN(0); } + + +/** + Get hash value for the key from a key buffer + + @param key_info the key descriptor + @param used_key_part number of key parts used for the key + @param key pointer to the buffer with the key value + + @datails + When hashing we should take special care only of: + 1. NULLs (and keyparts which can be null so one byte reserved for it); + 2. Strings for which we have to take into account their collations + and the values of their lengths in the prefixes. + + @return hash value calculated for the key +*/ + +ulong key_hashnr(KEY *key_info, uint used_key_parts, const uchar *key) +{ + ulong nr=1, nr2=4; + KEY_PART_INFO *key_part= key_info->key_part; + KEY_PART_INFO *end_key_part= key_part + used_key_parts; + + for (; key_part < end_key_part; key_part++) + { + uchar *pos= (uchar*)key; + CHARSET_INFO *cs; + uint length, pack_length; + bool is_string= TRUE; + LINT_INIT(cs); + key+= key_part->length; + if (key_part->null_bit) + { + key++; /* Skip null byte */ + if (*pos) /* Found null */ + { + nr^= (nr << 1) | 1; + /* Add key pack length to key for VARCHAR segments */ + switch (key_part->type) { + case HA_KEYTYPE_VARTEXT1: + case HA_KEYTYPE_VARBINARY1: + key++; + break; + case HA_KEYTYPE_VARTEXT2: + case HA_KEYTYPE_VARBINARY2: + key+= 2; + break; + default: + ; + } + continue; + } + pos++; /* Skip null byte */ + } + /* If it is string set parameters of the string */ + switch (key_part->type) { + case HA_KEYTYPE_TEXT: + cs= key_part->field->charset(); + length= key_part->length; + pack_length= 0; + break; + case HA_KEYTYPE_BINARY : + cs= &my_charset_bin; + length= key_part->length; + pack_length= 0; + break; + case HA_KEYTYPE_VARTEXT1: + cs= key_part->field->charset(); + length= (uint)(pos[0]); + pack_length= 1; + break; + case HA_KEYTYPE_VARBINARY1: + cs= &my_charset_bin; + length= (uint)(pos[0]); + pack_length= 1; + break; + case HA_KEYTYPE_VARTEXT2: + cs= key_part->field->charset(); + length= uint2korr(pos); + pack_length= 2; + break; + case HA_KEYTYPE_VARBINARY2: + cs= &my_charset_bin; + length= uint2korr(pos); + pack_length= 2; + break; + default: + is_string= FALSE; + } + + if (is_string) + { + if (cs->mbmaxlen > 1) + { + uint char_length= my_charpos(cs, pos + pack_length, + pos + pack_length + length, + length / cs->mbmaxlen); + set_if_smaller(length, char_length); + } + cs->coll->hash_sort(cs, pos+pack_length, length, &nr, &nr2); + key+= pack_length; + } + else + { + for (; pos < (uchar*)key ; pos++) + { + nr^=(ulong) ((((uint) nr & 63)+nr2)*((uint) *pos)) + (nr << 8); + nr2+=3; + } + } + } + DBUG_PRINT("exit", ("hash: %lx", nr)); + return(nr); +} + + +/** + Check whether two keys in the key buffers are equal + + @param key_info the key descriptor + @param used_key_part number of key parts used for the keys + @param key1 pointer to the buffer with the first key + @param key2 pointer to the buffer with the second key + + @detail See details of key_hashnr(). + + @retval TRUE keys in the buffers are NOT equal + @retval FALSE keys in the buffers are equal +*/ + +bool key_buf_cmp(KEY *key_info, uint used_key_parts, + const uchar *key1, const uchar *key2) +{ + KEY_PART_INFO *key_part= key_info->key_part; + KEY_PART_INFO *end_key_part= key_part + used_key_parts; + + for (; key_part < end_key_part; key_part++) + { + uchar *pos1= (uchar*)key1; + uchar *pos2= (uchar*)key2; + CHARSET_INFO *cs; + uint length1, length2, pack_length; + bool is_string= TRUE; + LINT_INIT(cs); + key1+= key_part->length; + key2+= key_part->length; + if (key_part->null_bit) + { + key1++; key2++; /* Skip null byte */ + if (*pos1 && *pos2) /* Both are null */ + { + /* Add key pack length to key for VARCHAR segments */ + switch (key_part->type) { + case HA_KEYTYPE_VARTEXT1: + case HA_KEYTYPE_VARBINARY1: + key1++; key2++; + break; + case HA_KEYTYPE_VARTEXT2: + case HA_KEYTYPE_VARBINARY2: + key1+= 2; key2+= 2; + break; + default: + ; + } + continue; + } + if (*pos1 != *pos2) + return FALSE; + pos1++; pos2++; + } + + /* If it is string set parameters of the string */ + switch (key_part->type) { + case HA_KEYTYPE_TEXT: + cs= key_part->field->charset(); + length1= length2= key_part->length; + pack_length= 0; + break; + case HA_KEYTYPE_BINARY : + cs= &my_charset_bin; + length1= length2= key_part->length; + pack_length= 0; + break; + case HA_KEYTYPE_VARTEXT1: + cs= key_part->field->charset(); + length1= (uint)(pos1[0]); + length2= (uint)(pos2[0]); + pack_length= 1; + break; + case HA_KEYTYPE_VARBINARY1: + cs= &my_charset_bin; + length1= (uint)(pos1[0]); + length2= (uint)(pos2[0]); + pack_length= 1; + break; + case HA_KEYTYPE_VARTEXT2: + cs= key_part->field->charset(); + length1= uint2korr(pos1); + length2= uint2korr(pos2); + pack_length= 2; + break; + case HA_KEYTYPE_VARBINARY2: + cs= &my_charset_bin; + length1= uint2korr(pos1); + length2= uint2korr(pos2); + pack_length= 2; + break; + default: + is_string= FALSE; + } + + if (is_string) + { + /* + Compare the strings taking into account length in characters + and collation + */ + uint byte_len1= length1, byte_len2= length2; + if (cs->mbmaxlen > 1) + { + uint char_length1= my_charpos(cs, pos1 + pack_length, + pos1 + pack_length + length1, + length1 / cs->mbmaxlen); + uint char_length2= my_charpos(cs, pos2 + pack_length, + pos2 + pack_length + length2, + length2 / cs->mbmaxlen); + set_if_smaller(length1, char_length1); + set_if_smaller(length2, char_length2); + } + if (length1 != length2 || + cs->coll->strnncollsp(cs, + pos1 + pack_length, byte_len1, + pos2 + pack_length, byte_len2, + 1)) + return TRUE; + key1+= pack_length; key2+= pack_length; + } + else + { + /* it is OK to compare non-string byte per byte */ + for (; pos1 < (uchar*)key1 ; pos1++, pos2++) + { + if (pos1[0] != pos2[0]) + return TRUE; + } + } + } + return FALSE; +} + -- cgit v1.2.1 From 18dc64eca2bbe29697043dc0ae51ae21d19af365 Mon Sep 17 00:00:00 2001 From: Igor Babaev Date: Sun, 26 Dec 2010 16:31:03 -0800 Subject: Fixed LP bug #694443. One of the hash functions employed by the BNLH join algorithm calculates the the value of hash index for key value utilizing every byte of the key buffer. To make this calculation valid one has to ensure that for any key value unused bytes of the buffer are filled with with a certain filler. We choose 0 as a filler for these bytes. Added an optional boolean parameter with_zerofill to the function key_copy. If the value of the parameter is TRUE all unused bytes of the key buffer is filled with 0. --- sql/key.cc | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) (limited to 'sql/key.cc') diff --git a/sql/key.cc b/sql/key.cc index 0de1d1fd642..780cc6733c1 100644 --- a/sql/key.cc +++ b/sql/key.cc @@ -103,10 +103,11 @@ int find_ref_key(KEY *key, uint key_count, uchar *record, Field *field, @param from_record full record to be copied from @param key_info descriptor of the index @param key_length specifies length of all keyparts that will be copied + @param with_zerofill skipped bytes in the key buffer to be filled with 0 */ void key_copy(uchar *to_key, uchar *from_record, KEY *key_info, - uint key_length) + uint key_length, bool with_zerofill) { uint length; KEY_PART_INFO *key_part; @@ -129,6 +130,8 @@ void key_copy(uchar *to_key, uchar *from_record, KEY *key_info, The -1 below is to subtract the null byte which is already handled */ length= min(key_length, (uint) key_part->store_length-1); + if (with_zerofill) + bzero((char*) to_key, length); continue; } } @@ -137,7 +140,9 @@ void key_copy(uchar *to_key, uchar *from_record, KEY *key_info, { key_length-= HA_KEY_BLOB_LENGTH; length= min(key_length, key_part->length); - key_part->field->get_key_image(to_key, length, Field::itRAW); + uint bytes= key_part->field->get_key_image(to_key, length, Field::itRAW); + if (with_zerofill && bytes < length) + bzero((char*) to_key + bytes, length - bytes); to_key+= HA_KEY_BLOB_LENGTH; } else -- cgit v1.2.1 From cb4fa7f401267bf887066100726c53f10b712e6d Mon Sep 17 00:00:00 2001 From: Igor Babaev Date: Wed, 5 Jan 2011 15:03:30 -0800 Subject: Fixed LP bug #697557. When stored in a key buffer any varchar field has a length prefix that always takes 2 bytes. --- sql/key.cc | 22 ++-------------------- 1 file changed, 2 insertions(+), 20 deletions(-) (limited to 'sql/key.cc') diff --git a/sql/key.cc b/sql/key.cc index 780cc6733c1..fd5c129eee8 100644 --- a/sql/key.cc +++ b/sql/key.cc @@ -689,20 +689,12 @@ ulong key_hashnr(KEY *key_info, uint used_key_parts, const uchar *key) pack_length= 0; break; case HA_KEYTYPE_VARTEXT1: - cs= key_part->field->charset(); - length= (uint)(pos[0]); - pack_length= 1; - break; - case HA_KEYTYPE_VARBINARY1: - cs= &my_charset_bin; - length= (uint)(pos[0]); - pack_length= 1; - break; case HA_KEYTYPE_VARTEXT2: cs= key_part->field->charset(); length= uint2korr(pos); pack_length= 2; break; + case HA_KEYTYPE_VARBINARY1: case HA_KEYTYPE_VARBINARY2: cs= &my_charset_bin; length= uint2korr(pos); @@ -806,23 +798,13 @@ bool key_buf_cmp(KEY *key_info, uint used_key_parts, pack_length= 0; break; case HA_KEYTYPE_VARTEXT1: - cs= key_part->field->charset(); - length1= (uint)(pos1[0]); - length2= (uint)(pos2[0]); - pack_length= 1; - break; - case HA_KEYTYPE_VARBINARY1: - cs= &my_charset_bin; - length1= (uint)(pos1[0]); - length2= (uint)(pos2[0]); - pack_length= 1; - break; case HA_KEYTYPE_VARTEXT2: cs= key_part->field->charset(); length1= uint2korr(pos1); length2= uint2korr(pos2); pack_length= 2; break; + case HA_KEYTYPE_VARBINARY1: case HA_KEYTYPE_VARBINARY2: cs= &my_charset_bin; length1= uint2korr(pos1); -- cgit v1.2.1 From d492903502f2c584a063609e9a77cd25d1e50335 Mon Sep 17 00:00:00 2001 From: Igor Babaev Date: Thu, 27 Jan 2011 21:23:02 -0800 Subject: Fixed LP bug #707827. This bug could manifest itself when hash join over a varchar column with NULL values in some rows was used. It happened because the function key_buf_cmp erroneously returned FALSE when one of the joined key fields was null while the second was not. Also fixed two other bugs in the functions key_hashnr and key_buf_cmp that could possibly lead to wrong results for some queries that used hash join over several columns with nulls. Also reverted the latest addition of the test case for bug #45092. It had been already backported earlier. --- sql/key.cc | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) (limited to 'sql/key.cc') diff --git a/sql/key.cc b/sql/key.cc index fd5c129eee8..19db7e9ec1f 100644 --- a/sql/key.cc +++ b/sql/key.cc @@ -663,8 +663,6 @@ ulong key_hashnr(KEY *key_info, uint used_key_parts, const uchar *key) switch (key_part->type) { case HA_KEYTYPE_VARTEXT1: case HA_KEYTYPE_VARBINARY1: - key++; - break; case HA_KEYTYPE_VARTEXT2: case HA_KEYTYPE_VARBINARY2: key+= 2; @@ -769,8 +767,6 @@ bool key_buf_cmp(KEY *key_info, uint used_key_parts, switch (key_part->type) { case HA_KEYTYPE_VARTEXT1: case HA_KEYTYPE_VARBINARY1: - key1++; key2++; - break; case HA_KEYTYPE_VARTEXT2: case HA_KEYTYPE_VARBINARY2: key1+= 2; key2+= 2; @@ -778,10 +774,10 @@ bool key_buf_cmp(KEY *key_info, uint used_key_parts, default: ; } - continue; + continue; } if (*pos1 != *pos2) - return FALSE; + return TRUE; pos1++; pos2++; } -- cgit v1.2.1 From d5adc29d1c39027c827074f936d3f28e71f87800 Mon Sep 17 00:00:00 2001 From: Michael Widenius Date: Fri, 1 Apr 2011 12:04:59 +0300 Subject: Fixed compiler warnings sql/key.cc: Fixed compiler warnings about not initialized variables --- sql/key.cc | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'sql/key.cc') diff --git a/sql/key.cc b/sql/key.cc index 19db7e9ec1f..f7966140cdf 100644 --- a/sql/key.cc +++ b/sql/key.cc @@ -652,6 +652,9 @@ ulong key_hashnr(KEY *key_info, uint used_key_parts, const uchar *key) uint length, pack_length; bool is_string= TRUE; LINT_INIT(cs); + LINT_INIT(length); + LINT_INIT(pack_length); + key+= key_part->length; if (key_part->null_bit) { @@ -756,6 +759,9 @@ bool key_buf_cmp(KEY *key_info, uint used_key_parts, uint length1, length2, pack_length; bool is_string= TRUE; LINT_INIT(cs); + LINT_INIT(length1); + LINT_INIT(length2); + key1+= key_part->length; key2+= key_part->length; if (key_part->null_bit) -- cgit v1.2.1 From 8d52c2cffe0ff75ea9a4313c776cc5441e70aef3 Mon Sep 17 00:00:00 2001 From: Michael Widenius Date: Wed, 11 May 2011 13:59:17 +0300 Subject: Fixed compiler warnings and test cases problems found by buildbot mysql-test/r/dyncol.result: Updated test results mysql-test/r/index_intersect.result: Updated results mysql-test/r/index_intersect_innodb.result: Updated results mysql-test/t/dyncol.test: Added replace_result for floating point results that are different on windows Added round() around a result to get same result on all platforms. mysql-test/t/index_intersect.test: Added replace_result to fix that index_merge may put key names in different order. mysys/ma_dyncol.c: Fixed compiler warnings on Solaris sql/key.cc: Fixed compiler warnings on Solaris sql/mysqld.cc: Fixed compiler warning on windows support-files/compiler_warnings.supp: Suppressed an unintersting warning on Solaris --- sql/key.cc | 1 + 1 file changed, 1 insertion(+) (limited to 'sql/key.cc') diff --git a/sql/key.cc b/sql/key.cc index f7966140cdf..7f484bee32b 100644 --- a/sql/key.cc +++ b/sql/key.cc @@ -761,6 +761,7 @@ bool key_buf_cmp(KEY *key_info, uint used_key_parts, LINT_INIT(cs); LINT_INIT(length1); LINT_INIT(length2); + LINT_INIT(pack_length); key1+= key_part->length; key2+= key_part->length; -- cgit v1.2.1