Hi, Sergey!
On Mar 31, Sergey Glukhov wrote:
> #At file:///home/gluh/MySQL/mysql-5.1-bug-33717/ based on
> revid:matthias.leich@stripped
>
> 2840 Sergey Glukhov 2009-03-31
> Bug#33717 INSERT...(default) fails for enum. Crashes CSV tables, loads spaces
> for MyISAM
> Table corruption happens during table reading in ha_tina::find_current_row()
> func.
> Field::store() method returns error(true) if stored value is 0.
> The fix:
> added special case for enum type which correctly processes 0 value.
> Additional fix:
> INSERT...(default) and INSERT...() have the same behaviour now.
> @ mysql-test/r/csv.result
> test result
> @ mysql-test/r/default.result
> result fix
> @ mysql-test/t/csv.test
> test case
> @ sql/item.cc
> Changes:
> --do not print warning for 'enum' type if there is no default value
> --set default value
> @ storage/csv/ha_tina.cc
> Table corruption happens during table reading in ha_tina::find_current_row()
> func.
> Field::store() method returns error(true) if stored value is 0.
> The fix:
> added special case for enum type which correctly processes 0 value.
>
> === modified file 'mysql-test/r/default.result'
> --- a/mysql-test/r/default.result 2008-09-03 08:06:03 +0000
> +++ b/mysql-test/r/default.result 2009-03-31 08:49:59 +0000
> @@ -180,7 +180,6 @@ insert into bug20691 values (2, 3, 5, '0
> insert into bug20691 values (DEFAULT, DEFAULT, DEFAULT, DEFAULT, DEFAULT, DEFAULT,
> DEFAULT, DEFAULT, DEFAULT, 4);
> Warnings:
> Warning 1364 Field 'a' doesn't have a default value
> -Warning 1364 Field 'b' doesn't have a default value
agree
> Warning 1364 Field 'c' doesn't have a default value
> Warning 1364 Field 'd' doesn't have a default value
> Warning 1364 Field 'e' doesn't have a default value
> === modified file 'sql/item.cc'
> --- a/sql/item.cc 2009-03-27 13:00:20 +0000
> +++ b/sql/item.cc 2009-03-31 08:49:59 +0000
> @@ -6234,6 +6234,7 @@ void Item_default_value::print(String *s
>
> int Item_default_value::save_in_field(Field *field_arg, bool no_conversions)
> {
> + int err= 0;
> if (!arg)
> {
> if (field_arg->flags & NO_DEFAULT_VALUE_FLAG)
> @@ -6255,7 +6256,7 @@ int Item_default_value::save_in_field(Fi
> view->view_db.str,
> view->view_name.str);
> }
> - else
> + else if (field_arg->real_type() != MYSQL_TYPE_ENUM)
why it's here and not above as in
if (field_arg->flags & NO_DEFAULT_VALUE_FLAG ||
field_arg->real_type() != MYSQL_TYPE_ENUM)
Actually, I'd even prefer
if (field_arg->flags & NO_DEFAULT_VALUE_FLAG
#if MYSQL_VERSION_ID < 0x60000
|| field_arg->real_type() != MYSQL_TYPE_ENUM)
#endif
:)
> {
> push_warning_printf(field_arg->table->in_use,
> MYSQL_ERROR::WARN_LEVEL_WARN,
> @@ -6263,10 +6264,10 @@ int Item_default_value::save_in_field(Fi
> ER(ER_NO_DEFAULT_FOR_FIELD),
> field_arg->field_name);
> }
> - return 1;
> + err= 1;
you wouldn't need it if you'd added the condition to the upper-level if().
> }
> field_arg->set_default();
> - return 0;
> + return err;
> }
> return Item_field::save_in_field(field_arg, no_conversions);
> }
>
> === modified file 'storage/csv/ha_tina.cc'
> --- a/storage/csv/ha_tina.cc 2009-03-24 09:02:01 +0000
> +++ b/storage/csv/ha_tina.cc 2009-03-31 08:49:59 +0000
> @@ -679,9 +679,13 @@ int ha_tina::find_current_row(uchar *buf
>
> if (read_all || bitmap_is_set(table->read_set, (*field)->field_index))
> {
> + bool is_enum= ((*field)->real_type() == MYSQL_TYPE_ENUM);
> if ((*field)->store(buffer.ptr(), buffer.length(), buffer.charset(),
> - CHECK_FIELD_WARN))
> - goto err;
> + is_enum ? CHECK_FIELD_IGNORE : CHECK_FIELD_WARN))
> + {
> + if (!is_enum)
> + goto err;
> + }
I don't understand this part. Why would Field_enum::store() return 1
here ? And if it would - why it's not an error ?
> if ((*field)->flags & BLOB_FLAG)
> {
> Field_blob *blob= *(Field_blob**) field;
Regards / Mit vielen Grüßen,
Sergei
--
__ ___ ___ ____ __
/ |/ /_ __/ __/ __ \/ / Sergei Golubchik <serg@stripped>
/ /|_/ / // /\ \/ /_/ / /__ Principal Software Engineer/Server Architect
/_/ /_/\_, /___/\___\_\___/ Sun Microsystems GmbH, HRB München 161028
<___/ Sonnenallee 1, 85551 Kirchheim-Heimstetten
Geschäftsführer: Thomas Schroeder, Wolfgang Engels, Dr. Roland Boemer
Vorsitzender des Aufsichtsrates: Martin Häring
