Hi Sanja, From my understanding, the patch fixes consequences of the problem, but not the problem itself. It very likely will still have crashes with slightly modified SQL scripts. The source of the problem is that Item_func_hybrid_result_type::val_decimal() does not return NULL when int_op() or real_op() set null_value to "true". By the way, compare Item_func_hybrid_result_type::val_decimal() with a very similar method Item_func_hybrid_result_type::val_str(). The latter already handle the INT_RESULT and REAL_RESULT cases well and return NULL properly in case of null_value. get_date(), val_int(), val_real() also look fine for me. So the are problems only in val_decimal(). I propose a small patch that fixes the source of the problem. This is the small patch:

@@ -1038,17 +1026,21 @@ my_decimal *Item_func_hybrid_result_type::val_decimal(my_decimal *decimal_value) case INT_RESULT: { longlong result= int_op(); + if (null_value) + return NULL; int2my_decimal(E_DEC_FATAL_ERROR, result, unsigned_flag, decimal_value); break; } case REAL_RESULT: { double result= (double)real_op(); + if (null_value) + return NULL; double2my_decimal(E_DEC_FATAL_ERROR, result, decimal_value); break; }

I also propose an extended patch version, to cover the code with more DBUG_ASSERTs, so in case of the null_value related bugs it crashes much earlier than in Item::send(). It will be easier to debug problems this way. To avoid much code duplication, I added three private helper methods. The extended version of the patch is attached. The scripts from the bug report start to return expected results, with no crashes in DEBUG build, and "mtr --suite=main" works fine. I have not run the entire mtr though, to save time. Greetings. On 09/03/2015 08:00 PM, sanja@mariadb.com wrote:

revision-id: 62fe87c5a239c7304b245ce5954c87aecb6382ad (mariadb-5.5.45-1-g62fe87c) parent(s): fa51f70dc68fe2f3afe943e2c81fcbdb34f16cad committer: Oleksandr Byelkin timestamp: 2015-09-03 18:00:43 +0200 message:

MDEV-8663: IF Statement returns multiple values erroneously (or Assertion `!null_value' failed in Item::send(Protocol*, String*))

keeping contract: NULL value mean NULL pointer in val_str and val_deciman.

--- mysql-test/r/func_if.result | 17 +++++++++++++++++ mysql-test/t/func_if.test | 14 ++++++++++++++ sql/item_cmpfunc.cc | 6 ++++-- sql/item_func.cc | 5 +++++ 4 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/mysql-test/r/func_if.result b/mysql-test/r/func_if.result index c7f548a..61f63cc 100644 --- a/mysql-test/r/func_if.result +++ b/mysql-test/r/func_if.result @@ -234,3 +234,20 @@ SELECT if(1, NULL, (SELECT min('hello'))); if(1, NULL, (SELECT min('hello'))) NULL End of 5.2 tests +# +# MDEV-8663: IF Statement returns multiple values erroneously +# (or Assertion `!null_value' failed in Item::send(Protocol*, String*) +# +CREATE TABLE `t1` ( +`datas` VARCHAR(25) NOT NULL +) ENGINE=InnoDB DEFAULT CHARSET=utf8; +Warnings: +Warning 1286 Unknown storage engine 'InnoDB' +Warning 1266 Using storage engine MyISAM for table 't1' +INSERT INTO `t1` VALUES ('1,2'), ('2,3'), ('3,4'); +SELECT IF(FIND_IN_SET('1', `datas`), 1.5, IF(FIND_IN_SET('2', `datas`), 2, NULL)) AS `First`, '1' AS `Second`, '2' AS `Third` FROM `t1`; +First Second Third +1.5 1 2 +2.0 1 2 +NULL 1 2 +drop table t1; diff --git a/mysql-test/t/func_if.test b/mysql-test/t/func_if.test index 2b89a61..8fdba77 100644 --- a/mysql-test/t/func_if.test +++ b/mysql-test/t/func_if.test @@ -206,6 +206,20 @@ SELECT if(1, NULL, (SELECT min('hello')));

--echo End of 5.2 tests

+--echo # +--echo # MDEV-8663: IF Statement returns multiple values erroneously +--echo # (or Assertion `!null_value' failed in Item::send(Protocol*, String*) +--echo # +CREATE TABLE `t1` ( +`datas` VARCHAR(25) NOT NULL +) ENGINE=InnoDB DEFAULT CHARSET=utf8; + +INSERT INTO `t1` VALUES ('1,2'), ('2,3'), ('3,4'); + +SELECT IF(FIND_IN_SET('1', `datas`), 1.5, IF(FIND_IN_SET('2', `datas`), 2, NULL)) AS `First`, '1' AS `Second`, '2' AS `Third` FROM `t1`; + +drop table t1; + --disable_query_log # Restore timezone to default set time_zone= @@global.time_zone; diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc index 998cb1c..4f07318 100644 --- a/sql/item_cmpfunc.cc +++ b/sql/item_cmpfunc.cc @@ -2692,7 +2692,8 @@ Item_func_if::str_op(String *str) String *res=arg->val_str(str); if (res) res->set_charset(collation.collation); - null_value=arg->null_value; + if (null_value=arg->null_value) + res= NULL; return res; }

@@ -2703,7 +2704,8 @@ Item_func_if::decimal_op(my_decimal *decimal_value) DBUG_ASSERT(fixed == 1); Item *arg= args[0]->val_bool() ? args[1] : args[2]; my_decimal *value= arg->val_decimal(decimal_value); - null_value= arg->null_value; + if (null_value= arg->null_value) + value= NULL; return value; }

diff --git a/sql/item_func.cc b/sql/item_func.cc index 2f51308..cbb8aa0 100644 --- a/sql/item_func.cc +++ b/sql/item_func.cc @@ -924,6 +924,7 @@ String *Item_func_hybrid_result_type::val_str(String *str) ltime.time_type= mysql_type_to_time_type(field_type()); str->length(my_TIME_to_str(<ime, const_cast<char*>(str->ptr()), decimals)); str->set_charset(&my_charset_bin); + DBUG_ASSERT(!null_value); return str; } return str_op(&str_value); @@ -932,6 +933,7 @@ String *Item_func_hybrid_result_type::val_str(String *str) case IMPOSSIBLE_RESULT: DBUG_ASSERT(0); } + DBUG_ASSERT(!null_value || (str == NULL)); return str; }

@@ -1071,7 +1073,10 @@ my_decimal *Item_func_hybrid_result_type::val_decimal(my_decimal *decimal_value) } String *res; if (!(res= str_op(&str_value))) + { + null_value= 1; return NULL; + }

str2my_decimal(E_DEC_FATAL_ERROR, (char*) res->ptr(), res->length(), res->charset(), decimal_value); _______________________________________________ commits mailing list commits@mariadb.org https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits