Merge pull request #492 from mariadb-corporation/MCOL-392-fixes

Fix a bunch of issues around TIME data type

dbcon/mysql/ha_calpont_dml.cpp

@@ -895,6 +895,10 @@ int ha_calpont_impl_write_batch_row_(uchar* buf, TABLE* table, cal_impl_if::cal_
                         longlong tmp = my_time_packed_from_binary(pos, table->field[colpos]->decimals());
                         TIME_from_longlong_time_packed(&ltime, tmp);
 
+                        if (ltime.neg)
+                        {
+                            fprintf(ci.filePtr, "-");
+                        }
                         if (!ltime.second_part)
                         {
                             fprintf(ci.filePtr, "%02d:%02d:%02d%c",

dbcon/mysql/ha_calpont_impl.cpp

@@ -582,7 +582,7 @@ int fetchNextRow(uchar* buf, cal_table_info& ti, cal_connection_info* ci, bool h
                         *(*f)->null_ptr &= ~(*f)->null_bit;
 
                     intColVal = row.getUintField<8>(s);
-                    DataConvert::datetimeToString(intColVal, tmp, 255);
+                    DataConvert::datetimeToString(intColVal, tmp, 255, colType.precision);
 
                     /* setting the field_length is a sort-of hack. The length
                      * at this point can be long enough to include mseconds.
@@ -606,7 +606,7 @@ int fetchNextRow(uchar* buf, cal_table_info& ti, cal_connection_info* ci, bool h
                         *(*f)->null_ptr &= ~(*f)->null_bit;
 
                     intColVal = row.getUintField<8>(s);
-                    DataConvert::timeToString(intColVal, tmp, 255);
+                    DataConvert::timeToString(intColVal, tmp, 255, colType.precision);
 
                     Field_varstring* f2 = (Field_varstring*)*f;
                     f2->store(tmp, strlen(tmp), f2->charset());

utils/dataconvert/dataconvert.cpp

@@ -859,7 +859,7 @@ bool mysql_str_to_datetime( const string& input, DateTime& output, bool& isDate
     return true;
 }
 
-bool mysql_str_to_time( const string& input, Time& output )
+bool mysql_str_to_time( const string& input, Time& output, long decimals )
 {
     int32_t datesepct = 0;
     uint32_t dtend = 0;
@@ -999,20 +999,21 @@ bool mysql_str_to_time( const string& input, Time& output )
     if ( !isTimeValid( hour, min, sec, usec ) )
     {
         // Emulate MariaDB's time saturation
-        if (hour > 838)
+        // TODO: msec saturation
+        if ((hour > 838) && !isNeg)
         {
             output.hour = 838;
             output.minute = 59;
             output.second = 59;
-            output.msecond = 999999;
+            output.msecond = exp10(decimals) - 1;
             output.is_neg = 0;
         }
-        else if (hour < -838)
+        else if ((hour < -838) || ((hour > 838) && isNeg))
         {
             output.hour = -838;
             output.minute = 59;
             output.second = 59;
-            output.msecond = 999999;
+            output.msecond = exp10(decimals) - 1;
             output.is_neg = 1;
         }
         // If neither of the above match then we return a 0 time
@@ -1068,9 +1069,9 @@ bool stringToDatetimeStruct(const string& data, DateTime& dtime, bool* date)
     return true;
 }
 
-bool stringToTimeStruct(const string& data, Time& dtime)
+bool stringToTimeStruct(const string& data, Time& dtime, long decimals)
 {
-    if ( !mysql_str_to_time( data, dtime ) )
+    if ( !mysql_str_to_time( data, dtime, decimals ) )
         return false;
 
     return true;
@@ -1415,15 +1416,11 @@ DataConvert::convertColumnData(const CalpontSystemCatalog::ColType& colType,
             {
                 Time aTime;
 
-                if (stringToTimeStruct(data, aTime))
+                if (!stringToTimeStruct(data, aTime, colType.precision))
                 {
-                    value = (int64_t) * (reinterpret_cast<int64_t*>(&aTime));
-                }
-                else
-                {
-                    value = (int64_t) 0;
                     pushWarning = true;
                 }
+                value = (int64_t) * (reinterpret_cast<int64_t*>(&aTime));
             }
             break;
 
@@ -1910,6 +1907,7 @@ int64_t DataConvert::convertColumnTime(
 {
     status = 0;
     char* p;
+    char* retp = NULL;
     char* savePoint = NULL;
     p = const_cast<char*>(dataOrg);
     int64_t value = 0;
@@ -1926,6 +1924,17 @@ int64_t DataConvert::convertColumnTime(
         return value;
     }
 
+    if (dataOrgLen == 0)
+    {
+        return value;
+    }
+    if (dataOrgLen < 3)
+    {
+        // Not enough chars to be a time
+        status = -1;
+        return value;
+    }
+
     if (p[0] == '-')
     {
         isNeg = true;
@@ -1934,9 +1943,9 @@ int64_t DataConvert::convertColumnTime(
     errno = 0;
 
     p = strtok_r(p, ":.", &savePoint);
-    inHour = strtol(p, 0, 10);
+    inHour = strtol(p, &retp, 10);
 
-    if (errno)
+    if (errno || !retp)
     {
         status = -1;
         return value;
@@ -1950,9 +1959,9 @@ int64_t DataConvert::convertColumnTime(
         return value;
     }
 
-    inMinute = strtol(p, 0, 10);
+    inMinute = strtol(p, &retp, 10);
 
-    if (errno)
+    if (errno || !retp)
     {
         status = -1;
         return value;
@@ -1966,9 +1975,9 @@ int64_t DataConvert::convertColumnTime(
         return value;
     }
 
-    inSecond = strtol(p, 0, 10);
+    inSecond = strtol(p, &retp, 10);
 
-    if (errno)
+    if (errno || !retp)
     {
         status = -1;
         return value;
@@ -1978,9 +1987,9 @@ int64_t DataConvert::convertColumnTime(
 
     if (p != NULL)
     {
-        inMicrosecond = strtol(p, 0, 10);
+        inMicrosecond = strtol(p, &retp, 10);
 
-        if (errno)
+        if (errno || !retp)
         {
             status = -1;
             return value;
@@ -2082,13 +2091,8 @@ std::string DataConvert::datetimeToString( long long  datetimevalue, long decima
 
     if (dt.msecond && decimals)
     {
-        snprintf(buf + strlen(buf), 21 + decimals, ".%d", dt.msecond);
+        // Pad start with zeros
-
+        sprintf(buf + strlen(buf), ".%0*d", (int)decimals, dt.msecond);
-        // Pad end with zeros
-        if (strlen(buf) < (size_t)(21 + decimals))
-        {
-            sprintf(buf + strlen(buf), "%0*d", (int)(21 + decimals - strlen(buf)), 0);
-        }
     }
 
     return buf;
@@ -2118,14 +2122,8 @@ std::string DataConvert::timeToString( long long  timevalue, long decimals )
 
     if (dt.msecond && decimals)
     {
-        size_t start = strlen(buf);
+        // Pad start with zeros
-        snprintf(buf + strlen(buf), 12 + decimals, ".%d", dt.msecond);
+        sprintf(buf + strlen(buf), ".%0*d", (int)decimals, dt.msecond);
-
-        // Pad end with zeros
-        if (strlen(buf) - start < (size_t)decimals)
-        {
-            sprintf(buf + strlen(buf), "%0*d", (int)(decimals - (strlen(buf) - start) + 1), 0);
-        }
     }
 
     return buf;

utils/dataconvert/dataconvert.h

@@ -587,14 +587,7 @@ inline void DataConvert::datetimeToString( long long datetimevalue, char* buf, u
 
     if (msec || decimals)
     {
-        size_t start = strlen(buf);
+        snprintf(buf + strlen(buf), buflen - strlen(buf), ".%0*d", (int)decimals, msec);
-        snprintf(buf + strlen(buf), buflen - start, ".%d", msec);
-
-        // Pad end with zeros
-        if (strlen(buf) - start < (size_t)decimals)
-        {
-            snprintf(buf + strlen(buf), buflen - strlen(buf), "%0*d", (int)(decimals - (strlen(buf) - start) + 1), 0);
-        }
     }
 }
 
@@ -636,14 +629,8 @@ inline void DataConvert::timeToString( long long timevalue, char* buf, unsigned
 
     if (msec || decimals)
     {
-        size_t start = strlen(buf);
+        // Pad start with zeros
-        snprintf(buf + strlen(buf), buflen - start, ".%d", msec);
+        snprintf(buf + strlen(buf), buflen - strlen(buf), ".%0*d", (int)decimals, msec);
-
-        // Pad end with zeros
-        if (strlen(buf) - start < (size_t)decimals)
-        {
-            snprintf(buf + strlen(buf), buflen - strlen(buf), "%0*d", (int)(decimals - (strlen(buf) - start) + 1), 0);
-        }
     }
 }
 

utils/funcexp/func_add_time.cpp

@@ -223,7 +223,14 @@ int64_t Func_add_time::getIntVal(rowgroup::Row& row,
                                  bool& isNull,
                                  CalpontSystemCatalog::ColType& op_ct)
 {
-    return getDatetimeIntVal(row, parm, isNull, op_ct);
+    if (parm[0]->data()->resultType().colDataType == execplan::CalpontSystemCatalog::TIME)
+    {
+        return getTimeIntVal(row, parm, isNull, op_ct);
+    }
+    else
+    {
+        return getDatetimeIntVal(row, parm, isNull, op_ct);
+    }
 }
 
 string Func_add_time::getStrVal(rowgroup::Row& row,

utils/funcexp/func_dayname.cpp

@@ -145,7 +145,9 @@ string Func_dayname::getStrVal(rowgroup::Row& row,
                                bool& isNull,
                                CalpontSystemCatalog::ColType& op_ct)
 {
-    uint32_t weekday = getIntVal(row, parm, isNull, op_ct);
+    int32_t weekday = getIntVal(row, parm, isNull, op_ct);
+    if (weekday == -1)
+        return "";
     return helpers::weekdayFullNames[weekday];
 }
 

utils/funcexp/func_monthname.cpp

@@ -47,7 +47,9 @@ string Func_monthname::getStrVal(rowgroup::Row& row,
                                  bool& isNull,
                                  CalpontSystemCatalog::ColType& op_ct)
 {
-    uint32_t month = getIntVal(row, parm, isNull, op_ct);
+    int32_t month = getIntVal(row, parm, isNull, op_ct);
+    if (month == -1)
+        return "";
     return helpers::monthFullNames[month];
 }