Bug #11766860 - 60085: CRASH IN ITEM::SAVE_IN_FIELD() WITH TIME DATA TYPE

This assumption in Item_cache_datetime::cache_value_int
was wrong:
-  /* Assume here that the underlying item will do correct conversion.*/
-  int_value= example->val_int_result();


mysql-test/r/subselect_innodb.result:
  New test case.
mysql-test/t/subselect_innodb.test:
  New test case.
sql/item.cc:
  In Item_cache_datetime::cache_value_int()
   - call get_time() or get_date() depending on desired type
   - convert the returned MYSQL_TIME value to longlong depending on desired type
sql/item.h:
  The cached int_value in Item_cache_datetime should not be unsigned:
   - it is used mostly in signed context
   - it can actually have negative value (for TIME data type)
sql/item_cmpfunc.cc:
  Add comment on Bug#59685
sql/item_subselect.cc:
  Add some DBUG_TRACE for easier bug-hunting.

6 files changed, 62 insertions, 13 deletions

diff --git a/mysql-test/r/subselect_innodb.result b/mysql-test/r/subselect_innodb.result
index 6c6d563e284..ab623ad6a28 100644
--- a/mysql-test/r/subselect_innodb.result
+++ b/mysql-test/r/subselect_innodb.result
@@ -245,3 +245,12 @@ x
 NULL
 drop procedure p1;
 drop tables t1,t2,t3;
+#
+# Bug#60085 crash in Item::save_in_field() with time data type
+#
+CREATE TABLE t1(a date, b int, unique(b), unique(a), key(b)) engine=innodb;
+INSERT INTO t1 VALUES ('2011-05-13', 0);
+SELECT * FROM t1 WHERE b < (SELECT CAST(a as date) FROM t1 GROUP BY a);
+a	b
+2011-05-13	0
+DROP TABLE t1;
diff --git a/mysql-test/t/subselect_innodb.test b/mysql-test/t/subselect_innodb.test
index 573fe0c1810..73491417e0c 100644
--- a/mysql-test/t/subselect_innodb.test
+++ b/mysql-test/t/subselect_innodb.test
@@ -238,3 +238,12 @@ call p1();
 call p1();
 drop procedure p1;
 drop tables t1,t2,t3;
+
+--echo #
+--echo # Bug#60085 crash in Item::save_in_field() with time data type
+--echo #
+
+CREATE TABLE t1(a date, b int, unique(b), unique(a), key(b)) engine=innodb;
+INSERT INTO t1 VALUES ('2011-05-13', 0);
+SELECT * FROM t1 WHERE b < (SELECT CAST(a as date) FROM t1 GROUP BY a); 
+DROP TABLE t1;
diff --git a/sql/item.cc b/sql/item.cc
index c7787d65c22..c15ef624a08 100644
--- a/sql/item.cc
+++ b/sql/item.cc
@@ -1059,7 +1059,9 @@ int Item::save_in_field_no_warnings(Field *field, bool no_conversions)
   ulonglong sql_mode= thd->variables.sql_mode;
   thd->variables.sql_mode&= ~(MODE_NO_ZERO_IN_DATE | MODE_NO_ZERO_DATE);
   thd->count_cuted_fields= CHECK_FIELD_IGNORE;
+
   res= save_in_field(field, no_conversions);
+
   thd->count_cuted_fields= tmp;
   dbug_tmp_restore_column_map(table->write_set, old_map);
   thd->variables.sql_mode= sql_mode;
@@ -7462,16 +7464,43 @@ longlong Item_cache_int::val_int()
 bool  Item_cache_datetime::cache_value_int()
 {
   if (!example)
-    return FALSE;
+    return false;
 
-  value_cached= TRUE;
+  value_cached= true;
   // Mark cached string value obsolete
-  str_value_cached= FALSE;
-  /* Assume here that the underlying item will do correct conversion.*/
-  int_value= example->val_int_result();
+  str_value_cached= false;
+
+  MYSQL_TIME ltime;
+  const bool eval_error= 
+    (field_type() == MYSQL_TYPE_TIME) ?
+    example->get_time(&ltime) :
+    example->get_date(&ltime, TIME_FUZZY_DATE);
+
+  if (eval_error)
+    int_value= 0;
+  else
+  {
+    switch(field_type())
+    {
+    case MYSQL_TYPE_DATETIME:
+    case MYSQL_TYPE_TIMESTAMP:
+      int_value= TIME_to_ulonglong_datetime(&ltime);
+      break;
+    case MYSQL_TYPE_TIME:
+      int_value= TIME_to_ulonglong_time(&ltime);
+      break;
+    default:
+      int_value= TIME_to_ulonglong_date(&ltime);
+      break;
+    }
+    if (ltime.neg)
+      int_value= -int_value;
+  }
+
   null_value= example->null_value;
   unsigned_flag= example->unsigned_flag;
-  return TRUE;
+
+  return true;
 }
 
 
diff --git a/sql/item.h b/sql/item.h
index fc203f03e79..6cd91f03604 100644
--- a/sql/item.h
+++ b/sql/item.h
@@ -3449,7 +3449,7 @@ class Item_cache_datetime: public Item_cache
 {
 protected:
   String str_value;
-  ulonglong int_value;
+  longlong int_value;
   bool str_value_cached;
 public:
   Item_cache_datetime(enum_field_types field_type_arg):
diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc
index 6be1d09323d..9586004c630 100644
--- a/sql/item_cmpfunc.cc
+++ b/sql/item_cmpfunc.cc
@@ -403,6 +403,7 @@ static bool convert_constant_item(THD *thd, Item_field *field_item,
   Field *field= field_item->field;
   int result= 0;
 
+  // TODO: revert Bug#59685 here, as we now cache datetimes correctly.
   if (!(*item)->with_subselect && (*item)->const_item())
   {
     TABLE *table= field->table;
diff --git a/sql/item_subselect.cc b/sql/item_subselect.cc
index 6b54a088112..118bae7342f 100644
--- a/sql/item_subselect.cc
+++ b/sql/item_subselect.cc
@@ -256,30 +256,31 @@ bool Item_subselect::walk(Item_processor processor, bool walk_subquery,
 
 bool Item_subselect::exec()
 {
-  int res;
+  DBUG_ENTER("Item_subselect::exec");
 
   /*
     Do not execute subselect in case of a fatal error
     or if the query has been killed.
   */
   if (thd->is_error() || thd->killed)
-    return 1;
+    DBUG_RETURN(true);
 
   DBUG_ASSERT(!thd->lex->context_analysis_only);
   /*
     Simulate a failure in sub-query execution. Used to test e.g.
     out of memory or query being killed conditions.
   */
-  DBUG_EXECUTE_IF("subselect_exec_fail", return 1;);
+  DBUG_EXECUTE_IF("subselect_exec_fail", DBUG_RETURN(true););
 
-  res= engine->exec();
+  bool res= engine->exec();
 
   if (engine_changed)
   {
     engine_changed= 0;
-    return exec();
+    res= exec();
+    DBUG_RETURN(res);
   }
-  return (res);
+  DBUG_RETURN(res);
 }
 
 Item::Type Item_subselect::type() const