Fix regression in a query involving an aggregate function on a

non-wide decimal column in the HAVING clause.

In buildAggregateColumn(), if an aggregate function (such as avg)
is applied on a non-wide decimal column, we were setting the precision
of the resulting column as -1. This later down in the execution got
converted to 255 as in some cases, precision is stored as uint8_t.
The predicate operations on a DECIMAL column has logic that uses
the wide Decimal::s128value field if precision > 18. This logic incorrectly
used the Decimal::s128value instead of the correct value stored in the
narrow Decimal::value field, since precision of the Decimal column
was 255. The fix is to set the aggregate column precision to
datatypes::INT64MAXPRECISION (18) in buildAggregateColumn() when the
aggregate is applied on a non-wide decimal column.

This commit also partially fixes -Wstrict-aliasing GCC warnings.

datatypes/mcs_decimal.h

@@ -85,8 +85,6 @@ constexpr uint8_t INT64MAXPRECISION = 18U;
 constexpr uint8_t INT128MAXPRECISION = 38U;
 constexpr uint8_t MAXLEGACYWIDTH = 8U;
 constexpr uint8_t MAXSCALEINC4AVG = 4U;
-constexpr int8_t IGNOREPRECISION = -1;
-
 
 
 const uint64_t mcs_pow_10[20] =

dbcon/mysql/ha_mcs_execplan.cpp

@@ -5068,7 +5068,7 @@ ReturnedColumn* buildAggregateColumn(Item* item, gp_walk_info& gwi)
                     {
                         ct.scale += datatypes::MAXSCALEINC4AVG;
                     }
-                    ct.precision = datatypes::IGNOREPRECISION;
+                    ct.precision = datatypes::INT64MAXPRECISION;
                 }
                 ac->resultType(ct);
             }

utils/common/hasher.h

@@ -353,7 +353,7 @@ class Hash128
 public:
     inline size_t operator()(const int128_t i) const
     {
-        return *reinterpret_cast<const uint64_t*>(&i);
+        return (uint64_t)((int64_t) i);
     }
 };
 

utils/dataconvert/dataconvert.cpp

@@ -105,6 +105,27 @@ bool number_value ( const string& data )
 namespace dataconvert
 {
 
+// LE stands for Little Endian
+uint32_t getUInt32LE(const char* ptr)
+{
+    return reinterpret_cast<const uint32_t*>(ptr)[0];
+}
+
+int32_t getSInt32LE(const char* ptr)
+{
+    return reinterpret_cast<const int32_t*>(ptr)[0];
+}
+
+uint64_t getUInt64LE(const char* ptr)
+{
+    return reinterpret_cast<const uint64_t*>(ptr)[0];
+}
+
+int64_t getSInt64LE(const char* ptr)
+{
+    return reinterpret_cast<const int64_t*>(ptr)[0];
+}
+
 template <typename T>
 void number_int_value(const string& data,
                       cscDataType typeCode,
@@ -1627,7 +1648,7 @@ DataConvert::StringToDate(const std::string& data, bool& pushWarning)
 
     if (stringToDateStruct(data, aDay))
     {
-        boost::any value = (*(reinterpret_cast<uint32_t*> (&aDay)));
+        boost::any value = getUInt32LE((const char*) &aDay);
         return value;
     }
     boost::any value = (uint32_t) 0;
@@ -1643,7 +1664,7 @@ DataConvert::StringToDatetime(const std::string& data, bool& pushWarning)
 
     if (stringToDatetimeStruct(data, aDatetime, 0)) // QQ: why 0?
     {
-        boost::any value = *(reinterpret_cast<uint64_t*>(&aDatetime));
+        boost::any value = getUInt64LE((const char*) &aDatetime);
         return value;
     }
     boost::any value = (uint64_t) 0;
@@ -1664,7 +1685,7 @@ DataConvert::StringToTime(const datatypes::SystemCatalog::TypeAttributesStd& col
         pushWarning = true;
     }
 
-    boost::any value = (int64_t) * (reinterpret_cast<int64_t*>(&aTime));
+    boost::any value = getSInt64LE((const char*) &aTime);
     return value;
 }
 
@@ -1681,7 +1702,7 @@ DataConvert::StringToTimestamp(const datatypes::ConvertFromStringParam &prm,
         pushWarning = true;
     }
 
-    boost::any value = (uint64_t) *(reinterpret_cast<uint64_t*>(&aTimestamp));
+    boost::any value = getUInt64LE((const char*) &aTimestamp);
     return value;
 }
 
@@ -2448,7 +2469,10 @@ int64_t DataConvert::stringToDate(const string& data)
     Date aDay;
 
     if ( stringToDateStruct( data, aDay ) )
-        return (((*(reinterpret_cast<uint32_t*> (&aDay))) & 0xFFFFFFC0) | 0x3E);
+    {
+        uint32_t temp = getUInt32LE((const char*) &aDay);
+        return ((temp & 0xFFFFFFC0) | 0x3E);
+    }
     else
         return -1;
 }
@@ -2458,7 +2482,7 @@ int64_t DataConvert::stringToDatetime(const string& data, bool* date)
     DateTime dtime;
 
     if ( stringToDatetimeStruct( data, dtime, date ) )
-        return *(reinterpret_cast<uint64_t*>(&dtime));
+        return getUInt64LE((const char*) &dtime);
     else
         return -1;
 }
@@ -2468,7 +2492,7 @@ int64_t DataConvert::stringToTimestamp(const string& data, const string& timeZon
     TimeStamp aTimestamp;
 
     if ( stringToTimestampStruct( data, aTimestamp, timeZone ) )
-        return *(reinterpret_cast<uint64_t*>(&aTimestamp));
+        return getUInt64LE((const char*) &aTimestamp);
     else
         return -1;
 }
@@ -2485,7 +2509,7 @@ int64_t DataConvert::intToDate(int64_t data)
         aday.year = 0;
         aday.month = 0;
         aday.day = 0;
-        return *(reinterpret_cast<uint32_t*>(&aday));
+        return getUInt32LE((const char*) &aday);
     }
 
     // this snprintf call causes a compiler warning b/c we're potentially copying a 20-digit #
@@ -2597,7 +2621,7 @@ int64_t DataConvert::intToDate(int64_t data)
     aday.year = y;
     aday.month = m;
     aday.day = d;
-    return *(reinterpret_cast<uint32_t*>(&aday));
+    return getUInt32LE((const char*) &aday);
 }
 
 /* This is really painful and expensive b/c it seems the input is not normalized or
@@ -2621,7 +2645,7 @@ int64_t DataConvert::intToDatetime(int64_t data, bool* date)
         if (date)
             *date = true;
 
-        return *(reinterpret_cast<uint64_t*>(&adaytime));
+        return getUInt64LE((const char*) &adaytime);
     }
 
     // this snprintf call causes a compiler warning b/c we're potentially copying a 20-digit #
@@ -2747,7 +2771,7 @@ int64_t DataConvert::intToDatetime(int64_t data, bool* date)
     if (date)
         *date = isDate;
 
-    return *(reinterpret_cast<uint64_t*>(&adaytime));
+    return getUInt64LE((const char*) &adaytime);
 }
 
 /* This is really painful and expensive b/c it seems the input is not normalized or
@@ -2767,7 +2791,7 @@ int64_t DataConvert::intToTime(int64_t data, bool fromString)
         atime.msecond = 0;
         atime.is_neg = 0;
 
-        return *(reinterpret_cast<int64_t*>(&atime));
+        return getSInt64LE((const char*) &atime);
     }
 
     // this snprintf call causes a compiler warning b/c we're potentially copying a 20-digit #
@@ -2871,7 +2895,7 @@ int64_t DataConvert::intToTime(int64_t data, bool fromString)
     atime.msecond = ms;
     atime.is_neg = isNeg;
 
-    return *(reinterpret_cast<int64_t*>(&atime));
+    return getSInt64LE((const char*) &atime);
 }
 
 int64_t DataConvert::stringToTime(const string& data)
@@ -2991,7 +3015,7 @@ int64_t DataConvert::stringToTime(const string& data)
     atime.second = sec;
     atime.msecond = msec;
     atime.is_neg = isNeg;
-    return *(reinterpret_cast<int64_t*>(&atime));
+    return getSInt64LE((const char*) &atime);
 }