From 0752d62f55c90e481462a484ac926e21eb3f7ffc Mon Sep 17 00:00:00 2001 From: Sergey Glukhov Date: Wed, 10 Feb 2010 18:56:47 +0400 Subject: Bug#45195 valgrind warnings about uninitialized values in store_record_in_cache() The problem becomes apparent only if HAVE_purify is undefined. It related to the part of code placed in open_table_from_share() fuction where we initialize record buffer only if HAVE_purify is enabled. So in case of HAVE_purify=OFF record buffer is not initialized on open table stage. Next we read key, find NULL value and update appropriate null bit but do not update record buffer. After that the record is stored in the join cache(store_record_in_cache). For CHAR fields we strip trailing spaces and in our case this procedure uses uninitialized record buffer. The fix is to skip stripping space procedure in case of null values for CHAR fields(partially based on 6.0 JOIN_CACHE implementation). mysql-test/r/join.result: test case mysql-test/t/join.test: test case sql/field.cc: code updated according to new CACHE_FIELD struct sql/sql_select.cc: code updated according to new CACHE_FIELD struct sql/sql_select.h: CACHE_FIELD struct: added new fields: Field *field, uint type; removed fields: Field_blob *blob_field, bool strip; --- sql/field.cc | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) (limited to 'sql/field.cc') diff --git a/sql/field.cc b/sql/field.cc index d8db3fdbae4..15ee9c4a86c 100644 --- a/sql/field.cc +++ b/sql/field.cc @@ -1705,11 +1705,10 @@ uint Field::fill_cache_field(CACHE_FIELD *copy) uint store_length; copy->str=ptr; copy->length=pack_length(); - copy->blob_field=0; + copy->field= this; if (flags & BLOB_FLAG) { - copy->blob_field=(Field_blob*) this; - copy->strip=0; + copy->type= CACHE_BLOB; copy->length-= table->s->blob_ptr_size; return copy->length; } @@ -1717,15 +1716,15 @@ uint Field::fill_cache_field(CACHE_FIELD *copy) (type() == MYSQL_TYPE_STRING && copy->length >= 4 && copy->length < 256)) { - copy->strip=1; /* Remove end space */ + copy->type= CACHE_STRIPPED; store_length= 2; } else { - copy->strip=0; + copy->type= 0; store_length= 0; } - return copy->length+ store_length; + return copy->length + store_length; } -- cgit v1.2.1 From 0c07d7c4253ae12567ec5a257d2fe478a8e89fcf Mon Sep 17 00:00:00 2001 From: Staale Smedseng Date: Sun, 14 Mar 2010 17:01:45 +0100 Subject: Bug #49829 Many "hides virtual function" warnings with SunStudio SunStudio compilers of late warn about methods that might hide methods in base classes due to the use of overloading combined with overriding. SunStudio also warns about variables defined in local socpe or method arguments that have the same name as a member attribute of the class. This patch renames methods that might hide base class methods, to make it easier both for humans and compilers to see what is actually called. It also renames variables in local scope. sql/field.cc: Local scope variable or method argument same as class attribute. sql/item_cmpfunc.cc: Local scope variable or method argument same as class attribute. sql/item_create.cc: Renaming base class create() to create_func(). sql/item_create.h: Renaming base class create() to create_func(). sql/protocol.cc: Local scope variable or method argument same as class attribute. sql/sql_profile.cc: Local scope variable or method argument same as class attribute. sql/sql_select.cc: Local scope variable or method argument same as class attribute. sql/sql_yacc.yy: Renaming base class create() to create_func(). storage/federated/ha_federated.cc: Local scope variable or method argument same as class attribute. storage/myisammrg/ha_myisammrg.cc: Local scope variable or method argument same as class attribute. --- sql/field.cc | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) (limited to 'sql/field.cc') diff --git a/sql/field.cc b/sql/field.cc index 15ee9c4a86c..fff6919d88d 100644 --- a/sql/field.cc +++ b/sql/field.cc @@ -1,4 +1,4 @@ -/* Copyright 2000-2008 MySQL AB, 2008-2009 Sun Microsystems, Inc. +/* Copyright (c) 2000, 2010 Oracle and/or its affiliates. All rights reserved. This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by @@ -2944,16 +2944,16 @@ Field_new_decimal::unpack(uchar* to, a decimal and write that to the raw data buffer. */ decimal_digit_t dec_buf[DECIMAL_MAX_PRECISION]; - decimal_t dec; - dec.len= from_precision; - dec.buf= dec_buf; + decimal_t dec_val; + dec_val.len= from_precision; + dec_val.buf= dec_buf; /* Note: bin2decimal does not change the length of the field. So it is just the first step the resizing operation. The second step does the resizing using the precision and decimals from the slave. */ - bin2decimal((uchar *)from, &dec, from_precision, from_decimal); - decimal2bin(&dec, to, precision, decimals()); + bin2decimal((uchar *)from, &dec_val, from_precision, from_decimal); + decimal2bin(&dec_val, to, precision, decimals()); } else memcpy(to, from, len); // Sizes are the same, just copy the data. @@ -6334,7 +6334,7 @@ check_string_copy_error(Field_str *field, SYNOPSIS Field_longstr::report_if_important_data() - ptr - Truncated rest of string + pstr - Truncated rest of string end - End of truncated string count_spaces - Treat traling spaces as important data @@ -6350,12 +6350,12 @@ check_string_copy_error(Field_str *field, */ int -Field_longstr::report_if_important_data(const char *ptr, const char *end, +Field_longstr::report_if_important_data(const char *pstr, const char *end, bool count_spaces) { - if ((ptr < end) && table->in_use->count_cuted_fields) + if ((pstr < end) && table->in_use->count_cuted_fields) { - if (test_if_important_data(field_charset, ptr, end)) + if (test_if_important_data(field_charset, pstr, end)) { if (table->in_use->abort_on_warning) set_warning(MYSQL_ERROR::WARN_LEVEL_ERROR, ER_DATA_TOO_LONG, 1); @@ -7008,9 +7008,8 @@ const uint Field_varstring::MAX_SIZE= UINT_MAX16; */ int Field_varstring::do_save_field_metadata(uchar *metadata_ptr) { - char *ptr= (char *)metadata_ptr; DBUG_ASSERT(field_length <= 65535); - int2store(ptr, field_length); + int2store((char*)metadata_ptr, field_length); return 2; } -- cgit v1.2.1 From 028d12568bfc4e1a469a80c14e18329e82ca71d2 Mon Sep 17 00:00:00 2001 From: Mats Kindahl Date: Wed, 17 Mar 2010 15:28:49 +0100 Subject: BUG#49618: Field length stored incorrectly in binary log for InnoDB The class Field_bit_as_char stores the metadata for the field incorrecly because bytes_in_rec and bit_len are set to (field_length + 7 ) / 8 and 0 respectively, while Field_bit has the correct values field_length / 8 and field_length % 8. Solved the problem by re-computing the values for the metadata based on the field_length instead of using the bytes_in_rec and bit_len variables. To handle compatibility with old server, a table map flag was added to indicate that the bit computation is exact. If the flag is clear, the slave computes the number of bytes required to store the bit field and compares that instead, effectively allowing replication *without conversion* from any field length that require the same number of bytes to store. mysql-test/suite/rpl/t/rpl_typeconv_innodb.test: Adding test to check compatibility for bit field replication when using InnoDB sql/field.cc: Extending compatible_field_size() with flags from table map to allow fields to check master info. sql/field.h: Extending compatible_field_size() with flags from table map to allow fields to check master info. sql/log.cc: Removing table map flags since they are not used outside table map class. sql/log_event.cc: Removing flags parameter from table map constructor since it is not used and does not have to be exposed. sql/log_event.h: Adding flag to denote that bit length for bit field type is exact and not potentially rounded to even bytes. sql/rpl_utility.cc: Adding fields to table_def to store table map flags. sql/rpl_utility.h: Removing obsolete comment and adding flags to store table map flags from master. --- sql/field.cc | 53 ++++++++++++++++++++++++++++++++++------------------- 1 file changed, 34 insertions(+), 19 deletions(-) (limited to 'sql/field.cc') diff --git a/sql/field.cc b/sql/field.cc index 354c911e1c0..7c7e334dff1 100644 --- a/sql/field.cc +++ b/sql/field.cc @@ -1373,12 +1373,14 @@ bool Field::send_binary(Protocol *protocol) to the size of this field (the slave or destination). @param field_metadata Encoded size in field metadata + @param mflags Flags from the table map event for the table. @retval 0 if this field's size is < the source field's size @retval 1 if this field's size is >= the source field's size */ int Field::compatible_field_size(uint field_metadata, - const Relay_log_info *rli_arg __attribute__((unused))) + const Relay_log_info *rli_arg __attribute__((unused)), + uint16 mflags __attribute__((unused))) { uint const source_size= pack_length_from_metadata(field_metadata); uint const destination_size= row_pack_length(); @@ -2836,7 +2838,8 @@ uint Field_new_decimal::pack_length_from_metadata(uint field_metadata) @retval 1 if this field's size is >= the source field's size */ int Field_new_decimal::compatible_field_size(uint field_metadata, - const Relay_log_info * __attribute__((unused))) + const Relay_log_info * __attribute__((unused)), + uint16 mflags __attribute__((unused))) { int compatible= 0; uint const source_precision= (field_metadata >> 8U) & 0x00ff; @@ -6612,7 +6615,8 @@ check_field_for_37426(const void *param_arg) #endif int Field_string::compatible_field_size(uint field_metadata, - const Relay_log_info *rli_arg) + const Relay_log_info *rli_arg, + uint16 mflags __attribute__((unused))) { #ifdef HAVE_REPLICATION const Check_field_param check_param = { this }; @@ -6620,7 +6624,7 @@ int Field_string::compatible_field_size(uint field_metadata, check_field_for_37426, &check_param)) return FALSE; // Not compatible field sizes #endif - return Field::compatible_field_size(field_metadata, rli_arg); + return Field::compatible_field_size(field_metadata, rli_arg, mflags); } @@ -9172,8 +9176,13 @@ uint Field_bit::get_key_image(uchar *buff, uint length, imagetype type_arg) */ int Field_bit::do_save_field_metadata(uchar *metadata_ptr) { - *metadata_ptr= bit_len; - *(metadata_ptr + 1)= bytes_in_rec; + /* + Since this class and Field_bit_as_char have different ideas of + what should be stored here, we compute the values of the metadata + explicitly using the field_length. + */ + metadata_ptr[0]= field_length % 8; + metadata_ptr[1]= field_length / 8; return 2; } @@ -9213,20 +9222,26 @@ uint Field_bit::pack_length_from_metadata(uint field_metadata) @retval 1 if this field's size is >= the source field's size */ int Field_bit::compatible_field_size(uint field_metadata, - const Relay_log_info * __attribute__((unused))) + const Relay_log_info * __attribute__((unused)), + uint16 mflags) { - int compatible= 0; - uint const source_size= pack_length_from_metadata(field_metadata); - uint const destination_size= row_pack_length(); - uint const from_bit_len= field_metadata & 0x00ff; - uint const from_len= (field_metadata >> 8U) & 0x00ff; - if ((bit_len == 0) || (from_bit_len == 0)) - compatible= (source_size <= destination_size); - else if (from_bit_len > bit_len) - compatible= (from_len < bytes_in_rec); - else - compatible= ((from_bit_len <= bit_len) && (from_len <= bytes_in_rec)); - return (compatible); + uint from_bit_len= 8 * (field_metadata >> 8) + (field_metadata & 0xff); + uint to_bit_len= max_display_length(); + + /* + If the bit length exact flag is clear, we are dealing with an old + master, so we allow some less strict behaviour if replicating by + moving both bit lengths to an even multiple of 8. + + We do this by computing the number of bytes to store the field + instead, and then compare the result. + */ + if (!(mflags & Table_map_log_event::TM_BIT_LEN_EXACT_F)) { + from_bit_len= (from_bit_len + 7) / 8; + to_bit_len= (to_bit_len + 7) / 8; + } + + return from_bit_len <= to_bit_len; } -- cgit v1.2.1