Merge pull request #2874 from phoeinx/mcol-4632-4648-int-casts

Fix MCOL 4632 and 4648 and further issues when casting to [U]BIGINT.
mariadb-corporation · Aug 11, 2023 · c97c8b6 · c97c8b6
2 parents c672edf + 48562e4
commit c97c8b6
Show file tree

Hide file tree

Showing 12 changed files with 280 additions and 94 deletions.
diff --git a/datatypes/mcs_datatype.h b/datatypes/mcs_datatype.h
@@ -21,6 +21,7 @@
 #include <boost/any.hpp>
 #include "exceptclasses.h"
 #include "conststring.h"
+#include "mcs_datatype_basic.h"
 #include "mcs_numeric_limits.h"
 #include "mcs_data_condition.h"
 #include "mcs_decimal.h"
@@ -34,46 +35,6 @@ typedef int32_t mcs_sint32_t;
 struct charset_info_st;
 typedef const struct charset_info_st CHARSET_INFO;
 
-
-namespace
-{
-const int64_t MIN_TINYINT __attribute__((unused)) = std::numeric_limits<int8_t>::min() + 2;    // -126;
-const int64_t MAX_TINYINT __attribute__((unused)) = std::numeric_limits<int8_t>::max();        //  127;
-const int64_t MIN_SMALLINT __attribute__((unused)) = std::numeric_limits<int16_t>::min() + 2;  // -32766;
-const int64_t MAX_SMALLINT __attribute__((unused)) = std::numeric_limits<int16_t>::max();      //  32767;
-const int64_t MIN_MEDINT __attribute__((unused)) = -(1ULL << 23);                              // -8388608;
-const int64_t MAX_MEDINT __attribute__((unused)) = (1ULL << 23) - 1;                           //  8388607;
-const int64_t MIN_INT __attribute__((unused)) = std::numeric_limits<int32_t>::min() + 2;       // -2147483646;
-const int64_t MAX_INT __attribute__((unused)) = std::numeric_limits<int32_t>::max();           //  2147483647;
-const int64_t MIN_BIGINT __attribute__((unused)) =
-    std::numeric_limits<int64_t>::min() + 2;  // -9223372036854775806LL;
-const int64_t MAX_BIGINT __attribute__((unused)) =
-    std::numeric_limits<int64_t>::max();  //  9223372036854775807
-
-const uint64_t MIN_UINT __attribute__((unused)) = 0;
-const uint64_t MIN_UTINYINT __attribute__((unused)) = 0;
-const uint64_t MIN_USMALLINT __attribute__((unused)) = 0;
-const uint64_t MIN_UMEDINT __attribute__((unused)) = 0;
-const uint64_t MIN_UBIGINT __attribute__((unused)) = 0;
-const uint64_t MAX_UINT __attribute__((unused)) = std::numeric_limits<uint32_t>::max() - 2;     // 4294967293
-const uint64_t MAX_UTINYINT __attribute__((unused)) = std::numeric_limits<uint8_t>::max() - 2;  // 253;
-const uint64_t MAX_USMALLINT __attribute__((unused)) = std::numeric_limits<uint16_t>::max() - 2;  // 65533;
-const uint64_t MAX_UMEDINT __attribute__((unused)) = (1ULL << 24) - 1;                            // 16777215
-const uint64_t MAX_UBIGINT __attribute__((unused)) =
-    std::numeric_limits<uint64_t>::max() - 2;  // 18446744073709551613
-
-const float MAX_FLOAT __attribute__((unused)) = std::numeric_limits<float>::max();  // 3.402823466385289e+38
-const float MIN_FLOAT __attribute__((unused)) = -std::numeric_limits<float>::max();
-const double MAX_DOUBLE __attribute__((unused)) =
-    std::numeric_limits<double>::max();  // 1.7976931348623157e+308
-const double MIN_DOUBLE __attribute__((unused)) = -std::numeric_limits<double>::max();
-const long double MAX_LONGDOUBLE __attribute__((unused)) =
-    std::numeric_limits<long double>::max();  // 1.7976931348623157e+308
-const long double MIN_LONGDOUBLE __attribute__((unused)) = -std::numeric_limits<long double>::max();
-
-const uint64_t AUTOINCR_SATURATED __attribute__((unused)) = std::numeric_limits<uint64_t>::max();
-}  // namespace
-
 using namespace std;  // e.g. string
 
 namespace ddlpackage

diff --git a/datatypes/mcs_datatype_basic.h b/datatypes/mcs_datatype_basic.h
@@ -18,11 +18,50 @@
 */
 #pragma once
 
+#include <limits>
+#include "joblisttypes.h"
+
 /*
   This file contains simple definitions that can be
   needed in multiple mcs_TYPE.h files.
 */
 
+namespace
+{
+const int64_t MIN_TINYINT __attribute__((unused)) = std::numeric_limits<int8_t>::min() + 2;    // -126;
+const int64_t MAX_TINYINT __attribute__((unused)) = std::numeric_limits<int8_t>::max();        //  127;
+const int64_t MIN_SMALLINT __attribute__((unused)) = std::numeric_limits<int16_t>::min() + 2;  // -32766;
+const int64_t MAX_SMALLINT __attribute__((unused)) = std::numeric_limits<int16_t>::max();      //  32767;
+const int64_t MIN_MEDINT __attribute__((unused)) = -(1ULL << 23);                              // -8388608;
+const int64_t MAX_MEDINT __attribute__((unused)) = (1ULL << 23) - 1;                           //  8388607;
+const int64_t MIN_INT __attribute__((unused)) = std::numeric_limits<int32_t>::min() + 2;       // -2147483646;
+const int64_t MAX_INT __attribute__((unused)) = std::numeric_limits<int32_t>::max();           //  2147483647;
+const int64_t MIN_BIGINT = std::numeric_limits<int64_t>::min() + 2;  // -9223372036854775806LL;
+const int64_t MAX_BIGINT = std::numeric_limits<int64_t>::max();      //  9223372036854775807
+
+const uint64_t MIN_UINT __attribute__((unused)) = 0;
+const uint64_t MIN_UTINYINT __attribute__((unused)) = 0;
+const uint64_t MIN_USMALLINT __attribute__((unused)) = 0;
+const uint64_t MIN_UMEDINT __attribute__((unused)) = 0;
+const uint64_t MIN_UBIGINT = 0;
+const uint64_t MAX_UINT __attribute__((unused)) = std::numeric_limits<uint32_t>::max() - 2;     // 4294967293
+const uint64_t MAX_UTINYINT __attribute__((unused)) = std::numeric_limits<uint8_t>::max() - 2;  // 253;
+const uint64_t MAX_USMALLINT __attribute__((unused)) = std::numeric_limits<uint16_t>::max() - 2;  // 65533;
+const uint64_t MAX_UMEDINT __attribute__((unused)) = (1ULL << 24) - 1;                            // 16777215
+const uint64_t MAX_UBIGINT = std::numeric_limits<uint64_t>::max() - 2;  // 18446744073709551613
+
+const float MAX_FLOAT __attribute__((unused)) = std::numeric_limits<float>::max();  // 3.402823466385289e+38
+const float MIN_FLOAT __attribute__((unused)) = -std::numeric_limits<float>::max();
+const double MAX_DOUBLE __attribute__((unused)) =
+    std::numeric_limits<double>::max();  // 1.7976931348623157e+308
+const double MIN_DOUBLE __attribute__((unused)) = -std::numeric_limits<double>::max();
+const long double MAX_LONGDOUBLE __attribute__((unused)) =
+    std::numeric_limits<long double>::max();  // 1.7976931348623157e+308
+const long double MIN_LONGDOUBLE __attribute__((unused)) = -std::numeric_limits<long double>::max();
+
+const uint64_t AUTOINCR_SATURATED __attribute__((unused)) = std::numeric_limits<uint64_t>::max();
+}  // namespace
+
 namespace datatypes
 {
 // Convert a positive floating point value to
@@ -57,9 +96,14 @@ template <typename SRC>
 int64_t xFloatToMCSSInt64Round(SRC value)
 {
   if (value > 0)
-    return positiveXFloatToXIntRound<SRC, int64_t>(value, numeric_limits<int64_t>::max());
+  {
+    return positiveXFloatToXIntRound<SRC, int64_t>(value, MAX_BIGINT);
+  }
   if (value < 0)
-    return negativeXFloatToXIntRound<SRC, int64_t>(value, numeric_limits<int64_t>::min() + 2);
+  {
+    //@Bug 4632 and 4648: Don't return marker value for NULL, but allow return of marker value for EMPTYROW.
+    return negativeXFloatToXIntRound<SRC, int64_t>(value, joblist::BIGINTEMPTYROW);
+  }
   return 0;
 }
 
@@ -69,12 +113,17 @@ template <typename SRC>
 uint64_t xFloatToMCSUInt64Round(SRC value)
 {
   if (value > 0)
-    return positiveXFloatToXIntRound<SRC, uint64_t>(value, numeric_limits<uint64_t>::max() - 2);
+  {
+    //@Bug 4632 and 4648: Don't return marker value for NULL, but allow return of marker value for EMPTYROW.
+    const auto val = positiveXFloatToXIntRound<SRC, uint64_t>(value, joblist::UBIGINTEMPTYROW);
+    return val == joblist::UBIGINTNULL ? MAX_UBIGINT : val;
+  }
   if (value < 0)
-    return negativeXFloatToXIntRound<SRC, uint64_t>(value, 0);
+  {
+    return negativeXFloatToXIntRound<SRC, uint64_t>(value, MIN_UBIGINT);
+  }
 
   return 0;
 }
 
 }  // end of namespace datatypes
-
diff --git a/datatypes/mcs_decimal.h b/datatypes/mcs_decimal.h
@@ -185,7 +185,6 @@ const long long columnstore_precision[19] = {0,
                                              99999999999999999LL,
                                              999999999999999999LL};
 
-
 const int128_t ConversionRangeMaxValue[20] = {9999999999999999999_xxl,
                                               99999999999999999999_xxl,
                                               999999999999999999999_xxl,
@@ -667,6 +666,18 @@ class Decimal : public TDecimal128, public TDecimal64
                                                    : TDecimal64::toUInt64Round((uint32_t)scale);
   }
 
+  int64_t toMCSInt64Round() const
+  {
+    //@Bug 4632 and 4648: Don't return marker value for NULL, but allow return of marker value for EMPTYROW.
+    return std::max(toSInt64Round(), static_cast<int64_t>(joblist::BIGINTEMPTYROW));
+  }
+  uint64_t toMCSUInt64Round() const
+  {
+    const auto val = toUInt64Round();
+    //@Bug 4632 and 4648: Don't return marker value for NULL, but allow return of marker value for EMPTYROW.
+    return val == joblist::UBIGINTNULL ? MAX_UBIGINT : val;
+  }
+
   // FLOOR related routines
   int64_t toSInt64Floor() const
   {

diff --git a/datatypes/mcs_int64.h b/datatypes/mcs_int64.h
@@ -18,7 +18,9 @@
 */
 #pragma once
 
+#include <algorithm>
 #include "exceptclasses.h"
+#include "mcs_datatype_basic.h"
 
 namespace datatypes
 {
@@ -60,6 +62,18 @@ class TUInt64
   {
     *(uint64_t*)dst = mValue;
   }
+
+  uint64_t toMCSUInt64() const
+  {
+    //@Bug 4632 and 4648: Don't return marker value for NULL, but allow return of marker value for EMPTYROW.
+    return mValue == joblist::UBIGINTNULL ? MAX_UBIGINT : mValue;
+  }
+
+  int64_t toMCSInt64() const
+  {
+    //@Bug 4632 and 4648: Don't return marker value for NULL, but allow return of marker value for EMPTYROW.
+    return std::max(static_cast<int64_t>(mValue), static_cast<int64_t>(joblist::BIGINTEMPTYROW));
+  }
 };
 
 class TSInt64
@@ -84,6 +98,16 @@ class TSInt64
   {
     return mValue < 0 ? 0 : static_cast<uint64_t>(mValue);
   }
+
+  uint64_t toMCSUInt64() const
+  {
+    return static_cast<uint64_t>(mValue);
+  }
+
+  int64_t toMCSInt64() const
+  {
+    return std::max(static_cast<int64_t>(mValue), static_cast<int64_t>(joblist::BIGINTEMPTYROW));
+  }
 };
 
 class TUInt64Null : public TUInt64, public TNullFlag
@@ -178,4 +202,3 @@ class TSInt64Null : public TSInt64, public TNullFlag
 };
 
 }  // end of namespace datatypes
-
diff --git a/dbcon/ddlpackageproc/altertableprocessor.cpp b/dbcon/ddlpackageproc/altertableprocessor.cpp
@@ -424,7 +424,6 @@ AlterTableProcessor::DDLResult AlterTableProcessor::processPackage(
           abs_ts.tv_nsec = rm_ts.tv_nsec;
         } while (nanosleep(&abs_ts, &rm_ts) < 0);
 
-
         try
         {
           processID = ::getpid();
@@ -939,7 +938,7 @@ void AlterTableProcessor::addColumn(uint32_t sessionID, execplan::CalpontSystemC
     createWriteDropLogFile(ropair.objnum, uniqueId, oidList);
 
     //@Bug 1358,1427 Always use the first column in the table, not the first one in columnlist to prevent
-    //random result
+    // random result
     //@Bug 4182. Use widest column as reference column
 
     // Find the widest column

diff --git a/mysql-test/columnstore/basic/r/mcol641-functions.result b/mysql-test/columnstore/basic/r/mcol641-functions.result
@@ -1112,8 +1112,8 @@ SELECT "convert_test", CONVERT(d1, SIGNED), CONVERT(d2, SIGNED), CONVERT(d3, SIG
 convert_test	CONVERT(d1, SIGNED)	CONVERT(d2, SIGNED)	CONVERT(d3, SIGNED)
 convert_test	NULL	NULL	NULL
 convert_test	0	0	0
-convert_test	NULL	NULL	0
-convert_test	NULL	NULL	0
+convert_test	-9223372036854775807	-9223372036854775807	0
+convert_test	-9223372036854775807	-9223372036854775807	0
 convert_test	-123456	-123	0
 convert_test	123456	123	0
 convert_test	9223372036854775807	9223372036854775807	0

diff --git a/mysql-test/columnstore/bugfixes/mcol_4632.result b/mysql-test/columnstore/bugfixes/mcol_4632.result
@@ -0,0 +1,41 @@
+#
+# MCOL-4632: Disallow casting to NULL marker values, allow casting to EMPTY ROW values.
+#
+# Defined in joblisttypes.h: BIGINTNULL = 0x8000000000000000ULL == -9223372036854775808 (when casted to int64_t)
+# Defined in joblisttypes.h: BIGINTEMPTYROW = 0x8000000000000001ULL == -9223372036854775807 (when casted to int64_t)
+#
+DROP DATABASE IF EXISTS mcol_4632;
+CREATE DATABASE mcol_4632;
+USE mcol_4632;
+CREATE TABLE t1 
+(d1 DECIMAL(30,0), d2 DECIMAL(30,0) NOT NULL,
+i1 BIGINT UNSIGNED, i2 BIGINT UNSIGNED NOT NULL,
+str1 TEXT, str2 TEXT NOT NULL,
+double1 DOUBLE(30,0), double2 DOUBLE(30,0) NOT NULL) ENGINE=ColumnStore;
+INSERT INTO t1 VALUES 
+(-9223372036854775808, -9223372036854775808,
+9223372036854775808, 9223372036854775808,
+"-9223372036854775808", "-9223372036854775808",
+-9223372036854775808.0, -9223372036854775808.0);
+INSERT INTO t1 VALUES 
+(-9223372036854775807, -9223372036854775807,
+9223372036854775809, 9223372036854775809,
+"-9223372036854775807", "-9223372036854775807",
+-9223372036854775807.0, -9223372036854775807.0);
+SELECT d1, CAST(d1 AS SIGNED), CAST(d2 AS SIGNED) FROM t1;
+d1	CAST(d1 AS SIGNED)	CAST(d2 AS SIGNED)
+-9223372036854775808	-9223372036854775807	-9223372036854775807
+-9223372036854775807	-9223372036854775807	-9223372036854775807
+SELECT i1, CAST(i1 AS SIGNED), CAST(i2 AS SIGNED) FROM t1;
+i1	CAST(i1 AS SIGNED)	CAST(i2 AS SIGNED)
+9223372036854775808	-9223372036854775807	-9223372036854775807
+9223372036854775809	-9223372036854775807	-9223372036854775807
+SELECT str1, CAST(str1 AS SIGNED), CAST(str2 AS SIGNED) FROM t1;
+str1	CAST(str1 AS SIGNED)	CAST(str2 AS SIGNED)
+-9223372036854775808	-9223372036854775807	-9223372036854775807
+-9223372036854775807	-9223372036854775807	-9223372036854775807
+SELECT double1, CAST(double1 AS SIGNED), CAST(double2 AS SIGNED) FROM t1;
+double1	CAST(double1 AS SIGNED)	CAST(double2 AS SIGNED)
+-9223372036854776000	-9223372036854775807	-9223372036854775807
+-9223372036854776000	-9223372036854775807	-9223372036854775807
+DROP DATABASE mcol_4632;
diff --git a/mysql-test/columnstore/bugfixes/mcol_4632.test b/mysql-test/columnstore/bugfixes/mcol_4632.test
@@ -0,0 +1,32 @@
+--source ../include/have_columnstore.inc
+--echo #
+--echo # MCOL-4632: Disallow casting to NULL marker values, allow casting to EMPTY ROW values.
+--echo #
+--echo # Defined in joblisttypes.h: BIGINTNULL = 0x8000000000000000ULL == -9223372036854775808 (when casted to int64_t)
+--echo # Defined in joblisttypes.h: BIGINTEMPTYROW = 0x8000000000000001ULL == -9223372036854775807 (when casted to int64_t)
+--echo #
+--disable_warnings
+DROP DATABASE IF EXISTS mcol_4632;
+--enable_warnings
+CREATE DATABASE mcol_4632;
+USE mcol_4632;
+CREATE TABLE t1 
+     (d1 DECIMAL(30,0), d2 DECIMAL(30,0) NOT NULL,
+	 i1 BIGINT UNSIGNED, i2 BIGINT UNSIGNED NOT NULL,
+     str1 TEXT, str2 TEXT NOT NULL,
+     double1 DOUBLE(30,0), double2 DOUBLE(30,0) NOT NULL) ENGINE=ColumnStore;
+INSERT INTO t1 VALUES 
+	 (-9223372036854775808, -9223372036854775808,
+     9223372036854775808, 9223372036854775808,
+     "-9223372036854775808", "-9223372036854775808",
+     -9223372036854775808.0, -9223372036854775808.0);
+INSERT INTO t1 VALUES 
+	 (-9223372036854775807, -9223372036854775807,
+     9223372036854775809, 9223372036854775809,
+     "-9223372036854775807", "-9223372036854775807",
+     -9223372036854775807.0, -9223372036854775807.0);
+SELECT d1, CAST(d1 AS SIGNED), CAST(d2 AS SIGNED) FROM t1;
+SELECT i1, CAST(i1 AS SIGNED), CAST(i2 AS SIGNED) FROM t1;
+SELECT str1, CAST(str1 AS SIGNED), CAST(str2 AS SIGNED) FROM t1;
+SELECT double1, CAST(double1 AS SIGNED), CAST(double2 AS SIGNED) FROM t1;
+DROP DATABASE mcol_4632;
diff --git a/mysql-test/columnstore/bugfixes/mcol_4648.result b/mysql-test/columnstore/bugfixes/mcol_4648.result
@@ -0,0 +1,36 @@
+#
+# MCOL-4648: Disallow casting to NULL marker values, allow casting to EMPTY ROW values.
+#
+# Defined in joblisttypes.h: UBIGINTNULL == 0xFFFFFFFFFFFFFFFEULL == 18446744073709551614
+# Defined in joblisttypes.h: UBIGINTEMPTYROW == 0xFFFFFFFFFFFFFFFFULL == 18446744073709551615
+#
+DROP DATABASE IF EXISTS mcol_4648;
+CREATE DATABASE mcol_4648;
+USE mcol_4648;
+CREATE TABLE t1 
+(d1 DECIMAL(30,0), d2 DECIMAL(30,0) NOT NULL,
+str1 TEXT, str2 TEXT NOT NULL,
+double1 DOUBLE(30,0), double2 DOUBLE(30,0) NOT NULL) ENGINE=ColumnStore;
+INSERT INTO t1 VALUES 
+(18446744073709551614, 18446744073709551614,
+"18446744073709551614", "18446744073709551614",
+18446744073709551614.0, 18446744073709551614.0);
+INSERT INTO t1 VALUES
+(18446744073709551615, 18446744073709551615,
+"18446744073709551615", "18446744073709551615",
+18446744073709551615.0, 18446744073709551615.0);
+SELECT d1, CAST(d1 AS UNSIGNED), CAST(d2 AS UNSIGNED) FROM t1;
+d1	CAST(d1 AS UNSIGNED)	CAST(d2 AS UNSIGNED)
+18446744073709551614	18446744073709551613	18446744073709551613
+18446744073709551615	18446744073709551615	18446744073709551615
+SELECT str1, CAST(str1 AS UNSIGNED), CAST(str2 AS UNSIGNED) FROM t1;
+str1	CAST(str1 AS UNSIGNED)	CAST(str2 AS UNSIGNED)
+18446744073709551614	18446744073709551613	18446744073709551613
+18446744073709551615	18446744073709551615	18446744073709551615
+# Doubles only store about 15 digits of precision for decimal places, therefore both inserted numbers
+# are rounded to 18446744073709552000.0 and we expect them to be casted to EMPTYROW value each.
+SELECT double1, CAST(double1 AS UNSIGNED), CAST(double2 AS UNSIGNED) FROM t1;
+double1	CAST(double1 AS UNSIGNED)	CAST(double2 AS UNSIGNED)
+18446744073709552000	18446744073709551615	18446744073709551615
+18446744073709552000	18446744073709551615	18446744073709551615
+DROP DATABASE mcol_4648;
diff --git a/mysql-test/columnstore/bugfixes/mcol_4648.test b/mysql-test/columnstore/bugfixes/mcol_4648.test
@@ -0,0 +1,30 @@
+--source ../include/have_columnstore.inc
+--echo #
+--echo # MCOL-4648: Disallow casting to NULL marker values, allow casting to EMPTY ROW values.
+--echo #
+--echo # Defined in joblisttypes.h: UBIGINTNULL == 0xFFFFFFFFFFFFFFFEULL == 18446744073709551614
+--echo # Defined in joblisttypes.h: UBIGINTEMPTYROW == 0xFFFFFFFFFFFFFFFFULL == 18446744073709551615
+--echo #
+--disable_warnings
+DROP DATABASE IF EXISTS mcol_4648;
+--enable_warnings
+CREATE DATABASE mcol_4648;
+USE mcol_4648;
+CREATE TABLE t1 
+    (d1 DECIMAL(30,0), d2 DECIMAL(30,0) NOT NULL,
+     str1 TEXT, str2 TEXT NOT NULL,
+     double1 DOUBLE(30,0), double2 DOUBLE(30,0) NOT NULL) ENGINE=ColumnStore;
+INSERT INTO t1 VALUES 
+    (18446744073709551614, 18446744073709551614,
+    "18446744073709551614", "18446744073709551614",
+     18446744073709551614.0, 18446744073709551614.0);
+INSERT INTO t1 VALUES
+	(18446744073709551615, 18446744073709551615,
+	"18446744073709551615", "18446744073709551615",
+     18446744073709551615.0, 18446744073709551615.0);
+SELECT d1, CAST(d1 AS UNSIGNED), CAST(d2 AS UNSIGNED) FROM t1;
+SELECT str1, CAST(str1 AS UNSIGNED), CAST(str2 AS UNSIGNED) FROM t1;
+--echo # Doubles only store about 15 digits of precision for decimal places, therefore both inserted numbers
+--echo # are rounded to 18446744073709552000.0 and we expect them to be casted to EMPTYROW value each.
+SELECT double1, CAST(double1 AS UNSIGNED), CAST(double2 AS UNSIGNED) FROM t1;
+DROP DATABASE mcol_4648;