MCOL-641 This commit adds support for group_concat w/o ORDER BY.

Small refactoring in Row methods.

dbcon/joblist/groupconcat.cpp

@@ -56,6 +56,7 @@ using namespace ordering;
 #include "jobstep.h"
 #include "jlf_common.h"
 #include "limitedorderby.h"
+#include "mcs_decimal.h"
 
 namespace joblist
 {
@@ -368,7 +369,7 @@ void GroupConcatAgUM::applyMapping(const boost::shared_array<int>& mapping, cons
     // For some reason the rowgroup mapping fcns don't work right in this class.
     for (uint64_t i = 0; i < fRow.getColumnCount(); i++)
     {
-        if (fRow.getColumnWidth(i) > 8)
+        if (fRow.getColumnWidth(i) > datatypes::MAXLEGACYWIDTH)
         {
             if (fRow.getColTypes()[i] == execplan::CalpontSystemCatalog::CHAR ||
                 fRow.getColTypes()[i] == execplan::CalpontSystemCatalog::VARCHAR ||
@@ -380,6 +381,10 @@ void GroupConcatAgUM::applyMapping(const boost::shared_array<int>& mapping, cons
             {
                 fRow.setLongDoubleField(row.getLongDoubleField(mapping[i]), i);
             }
+            else if (datatypes::Decimal::isWideDecimalType(fRow.getColType(i), fRow.getColumnWidth(i)))
+            {
+                row.copyBinaryField<int128_t>(fRow, i, mapping[i]);
+            }
         }
         else
         {
@@ -440,25 +445,34 @@ void GroupConcator::outputRow(std::ostringstream& oss, const rowgroup::Row& row)
             case CalpontSystemCatalog::MEDINT:
             case CalpontSystemCatalog::INT:
             case CalpontSystemCatalog::BIGINT:
-            case CalpontSystemCatalog::DECIMAL:
-            case CalpontSystemCatalog::UDECIMAL:
             {
                 int64_t intVal = row.getIntField(*i);
-                int scale = (int) row.getScale(*i);
 
-                if (scale == 0)
+                oss << intVal;
-                {
-                    oss << intVal;
-                }
-                else
-                {
-                    long double dblVal = intVal / pow(10.0, (double)scale);
-                    oss << fixed << setprecision(scale) << dblVal;
-                }
 
                 break;
             }
 
+            case CalpontSystemCatalog::DECIMAL:
+            case CalpontSystemCatalog::UDECIMAL:
+            {
+                int scale = (int) row.getScale(*i);
+                //{
+                //    long double dblVal = intVal / pow(10.0, (double)scale);
+                //    oss << fixed << setprecision(scale) << dblVal;
+                //}
+                char buf[utils::MAXLENGTH16BYTES];
+ 
+                int128_t* dec = row.getBinaryField<int128_t>(*i);
+                dataconvert::DataConvert::decimalToString(dec,
+                    static_cast<uint32_t>(scale), buf,
+                    sizeof(buf), types[*i]);
+                oss << fixed << buf;
+
+                break;
+            }
+
+
             case CalpontSystemCatalog::UTINYINT:
             case CalpontSystemCatalog::USMALLINT:
             case CalpontSystemCatalog::UMEDINT:
@@ -609,27 +623,7 @@ int64_t GroupConcator::lengthEstimate(const rowgroup::Row& row)
             case CalpontSystemCatalog::DECIMAL:
             case CalpontSystemCatalog::UDECIMAL:
             {
-                int64_t v = row.getIntField(*i);
+                fieldLen += row.getPrecision(*i)/2;
-                double scale = row.getScale(*i);
-
-                if (scale > 0)
-                {
-                    v /= (int64_t) pow(10.0, scale);
-
-                    if (v < 0) fieldLen++;
-
-                    while ((v /= 10) != 0) fieldLen++;
-
-                    fieldLen += (int64_t) scale + 2;;
-                }
-                else
-                {
-                    if (v < 0) fieldLen++;
-
-                    while ((v /= 10) != 0) fieldLen++;
-
-                    fieldLen += 1;;
-                }
 
                 break;
             }
@@ -1092,8 +1086,10 @@ void GroupConcatNoOrder::getResult(uint8_t* buff, const string& sep)
     }
 
     size_t resultSize = oss.str().size();
+    resultSize = (resultSize > fGroupConcatLen) ? fGroupConcatLen : resultSize;
     fOutputString.reset(new uint8_t[resultSize + 2]);
-    memset(fOutputString.get(), 0, resultSize + 2);
+    fOutputString[resultSize] = '\0';
+    fOutputString[resultSize + 1] = '\0';
 
     strncpy((char*)fOutputString.get(),
         oss.str().c_str(), resultSize);

tests/rowgroup-tests.cpp

@@ -291,8 +291,8 @@ TEST_F(RowDecimalTest, CopyBinaryFieldCheck)
         col2Out = rOut.getBinaryField<uint128_t>(1);
         EXPECT_NE(*col1In, *col1Out);
         EXPECT_NE(*col2In, *col2Out);
-        r.copyBinaryField(rOut, 0, 0);
+        r.copyBinaryField<int128_t>(rOut, 0, 0);
-        r.copyBinaryField(rOut, 1, 1);
+        r.copyBinaryField<int128_t>(rOut, 1, 1);
         col1Out = rOut.getBinaryField<int128_t>(0);
         col2Out = rOut.getBinaryField<uint128_t>(1);
         EXPECT_EQ(*col1In, *col1Out);

utils/rowgroup/rowgroup.h

@@ -429,6 +429,8 @@ public:
     template<typename T>
     inline void setBinaryField(T* strdata, uint32_t width, uint32_t colIndex);
     template<typename T>
+    inline void setBinaryField(T* strdata, uint32_t colIndex);
+    template<typename T>
     inline void setBinaryField_offset(T* strdata, uint32_t width, uint32_t colIndex);
     // support VARBINARY
     // Add 2-byte length at the CHARSET_INFO*beginning of the field.  NULL and zero length field are
@@ -475,7 +477,7 @@ public:
     // that's not string-table safe, this one is
     inline void copyField(Row& dest, uint32_t destIndex, uint32_t srcIndex) const;
 
-    // WIP MCOL-641
+    template<typename T>
     inline void copyBinaryField(Row& dest, uint32_t destIndex, uint32_t srcIndex) const;
 
     std::string toString() const;
@@ -816,6 +818,12 @@ inline void Row::setBinaryField(T* value, uint32_t width, uint32_t colIndex)
     memcpy(&data[offsets[colIndex]], value, width);
 }
 
+template<typename T>
+inline void Row::setBinaryField(T* value, uint32_t colIndex)
+{
+    *reinterpret_cast<T*>(&data[offsets[colIndex]]) = *value;
+}
+
 // This method !cannot! be applied to uint8_t* buffers.
 template<typename T>
 inline void Row::setBinaryField_offset(T* value, uint32_t width, uint32_t offset)
@@ -1221,7 +1229,7 @@ inline void Row::copyField(Row& out, uint32_t destIndex, uint32_t srcIndex) cons
     else if (UNLIKELY(datatypes::Decimal::isWideDecimalType(
         types[srcIndex], colWidths[srcIndex])))
     {
-        copyBinaryField(out, destIndex, srcIndex);
+        copyBinaryField<int128_t>(out, destIndex, srcIndex);
     }
     else
     {
@@ -1229,9 +1237,10 @@ inline void Row::copyField(Row& out, uint32_t destIndex, uint32_t srcIndex) cons
     }
 }
 
+template<typename T>
 inline void Row::copyBinaryField(Row& out, uint32_t destIndex, uint32_t srcIndex) const
 {
-    out.setBinaryField(getBinaryField<int128_t>(srcIndex), 16, destIndex);
+    out.setBinaryField(getBinaryField<T>(srcIndex), destIndex);
 }
 
 inline void Row::setRid(uint64_t rid)
@@ -1953,7 +1962,7 @@ inline void copyRow(const Row& in, Row* out, uint32_t colCount)
         else if (UNLIKELY(datatypes::Decimal::isWideDecimalType(
             in.getColType(i), in.getColumnWidth(i))))
         {
-            in.copyBinaryField(*out, i, i);
+            in.copyBinaryField<int128_t>(*out, i, i);
         }
         else
         {