Merge pull request #2033 from dhall-MariaDB/MCOL-4738

MCOL-4738 AVG() returns a wrong result

CMakeLists.txt

@@ -198,7 +198,9 @@ ELSE ()
     MY_CHECK_AND_SET_COMPILER_FLAG("-D_DEBUG -O0" DEBUG)
 ENDIF()
 
-
+IF (MASK_LONGDOUBLE)
+    MY_CHECK_AND_SET_COMPILER_FLAG("-DMASK_LONGDOUBLE")
+ENDIF()
 
 SET (ENGINE_LDFLAGS    "-Wl,--no-as-needed -Wl,--add-needed")
 SET (ENGINE_DT_LIB datatypes)

cmake/configureEngine.cmake

@@ -6,6 +6,7 @@
 INCLUDE (CheckIncludeFiles)
 INCLUDE (CheckIncludeFileCXX)
 INCLUDE (CheckCSourceCompiles)
+INCLUDE (CheckCXXSourceRuns)
 INCLUDE (CheckCXXSourceCompiles)
 INCLUDE (CheckStructHasMember)
 INCLUDE (CheckLibraryExists)
@@ -719,3 +720,20 @@ EXECUTE_PROCESS(
     COMMAND rm -f conftest.data conftest.file conftest.sym
     WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}
     )
+    
+CHECK_CXX_SOURCE_RUNS("
+#include <limits>
+int main()
+{
+    // If long double is 16 bytes and digits and exponent are 64 and 16384 respectively, then we need to mask out the 
+    // unused bits, as they contain garbage. There are times we test for equality by memcmp of a buffer containing,
+    // in part, the long double set here. Garbage bytes will adversly affect that compare.
+    // Note: There may be compilers that store 80 bit floats in 12 bytes. We do not account for that here. I don't believe
+    // there are any modern Linux compilers that do that as a default. Windows uses 64 bits, so no masking is needed.
+    if (std::numeric_limits<long double>::digits == 64 
+     && std::numeric_limits<long double>::max_exponent == 16384 
+     && sizeof(long double) == 16)
+        return 0;
+    return 1; 
+}"
+MASK_LONGDOUBLE)

utils/rowgroup/rowgroup.h

@@ -1369,12 +1369,10 @@ inline void Row::setFloatField(float val, uint32_t colIndex)
 inline void Row::setLongDoubleField(const long double& val, uint32_t colIndex)
 {
     uint8_t* p = &data[offsets[colIndex]];
-    *((long double*)p) = val;
+    *reinterpret_cast<long double*>(p) = val;
-    if (sizeof(long double) == 16)
+#ifdef MASK_LONGDOUBLE
-    {
+    *(reinterpret_cast<uint64_t*>(p)+1) &= 0x000000000000FFFFULL;
-        // zero out the unused portion as there may be garbage there.
+#endif    
-        *((uint64_t*)p+1) &= 0x000000000000FFFFULL;
-    }
 }
 
 inline void Row::setInt128Field(const int128_t& val, uint32_t colIndex)