Merge pull request #21213 from wangjiaming0909/fix/TS-3350

fix: data compare of signed and unsigned integers

source/libs/scalar/test/CMakeLists.txt

@@ -1,4 +1,4 @@
 enable_testing()
 
-#add_subdirectory(filter)
+add_subdirectory(filter)
 add_subdirectory(scalar)

source/libs/scalar/test/filter/filterTests.cpp

@@ -33,6 +33,7 @@
 #include "os.h"
 
 #include "filter.h"
+#include "filterInt.h"
 #include "nodes.h"
 #include "scalar.h"
 #include "stub.h"
@@ -344,6 +345,7 @@ TEST(timerangeTest, greater_and_lower_not_strict) {
   nodesDestroyNode(logicNode1);
 }
 
+#if 0
 TEST(columnTest, smallint_column_greater_double_value) {
   SNode       *pLeft = NULL, *pRight = NULL, *opNode = NULL;
   int16_t      leftv[5] = {1, 2, 3, 4, 5};
@@ -1337,6 +1339,127 @@ TEST(scalarModelogicTest, diff_columns_or_and_or) {
   nodesDestroyNode(logicNode1);
   blockDataDestroy(src);
 }
+#endif
+
+template <class SignedT, class UnsignedT>
+int32_t compareSignedWithUnsigned(SignedT l, UnsignedT r) {
+  if (l < 0) return -1;
+  auto l_uint64 = static_cast<uint64_t>(l);
+  auto r_uint64 = static_cast<uint64_t>(r);
+  if (l_uint64 < r_uint64) return -1;
+  if (l_uint64 > r_uint64) return 1;
+  return 0;
+}
+
+template <class UnsignedT, class SignedT>
+int32_t compareUnsignedWithSigned(UnsignedT l, SignedT r) {
+  if (r < 0) return 1;
+  auto l_uint64 = static_cast<uint64_t>(l);
+  auto r_uint64 = static_cast<uint64_t>(r);
+  if (l_uint64 < r_uint64) return -1;
+  if (l_uint64 > r_uint64) return 1;
+  return 0;
+}
+
+template <class SignedT, class UnsignedT>
+void doCompareWithValueRange_SignedWithUnsigned(__compar_fn_t fp) {
+  int32_t signedMin = -10, signedMax = 10;
+  int32_t unsignedMin = 0, unsignedMax = 10;
+  for (SignedT l = signedMin; l <= signedMax; ++l) {
+    for (UnsignedT r = unsignedMin; r <= unsignedMax; ++r) {
+      ASSERT_EQ(fp(&l, &r), compareSignedWithUnsigned(l, r));
+    }
+  }
+}
+
+template <class UnsignedT, class SignedT>
+void doCompareWithValueRange_UnsignedWithSigned(__compar_fn_t fp) {
+  int32_t signedMin = -10, signedMax = 10;
+  int32_t unsignedMin = 0, unsignedMax = 10;
+  for (UnsignedT l = unsignedMin; l <= unsignedMax; ++l) {
+    for (SignedT r = signedMin; r <= signedMax; ++r) {
+      ASSERT_EQ(fp(&l, &r), compareUnsignedWithSigned(l, r));
+    }
+  }
+}
+
+template <class LType>
+void doCompareWithValueRange_OnlyLeftType(__compar_fn_t fp, int32_t rType) {
+  switch (rType) {
+    case TSDB_DATA_TYPE_UTINYINT:
+      doCompareWithValueRange_SignedWithUnsigned<LType, uint8_t>(fp);
+      break;
+    case TSDB_DATA_TYPE_USMALLINT:
+      doCompareWithValueRange_SignedWithUnsigned<LType, uint16_t>(fp);
+      break;
+    case TSDB_DATA_TYPE_UINT:
+      doCompareWithValueRange_SignedWithUnsigned<LType, uint32_t>(fp);
+      break;
+    case TSDB_DATA_TYPE_UBIGINT:
+      doCompareWithValueRange_SignedWithUnsigned<LType, uint64_t>(fp);
+      break;
+    case TSDB_DATA_TYPE_TINYINT:
+      doCompareWithValueRange_UnsignedWithSigned<LType, int8_t>(fp);
+      break;
+    case TSDB_DATA_TYPE_SMALLINT:
+      doCompareWithValueRange_UnsignedWithSigned<LType, int16_t>(fp);
+      break;
+    case TSDB_DATA_TYPE_INT:
+      doCompareWithValueRange_UnsignedWithSigned<LType, int32_t>(fp);
+      break;
+    case TSDB_DATA_TYPE_BIGINT:
+      doCompareWithValueRange_UnsignedWithSigned<LType, int64_t>(fp);
+      break;
+    default:
+      FAIL();
+  }
+}
+
+void doCompare(const std::vector<int32_t> &lTypes, const std::vector<int32_t> &rTypes, int32_t oper) {
+  for (int i = 0; i < lTypes.size(); ++i) {
+    for (int j = 0; j < rTypes.size(); ++j) {
+      auto fp = filterGetCompFuncEx(lTypes[i], rTypes[j], oper);
+      switch (lTypes[i]) {
+        case TSDB_DATA_TYPE_TINYINT:
+          doCompareWithValueRange_OnlyLeftType<int8_t>(fp, rTypes[j]);
+          break;
+        case TSDB_DATA_TYPE_SMALLINT:
+          doCompareWithValueRange_OnlyLeftType<int16_t>(fp, rTypes[j]);
+          break;
+        case TSDB_DATA_TYPE_INT:
+          doCompareWithValueRange_OnlyLeftType<int32_t>(fp, rTypes[j]);
+          break;
+        case TSDB_DATA_TYPE_BIGINT:
+          doCompareWithValueRange_OnlyLeftType<int64_t>(fp, rTypes[j]);
+          break;
+        case TSDB_DATA_TYPE_UTINYINT:
+          doCompareWithValueRange_OnlyLeftType<uint8_t>(fp, rTypes[j]);
+          break;
+        case TSDB_DATA_TYPE_USMALLINT:
+          doCompareWithValueRange_OnlyLeftType<uint16_t>(fp, rTypes[j]);
+          break;
+        case TSDB_DATA_TYPE_UINT:
+          doCompareWithValueRange_OnlyLeftType<uint32_t>(fp, rTypes[j]);
+          break;
+        case TSDB_DATA_TYPE_UBIGINT:
+          doCompareWithValueRange_OnlyLeftType<uint64_t>(fp, rTypes[j]);
+          break;
+        default:
+          FAIL();
+      }
+    }
+  }
+}
+
+TEST(dataCompareTest, signed_and_unsigned_int) {
+  std::vector<int32_t> lType = {TSDB_DATA_TYPE_TINYINT, TSDB_DATA_TYPE_SMALLINT, TSDB_DATA_TYPE_INT,
+                                TSDB_DATA_TYPE_BIGINT};
+  std::vector<int32_t> rType = {TSDB_DATA_TYPE_UTINYINT, TSDB_DATA_TYPE_USMALLINT, TSDB_DATA_TYPE_UINT,
+                                TSDB_DATA_TYPE_UBIGINT};
+
+  doCompare(lType, rType, OP_TYPE_GREATER_THAN);
+  doCompare(rType, lType, OP_TYPE_GREATER_THAN);
+}
 
 int main(int argc, char **argv) {
   taosSeedRand(taosGetTimestampSec());

source/util/src/tcompare.c

@@ -308,17 +308,19 @@ int32_t compareInt8Uint16(const void *pLeft, const void *pRight) {
 
 int32_t compareInt8Uint32(const void *pLeft, const void *pRight) {
   int8_t   left = GET_INT8_VAL(pLeft);
+  if (left < 0) return -1;
   uint32_t right = GET_UINT32_VAL(pRight);
-  if (left > right) return 1;
+  if ((uint32_t)left > right) return 1;
-  if (left < right) return -1;
+  if ((uint32_t)left < right) return -1;
   return 0;
 }
 
 int32_t compareInt8Uint64(const void *pLeft, const void *pRight) {
   int8_t   left = GET_INT8_VAL(pLeft);
+  if (left < 0) return -1;
   uint64_t right = GET_UINT64_VAL(pRight);
-  if (left > right) return 1;
+  if ((uint64_t)left > right) return 1;
-  if (left < right) return -1;
+  if ((uint64_t)left < right) return -1;
   return 0;
 }
 
@@ -380,17 +382,19 @@ int32_t compareInt16Uint16(const void *pLeft, const void *pRight) {
 
 int32_t compareInt16Uint32(const void *pLeft, const void *pRight) {
   int16_t  left = GET_INT16_VAL(pLeft);
+  if (left < 0) return -1;
   uint32_t right = GET_UINT32_VAL(pRight);
-  if (left > right) return 1;
+  if ((uint32_t)left > right) return 1;
-  if (left < right) return -1;
+  if ((uint32_t)left < right) return -1;
   return 0;
 }
 
 int32_t compareInt16Uint64(const void *pLeft, const void *pRight) {
   int16_t  left = GET_INT16_VAL(pLeft);
+  if (left < 0) return -1;
   uint64_t right = GET_UINT64_VAL(pRight);
-  if (left > right) return 1;
+  if ((uint64_t)left > right) return 1;
-  if (left < right) return -1;
+  if ((uint64_t)left < right) return -1;
   return 0;
 }
 
@@ -452,17 +456,19 @@ int32_t compareInt32Uint16(const void *pLeft, const void *pRight) {
 
 int32_t compareInt32Uint32(const void *pLeft, const void *pRight) {
   int32_t  left = GET_INT32_VAL(pLeft);
+  if (left < 0) return -1;
   uint32_t right = GET_UINT32_VAL(pRight);
-  if (left > right) return 1;
+  if ((uint32_t)left > right) return 1;
-  if (left < right) return -1;
+  if ((uint32_t)left < right) return -1;
   return 0;
 }
 
 int32_t compareInt32Uint64(const void *pLeft, const void *pRight) {
   int32_t  left = GET_INT32_VAL(pLeft);
+  if (left < 0) return -1;
   uint64_t right = GET_UINT64_VAL(pRight);
-  if (left > right) return 1;
+  if ((uint64_t)left > right) return 1;
-  if (left < right) return -1;
+  if ((uint64_t)left < right) return -1;
   return 0;
 }
 
@@ -532,9 +538,10 @@ int32_t compareInt64Uint32(const void *pLeft, const void *pRight) {
 
 int32_t compareInt64Uint64(const void *pLeft, const void *pRight) {
   int64_t  left = GET_INT64_VAL(pLeft);
+  if (left < 0) return -1;
   uint64_t right = GET_UINT64_VAL(pRight);
-  if (left > right) return 1;
+  if ((uint64_t)left > right) return 1;
-  if (left < right) return -1;
+  if ((uint64_t)left < right) return -1;
   return 0;
 }
 
@@ -857,24 +864,27 @@ int32_t compareUint16Uint64(const void *pLeft, const void *pRight) {
 int32_t compareUint32Int8(const void *pLeft, const void *pRight) {
   uint32_t left = GET_UINT32_VAL(pLeft);
   int8_t   right = GET_INT8_VAL(pRight);
-  if (left > right) return 1;
+  if (right < 0) return 1;
-  if (left < right) return -1;
+  if (left > (uint32_t)right) return 1;
+  if (left < (uint32_t)right) return -1;
   return 0;
 }
 
 int32_t compareUint32Int16(const void *pLeft, const void *pRight) {
   uint32_t left = GET_UINT32_VAL(pLeft);
   int16_t  right = GET_INT16_VAL(pRight);
-  if (left > right) return 1;
+  if (right < 0) return 1;
-  if (left < right) return -1;
+  if (left > (uint32_t)right) return 1;
+  if (left < (uint32_t)right) return -1;
   return 0;
 }
 
 int32_t compareUint32Int32(const void *pLeft, const void *pRight) {
   uint32_t left = GET_UINT32_VAL(pLeft);
   int32_t  right = GET_INT32_VAL(pRight);
-  if (left > right) return 1;
+  if (right < 0) return 1;
-  if (left < right) return -1;
+  if (left > (uint32_t)right) return 1;
+  if (left < (uint32_t)right) return -1;
   return 0;
 }
 
@@ -929,32 +939,36 @@ int32_t compareUint32Uint64(const void *pLeft, const void *pRight) {
 int32_t compareUint64Int8(const void *pLeft, const void *pRight) {
   uint64_t left = GET_UINT64_VAL(pLeft);
   int8_t   right = GET_INT8_VAL(pRight);
-  if (left > right) return 1;
+  if (right < 0) return 1;
-  if (left < right) return -1;
+  if (left > (uint64_t)right) return 1;
+  if (left < (uint64_t)right) return -1;
   return 0;
 }
 
 int32_t compareUint64Int16(const void *pLeft, const void *pRight) {
   uint64_t left = GET_UINT64_VAL(pLeft);
   int16_t  right = GET_INT16_VAL(pRight);
-  if (left > right) return 1;
+  if (right < 0) return 1;
-  if (left < right) return -1;
+  if (left > (uint64_t)right) return 1;
+  if (left < (uint64_t)right) return -1;
   return 0;
 }
 
 int32_t compareUint64Int32(const void *pLeft, const void *pRight) {
   uint64_t left = GET_UINT64_VAL(pLeft);
   int32_t  right = GET_INT32_VAL(pRight);
-  if (left > right) return 1;
+  if (right < 0) return 1;
-  if (left < right) return -1;
+  if (left > (uint64_t)right) return 1;
+  if (left < (uint64_t)right) return -1;
   return 0;
 }
 
 int32_t compareUint64Int64(const void *pLeft, const void *pRight) {
   uint64_t left = GET_UINT64_VAL(pLeft);
   int64_t  right = GET_INT64_VAL(pRight);
-  if (left > right) return 1;
+  if (right < 0) return 1;
-  if (left < right) return -1;
+  if (left > (uint64_t)right) return 1;
+  if (left < (uint64_t)right) return -1;
   return 0;
 }