A fix and test case for Bug#6049 "Loss of sign when using prepared

statements and negative time/date values". 
The bug was in wrong sprintf format used in the client library.
The fix moves TIME -> string conversion functions to sql-common and
utilized them in the client library.


include/my_time.h:
  Declarations for new functions shared between the client and server.
libmysql/libmysql.c:
  Fix for Bug#6049 "Loss of sign when using prepared statements and negative
  time/date values": use the same function as the server to convert
  date/time/datetime values to strings.
sql-common/my_time.c:
  Implementation of my_{time,datetime,date,TIME}_to_str: it's
  needed by the client library, so it should be shared.
sql/field.cc:
  Don't create String object if it's not needed.
sql/item.cc:
  Don't create String object if it's not needed: TIME_to_string was
  moved to my_TIME_to_str, with different arguments.
sql/item_timefunc.cc:
  Don't create String object if it's not needed.
sql/mysql_priv.h:
  TIME_to_string and MAX_DATE_REP_LENGTH moved to the client library.
  MAX_DATE_REP_LENGTH was renamed to MAX_DATE_STRING_REP_LENGTH to not 
  conflict with the same name in libmysql.c
sql/protocol.cc:
  Don't create String object if it's not needed.
sql/time.cc:
  Implementation of my_{time,date,datetime,TIME}_to_str moved to my_time.c
  shared between the client and the server.
tests/client_test.c:
  A test case for Bug#6049.

include/my_time.h

@@ -60,6 +60,20 @@ my_system_gmt_sec(const MYSQL_TIME *t, long *my_timezone, bool *in_dst_time_gap)
 
 void set_zero_time(MYSQL_TIME *tm);
 
+/*
+  Required buffer length for my_time_to_str, my_date_to_str,
+  my_datetime_to_str and TIME_to_string functions. Note, that the
+  caller is still responsible to check that given TIME structure
+  has values in valid ranges, otherwise size of the buffer could
+  be not enough.
+*/
+#define MAX_DATE_STRING_REP_LENGTH 30
+
+int my_time_to_str(const MYSQL_TIME *l_time, char *to);
+int my_date_to_str(const MYSQL_TIME *l_time, char *to);
+int my_datetime_to_str(const MYSQL_TIME *l_time, char *to);
+int my_TIME_to_str(const MYSQL_TIME *l_time, char *to);
+
 C_MODE_END
 
 #endif /* _my_time_h_ */

libmysql/libmysql.c

@@ -3556,28 +3556,8 @@ static void fetch_datetime_with_conversion(MYSQL_BIND *param,
       Convert time value  to string and delegate the rest to
       fetch_string_with_conversion:
     */
-    char buff[25];
+    char buff[MAX_DATE_STRING_REP_LENGTH];
-    uint length;
+    uint length= my_TIME_to_str(time, buff);
-
-    switch (time->time_type) {
-    case MYSQL_TIMESTAMP_DATE:
-      length= my_sprintf(buff,(buff, "%04d-%02d-%02d",
-                               time->year, time->month, time->day));
-      break;
-    case MYSQL_TIMESTAMP_DATETIME:
-      length= my_sprintf(buff,(buff, "%04d-%02d-%02d %02d:%02d:%02d",
-                               time->year, time->month, time->day,
-                               time->hour, time->minute, time->second));
-      break;
-    case MYSQL_TIMESTAMP_TIME:
-      length= my_sprintf(buff, (buff, "%02d:%02d:%02d",
-                                time->hour, time->minute, time->second));
-      break;
-    default:
-      length= 0;
-      buff[0]='\0';
-      break;
-    }
     /* Resort to string conversion */
     fetch_string_with_conversion(param, (char *)buff, length);
     break;

sql-common/my_time.c

@@ -726,3 +726,75 @@ void set_zero_time(MYSQL_TIME *tm)
   tm->time_type= MYSQL_TIMESTAMP_NONE;
 }
 
+
+/*
+  Functions to convert time/date/datetime value to a string,
+  using default format.
+  This functions don't check that given TIME structure members are
+  in valid range. If they are not, return value won't reflect any
+  valid date either. Additionally, make_time doesn't take into
+  account time->day member: it's assumed that days have been converted
+  to hours already.
+
+  RETURN
+    number of characters written to 'to'
+*/
+
+int my_time_to_str(const MYSQL_TIME *l_time, char *to)
+{
+  return my_sprintf(to, (to, "%s%02d:%02d:%02d",
+                         (l_time->neg ? "-" : ""),
+                         l_time->hour,
+                         l_time->minute,
+                         l_time->second));
+}
+
+int my_date_to_str(const MYSQL_TIME *l_time, char *to)
+{
+  return my_sprintf(to, (to, "%04d-%02d-%02d",
+                         l_time->year,
+                         l_time->month,
+                         l_time->day));
+}
+
+int my_datetime_to_str(const MYSQL_TIME *l_time, char *to)
+{
+  return my_sprintf(to, (to, "%04d-%02d-%02d %02d:%02d:%02d",
+                         l_time->year,
+                         l_time->month,
+                         l_time->day,
+                         l_time->hour,
+                         l_time->minute,
+                         l_time->second));
+}
+
+
+/*
+  Convert struct DATE/TIME/DATETIME value to string using built-in
+  MySQL time conversion formats.
+
+  SYNOPSIS
+    my_TIME_to_string()
+
+  NOTE
+    The string must have at least MAX_DATE_STRING_REP_LENGTH bytes reserved.
+*/
+
+int my_TIME_to_str(const MYSQL_TIME *l_time, char *to)
+{
+  switch (l_time->time_type) {
+  case MYSQL_TIMESTAMP_DATETIME:
+    return my_datetime_to_str(l_time, to);
+  case MYSQL_TIMESTAMP_DATE:
+    return my_date_to_str(l_time, to);
+  case MYSQL_TIMESTAMP_TIME:
+    return my_time_to_str(l_time, to);
+  case MYSQL_TIMESTAMP_NONE:
+  case MYSQL_TIMESTAMP_ERROR:
+    to[0]='\0';
+    return 0;
+  default:
+    DBUG_ASSERT(0);
+    return 0;
+  }
+}

sql/field.cc

@@ -455,11 +455,9 @@ bool Field::get_time(TIME *ltime)
 
 void Field::store_time(TIME *ltime,timestamp_type type)
 {
-  char buff[MAX_DATE_REP_LENGTH];
+  char buff[MAX_DATE_STRING_REP_LENGTH];
-  String tmp;
+  uint length= (uint) my_TIME_to_str(ltime, buff);
-  tmp.set(buff, sizeof(buff), &my_charset_bin);
+  store(buff, length, &my_charset_bin);
-  TIME_to_string(ltime, &tmp);
-  store(buff, tmp.length(), &my_charset_bin);
 }
 
 

sql/item.cc

@@ -988,9 +988,10 @@ String *Item_param::val_str(String* str)
     return str;
   case TIME_VALUE:
   {
-    if (str->reserve(MAX_DATE_REP_LENGTH))
+    if (str->reserve(MAX_DATE_STRING_REP_LENGTH))
       break;
-    TIME_to_string(&value.time, str);
+    str->length((uint) my_TIME_to_str(&value.time, (char*) str->ptr()));
+    str->set_charset(&my_charset_bin);
     return str;
   }
   case NULL_VALUE:
@@ -1020,24 +1021,19 @@ const String *Item_param::query_val_str(String* str) const
   case TIME_VALUE:
     {
       char *buf, *ptr;
-      String tmp;
       str->length(0);
       /*
         TODO: in case of error we need to notify replication
         that binary log contains wrong statement 
       */
-      if (str->reserve(MAX_DATE_REP_LENGTH+3))
+      if (str->reserve(MAX_DATE_STRING_REP_LENGTH+3))
         break; 
 
       /* Create date string inplace */
       buf= str->c_ptr_quick();
       ptr= buf;
       *ptr++= '\'';
-      tmp.set(ptr, MAX_DATE_REP_LENGTH, &my_charset_bin);
+      ptr+= (uint) my_TIME_to_str(&value.time, ptr);
-      tmp.length(0);
-      TIME_to_string(&value.time, &tmp);
-
-      ptr+= tmp.length();
       *ptr++= '\'';
       str->length((uint32) (ptr - buf));
       break;

sql/item_timefunc.cc

@@ -1292,14 +1292,13 @@ String *Item_func_curtime::val_str(String *str)
 void Item_func_curtime::fix_length_and_dec()
 {
   TIME ltime;
-  String tmp((char*) buff,sizeof(buff), &my_charset_bin);
 
   decimals=0;
   collation.set(&my_charset_bin);
   store_now_in_TIME(&ltime);
   value= TIME_to_ulonglong_time(&ltime);
-  make_time((DATE_TIME_FORMAT *) 0, &ltime, &tmp);
+  buff_length= (uint) my_time_to_str(&ltime, buff);
-  max_length= buff_length= tmp.length();
+  max_length= buff_length;
 }
 
 
@@ -1341,16 +1340,14 @@ String *Item_func_now::val_str(String *str)
 
 void Item_func_now::fix_length_and_dec()
 {
-  String tmp((char*) buff,sizeof(buff),&my_charset_bin);
-
   decimals=0;
   collation.set(&my_charset_bin);
 
   store_now_in_TIME(&ltime);
   value= (longlong) TIME_to_ulonglong_datetime(&ltime);
 
-  make_datetime((DATE_TIME_FORMAT *) 0, &ltime, &tmp);
+  buff_length= (uint) my_datetime_to_str(&ltime, buff);
-  max_length= buff_length= tmp.length();
+  max_length= buff_length;
 }
 
 

sql/mysql_priv.h

@@ -284,14 +284,6 @@ void debug_sync_point(const char* lock_name, uint lock_timeout);
 #define WEEK_MONDAY_FIRST    1
 #define WEEK_YEAR            2
 #define WEEK_FIRST_WEEKDAY   4
-/*
-  Required buffer length for make_date, make_time, make_datetime
-  and TIME_to_string functions. Note, that the caller is still
-  responsible to check that given TIME structure has values
-  in valid ranges, otherwise size of the buffer could be not 
-  enough.
-*/
-#define MAX_DATE_REP_LENGTH 30
 
 enum enum_parsing_place
 {
@@ -1046,7 +1038,6 @@ void make_date(const DATE_TIME_FORMAT *format, const TIME *l_time,
                String *str);
 void make_time(const DATE_TIME_FORMAT *format, const TIME *l_time,
                String *str);
-void TIME_to_string(const TIME *time, String *str);
 ulonglong TIME_to_ulonglong_datetime(const TIME *time);
 ulonglong TIME_to_ulonglong_date(const TIME *time);
 ulonglong TIME_to_ulonglong_time(const TIME *time);

sql/protocol.cc

@@ -880,10 +880,9 @@ bool Protocol_simple::store_date(TIME *tm)
 	      field_types[field_pos] == MYSQL_TYPE_DATE);
   field_pos++;
 #endif
-  char buff[40];
+  char buff[MAX_DATE_STRING_REP_LENGTH];
-  String tmp((char*) buff,sizeof(buff),&my_charset_bin);
+  int length= my_date_to_str(tm, buff);
-  make_date((DATE_TIME_FORMAT *) 0, tm, &tmp);
+  return net_store_data(buff, (uint) length);
-  return net_store_data((char*) tmp.ptr(), tmp.length());
 }
 
 

sql/time.cc

@@ -747,13 +747,7 @@ const char *get_date_time_format_str(KNOWN_DATE_TIME_FORMAT *format,
 void make_time(const DATE_TIME_FORMAT *format __attribute__((unused)),
                const TIME *l_time, String *str)
 {
-  long length= my_sprintf((char*) str->ptr(),
+  uint length= (uint) my_time_to_str(l_time, (char*) str->ptr());
-			  ((char*) str->ptr(),
-			   "%s%02d:%02d:%02d",
-			   (l_time->neg ? "-" : ""),
-			   l_time->hour,
-			   l_time->minute,
-			   l_time->second));
   str->length(length);
   str->set_charset(&my_charset_bin);
 }
@@ -762,12 +756,7 @@ void make_time(const DATE_TIME_FORMAT *format __attribute__((unused)),
 void make_date(const DATE_TIME_FORMAT *format __attribute__((unused)),
                const TIME *l_time, String *str)
 {
-  long length= my_sprintf((char*) str->ptr(),
+  uint length= (uint) my_date_to_str(l_time, (char*) str->ptr());
-			  ((char*) str->ptr(),
-			   "%04d-%02d-%02d",
-			   l_time->year,
-			   l_time->month,
-			   l_time->day));
   str->length(length);
   str->set_charset(&my_charset_bin);
 }
@@ -776,15 +765,7 @@ void make_date(const DATE_TIME_FORMAT *format __attribute__((unused)),
 void make_datetime(const DATE_TIME_FORMAT *format __attribute__((unused)),
                    const TIME *l_time, String *str)
 {
-  long length= my_sprintf((char*) str->ptr(),
+  uint length= (uint) my_datetime_to_str(l_time, (char*) str->ptr());
-			  ((char*) str->ptr(),
-			   "%04d-%02d-%02d %02d:%02d:%02d",
-			   l_time->year,
-			   l_time->month,
-			   l_time->day,
-			   l_time->hour,
-			   l_time->minute,
-			   l_time->second));
   str->length(length);
   str->set_charset(&my_charset_bin);
 }
@@ -894,38 +875,4 @@ ulonglong TIME_to_ulonglong(const TIME *time)
   return 0;
 }
 
-
-/*
-  Convert struct DATE/TIME/DATETIME value to string using built-in
-  MySQL time conversion formats.
-
-  SYNOPSIS
-    TIME_to_string()
-
-  NOTE 
-    The string must have at least MAX_DATE_REP_LENGTH bytes reserved.
-*/
-
-void TIME_to_string(const TIME *time, String *str)
-{
-  switch (time->time_type) {
-  case MYSQL_TIMESTAMP_DATETIME:
-    make_datetime((DATE_TIME_FORMAT*) 0, time, str);
-    break;
-  case MYSQL_TIMESTAMP_DATE:
-    make_date((DATE_TIME_FORMAT*) 0, time, str);
-    break;
-  case MYSQL_TIMESTAMP_TIME:
-    make_time((DATE_TIME_FORMAT*) 0, time, str);
-    break;
-  case MYSQL_TIMESTAMP_NONE:
-  case MYSQL_TIMESTAMP_ERROR:
-    str->length(0);
-    str->set_charset(&my_charset_bin);
-    break;
-  default:
-    DBUG_ASSERT(0);
-  }
-}
-
 #endif

tests/client_test.c

@@ -10541,6 +10541,52 @@ static void test_bug5315()
 }
 
 
+static void test_bug6049()
+{
+  MYSQL_STMT *stmt;
+  MYSQL_BIND bind[1];
+  MYSQL_RES *res;
+  MYSQL_ROW row;
+  const char *stmt_text;
+  char *buffer[30];
+  ulong length;
+  int rc;
+
+  myheader("test_bug6049");
+
+  stmt_text= "SELECT MAKETIME(-25, 12, 12)";
+
+  rc= mysql_real_query(mysql, stmt_text, strlen(stmt_text));
+  myquery(rc);
+  res= mysql_store_result(mysql);
+  row= mysql_fetch_row(res);
+
+  stmt= mysql_stmt_init(mysql);
+  rc= mysql_stmt_prepare(stmt, stmt_text, strlen(stmt_text));
+  check_execute(stmt, rc);
+  rc= mysql_stmt_execute(stmt);
+  check_execute(stmt, rc);
+
+  bzero(bind, sizeof(bind));
+  bind[0].buffer_type    = MYSQL_TYPE_STRING;
+  bind[0].buffer         = &buffer;
+  bind[0].buffer_length  = sizeof(buffer);
+  bind[0].length         = &length;
+
+  mysql_stmt_bind_result(stmt, bind);
+  rc= mysql_stmt_fetch(stmt);
+  DIE_UNLESS(rc == 0);
+
+  printf("Result from query: %s\n", row[0]);
+  printf("Result from prepared statement: %s\n", buffer);
+
+  DIE_UNLESS(strcmp(row[0], buffer) == 0);
+
+  mysql_free_result(res);
+  mysql_stmt_close(stmt);
+}
+
+
 /*
   Read and parse arguments and MySQL options from my.cnf
 */
@@ -10851,6 +10897,7 @@ int main(int argc, char **argv)
     test_bug5194();         /* bulk inserts in prepared mode */
     test_bug5315();         /* check that mysql_change_user closes all
                                prepared statements */
+    test_bug6049();         /* check support for negative TIME values */
     /*
       XXX: PLEASE RUN THIS PROGRAM UNDER VALGRIND AND VERIFY THAT YOUR TEST
       DOESN'T CONTAIN WARNINGS/ERRORS BEFORE YOU PUSH.