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

Postreview addons by Bar

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

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;

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;

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;
 }
 

sql/item_func.cc

@@ -887,7 +887,7 @@ String *Item_func_hybrid_result_type::val_str(String *str)
   case DECIMAL_RESULT:
   {
     my_decimal decimal_value, *val;
-    if (!(val= decimal_op(&decimal_value)))
+    if (!(val= decimal_op_with_null_check(&decimal_value)))
       return 0;                                 // null is set
     my_decimal_round(E_DEC_FATAL_ERROR, val, decimals, FALSE, val);
     str->set_charset(collation.collation);
@@ -914,24 +914,22 @@ String *Item_func_hybrid_result_type::val_str(String *str)
     if (is_temporal_type(field_type()))
     {
       MYSQL_TIME ltime;
-      if (date_op(&ltime,
+      if (date_op_with_null_check(&ltime) ||
-                  field_type() == MYSQL_TYPE_TIME ? TIME_TIME_ONLY : 0) ||
+          (null_value= str->alloc(MAX_DATE_STRING_REP_LENGTH)))
-          str->alloc(MAX_DATE_STRING_REP_LENGTH))
-      {
-        null_value= 1;
         return (String *) 0;
-      }
       ltime.time_type= mysql_type_to_time_type(field_type());
       str->length(my_TIME_to_str(&ltime, const_cast<char*>(str->ptr()), decimals));
       str->set_charset(&my_charset_bin);
+      DBUG_ASSERT(!null_value);
       return str;
     }
-    return str_op(&str_value);
+    return str_op_with_null_check(&str_value);
   case TIME_RESULT:
   case ROW_RESULT:
   case IMPOSSIBLE_RESULT:
     DBUG_ASSERT(0);
   }
+  DBUG_ASSERT(!null_value || (str == NULL));
   return str;
 }
 
@@ -944,7 +942,7 @@ double Item_func_hybrid_result_type::val_real()
   {
     my_decimal decimal_value, *val;
     double result;
-    if (!(val= decimal_op(&decimal_value)))
+    if (!(val= decimal_op_with_null_check(&decimal_value)))
       return 0.0;                               // null is set
     my_decimal2double(E_DEC_FATAL_ERROR, val, &result);
     return result;
@@ -961,18 +959,14 @@ double Item_func_hybrid_result_type::val_real()
     if (is_temporal_type(field_type()))
     {
       MYSQL_TIME ltime;
-      if (date_op(&ltime,
+      if (date_op_with_null_check(&ltime))
-                  field_type() == MYSQL_TYPE_TIME ? TIME_TIME_ONLY : 0 ))
-      {
-        null_value= 1;
         return 0;
-      }
       ltime.time_type= mysql_type_to_time_type(field_type());
       return TIME_to_double(&ltime);
     }
     char *end_not_used;
     int err_not_used;
-    String *res= str_op(&str_value);
+    String *res= str_op_with_null_check(&str_value);
     return (res ? my_strntod(res->charset(), (char*) res->ptr(), res->length(),
 			     &end_not_used, &err_not_used) : 0.0);
   }
@@ -992,7 +986,7 @@ longlong Item_func_hybrid_result_type::val_int()
   case DECIMAL_RESULT:
   {
     my_decimal decimal_value, *val;
-    if (!(val= decimal_op(&decimal_value)))
+    if (!(val= decimal_op_with_null_check(&decimal_value)))
       return 0;                                 // null is set
     longlong result;
     my_decimal2int(E_DEC_FATAL_ERROR, val, unsigned_flag, &result);
@@ -1007,18 +1001,14 @@ longlong Item_func_hybrid_result_type::val_int()
     if (is_temporal_type(field_type()))
     {
       MYSQL_TIME ltime;
-      if (date_op(&ltime,
+      if (date_op_with_null_check(&ltime))
-                  field_type() == MYSQL_TYPE_TIME ? TIME_TIME_ONLY : 0))
-      {
-        null_value= 1;
         return 0;
-      }
       ltime.time_type= mysql_type_to_time_type(field_type());
       return TIME_to_ulonglong(&ltime);
     }
     int err_not_used;
     String *res;
-    if (!(res= str_op(&str_value)))
+    if (!(res= str_op_with_null_check(&str_value)))
       return 0;
 
     char *end= (char*) res->ptr() + res->length();
@@ -1040,17 +1030,21 @@ my_decimal *Item_func_hybrid_result_type::val_decimal(my_decimal *decimal_value)
   DBUG_ASSERT(fixed == 1);
   switch (cached_result_type) {
   case DECIMAL_RESULT:
-    val= decimal_op(decimal_value);
+    val= decimal_op_with_null_check(decimal_value);
     break;
   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;
   }
@@ -1059,19 +1053,20 @@ my_decimal *Item_func_hybrid_result_type::val_decimal(my_decimal *decimal_value)
     if (is_temporal_type(field_type()))
     {
       MYSQL_TIME ltime;
-      if (date_op(&ltime,
+      if (date_op_with_null_check(&ltime))
-                  field_type() == MYSQL_TYPE_TIME ? TIME_TIME_ONLY : 0))
       {
         my_decimal_set_zero(decimal_value);
-        null_value= 1;
         return 0;
       }
       ltime.time_type= mysql_type_to_time_type(field_type());
       return date2my_decimal(&ltime, decimal_value);
     }
     String *res;
-    if (!(res= str_op(&str_value)))
+    if (!(res= str_op_with_null_check(&str_value)))
+    {
+      null_value= 1;
       return NULL;
+    }
 
     str2my_decimal(E_DEC_FATAL_ERROR, (char*) res->ptr(),
                    res->length(), res->charset(), decimal_value);
@@ -1094,7 +1089,7 @@ bool Item_func_hybrid_result_type::get_date(MYSQL_TIME *ltime,
   case DECIMAL_RESULT:
   {
     my_decimal value, *res;
-    if (!(res= decimal_op(&value)) ||
+    if (!(res= decimal_op_with_null_check(&value)) ||
         decimal_to_datetime_with_warn(res, ltime, fuzzydate,
                                       field_name_or_null()))
       goto err;
@@ -1124,7 +1119,7 @@ bool Item_func_hybrid_result_type::get_date(MYSQL_TIME *ltime,
       return date_op(ltime, fuzzydate);
     char buff[40];
     String tmp(buff,sizeof(buff), &my_charset_bin),*res;
-    if (!(res= str_op(&tmp)) ||
+    if (!(res= str_op_with_null_check(&tmp)) ||
         str_to_datetime_with_warn(res->charset(), res->ptr(), res->length(),
                                   ltime, fuzzydate) <= MYSQL_TIMESTAMP_ERROR)
       goto err;

sql/item_func.h

@@ -411,6 +411,29 @@ public:
 
 class Item_func_hybrid_result_type: public Item_func
 {
+  /*
+    Helper methods to make sure that the result of
+    decimal_op(), str_op() and date_op() is properly synched with null_value.
+  */
+  bool date_op_with_null_check(MYSQL_TIME *ltime)
+  {
+     bool rc= date_op(ltime,
+                      field_type() == MYSQL_TYPE_TIME ? TIME_TIME_ONLY : 0);
+     DBUG_ASSERT(!rc ^ null_value);
+     return rc;
+  }
+  String *str_op_with_null_check(String *str)
+  {
+    String *res= str_op(str);
+    DBUG_ASSERT((res != NULL) ^ null_value);
+    return res;
+  }
+  my_decimal *decimal_op_with_null_check(my_decimal *decimal_buffer)
+  {
+    my_decimal *res= decimal_op(decimal_buffer);
+    DBUG_ASSERT((res != NULL) ^ null_value);
+    return res;
+  }
 protected:
   Item_result cached_result_type;