fix(tianmu): incorrect result when using where expr and args > bigint_max #1564

mysql-test/suite/tianmu/r/issue1564.result

@@ -0,0 +1,99 @@
+DROP DATABASE IF EXISTS issue1564;
+create database issue1564;
+use issue1564;
+create table t(a bigint not null);
+insert into t values(-222222), (-22), (-15),(-16),(0), (11), (12), (9223372036854775807);
+select * from t;
+a
+-222222
+-22
+-15
+-16
+0
+11
+12
+9223372036854775807
+select * from t where a = 18446744073709551601;
+a
+select * from t where a != 18446744073709551601;
+a
+-222222
+-22
+-15
+-16
+0
+11
+12
+9223372036854775807
+select * from t where a = -22;
+a
+-22
+select * from t where a != -22;
+a
+-222222
+-15
+-16
+0
+11
+12
+9223372036854775807
+select * from t where a in(-16, -15, -11);
+a
+-15
+-16
+select * from t where a > 18446744073709551599;
+a
+select * from t where a >= 18446744073709551599;
+a
+select * from t where a < 18446744073709551599;
+a
+-222222
+-22
+-15
+-16
+0
+11
+12
+9223372036854775807
+select * from t where a <= 18446744073709551599;
+a
+-222222
+-22
+-15
+-16
+0
+11
+12
+9223372036854775807
+select * from t where a between -22 and 18446744073709551599;
+a
+-22
+-15
+-16
+0
+11
+12
+9223372036854775807
+select * from t where a between -22 and 9223372036854775807;
+a
+-22
+-15
+-16
+0
+11
+12
+9223372036854775807
+select * from t where a between -222222 and 9223372036854775807;
+a
+-222222
+-22
+-15
+-16
+0
+11
+12
+9223372036854775807
+select * from t where a between 9223372036854775807 and -22;
+a
+drop table t;
+drop database issue1564;

mysql-test/suite/tianmu/t/issue1564.test

@@ -0,0 +1,27 @@
+--source include/have_tianmu.inc
+
+--disable_warnings
+DROP DATABASE IF EXISTS issue1564;
+--enable_warnings
+create database issue1564;
+use issue1564;
+
+create table t(a bigint not null);
+insert into t values(-222222), (-22), (-15),(-16),(0), (11), (12), (9223372036854775807);
+select * from t;
+select * from t where a = 18446744073709551601;
+select * from t where a != 18446744073709551601;
+select * from t where a = -22;
+select * from t where a != -22;
+select * from t where a in(-16, -15, -11);
+select * from t where a > 18446744073709551599;
+select * from t where a >= 18446744073709551599;
+select * from t where a < 18446744073709551599;
+select * from t where a <= 18446744073709551599;
+select * from t where a between -22 and 18446744073709551599;
+select * from t where a between -22 and 9223372036854775807;
+select * from t where a between -222222 and 9223372036854775807;
+select * from t where a between 9223372036854775807 and -22;
+
+drop table t;
+drop database issue1564;

storage/tianmu/core/mysql_expression.cpp

@@ -560,6 +560,9 @@ std::shared_ptr<ValueOrNull> MysqlExpression::ItemInt2ValueOrNull(Item *item) {
   if (v == common::NULL_VALUE_64)
     v++;
   val->SetFixed(v);
+  // add unsigned flag here as item may have unsigned valued, like where a = 18446744073709551601; 18446744073709551601
+  // is an unsigned valued stored in item. Introduced by a bug: https://github.com/stoneatom/stonedb/issues/1564
+  val->SetUnsignedFlag(static_cast<bool>(item->unsigned_flag));
   if (item->null_value)
     return std::make_shared<ValueOrNull>();
   return val;

storage/tianmu/core/value_or_null.cpp

@@ -77,7 +77,7 @@ void ValueOrNull::GetBString(types::BString &tianmu_s) const {
 }
 
 ValueOrNull::ValueOrNull(ValueOrNull const &von)
-    : x(von.x), len(von.len), string_owner(von.string_owner), null(von.null) {
+    : x(von.x), len(von.len), string_owner(von.string_owner), null(von.null), unsigned_flag_(von.unsigned_flag_) {
   if (string_owner && von.sp && len > 0) {
     sp = new (std::nothrow) char[len + 1];
 
@@ -115,9 +115,11 @@ ValueOrNull &ValueOrNull::operator=(ValueOrNull const &von) {
   return (*this);
 }
 
-ValueOrNull::ValueOrNull(types::TianmuNum const &tianmu_n) : x(tianmu_n.GetValueInt64()), null(tianmu_n.IsNull()) {}
+ValueOrNull::ValueOrNull(types::TianmuNum const &tianmu_n)
+    : x(tianmu_n.GetValueInt64()), null(tianmu_n.IsNull()), unsigned_flag_(tianmu_n.GetUnsignedFlag()) {}
 
-ValueOrNull::ValueOrNull(types::TianmuDateTime const &tianmu_dt) : x(tianmu_dt.GetInt64()), null(tianmu_dt.IsNull()) {}
+ValueOrNull::ValueOrNull(types::TianmuDateTime const &tianmu_dt)
+    : x(tianmu_dt.GetInt64()), null(tianmu_dt.IsNull()), unsigned_flag_(true) {}
 
 ValueOrNull::ValueOrNull(types::BString const &tianmu_s)
     : x(common::NULL_VALUE_64),
@@ -136,6 +138,7 @@ void ValueOrNull::Swap(ValueOrNull &von) {
     std::swap(sp, von.sp);
     std::swap(len, von.len);
     std::swap(string_owner, von.string_owner);
+    std::swap(unsigned_flag_, von.unsigned_flag_);
   }
 }
 

storage/tianmu/core/value_or_null.h

@@ -58,6 +58,8 @@ class ValueOrNull final {
   bool NotNull() const { return !null; }
   size_t StrLen() const { return len; }
   int64_t Get64() const { return x; }
+  bool GetUnsignedFlag() const { return unsigned_flag_; }
+  void SetUnsignedFlag(bool is_unsigned) { unsigned_flag_ = is_unsigned; }
 
   void SetFixed(int64_t v) {
     Clear();
@@ -119,16 +121,18 @@ class ValueOrNull final {
     null = true;
     x = common::NULL_VALUE_64;
     len = 0;
+    unsigned_flag_ = false;
   }
 
  private:
-  int64_t x;                  // 8-byte value of an expression; interpreted as int64_t or double
-  char *sp = nullptr;         // != 0 if string value assigned
-  uint len = 0;               // string length; used only for ValueType::VT_STRING
-  bool string_owner = false;  // if true, destructor must deallocate sp
+  int64_t x;                    // 8-byte value of an expression; interpreted as int64_t or double
+  bool unsigned_flag_ = false;  // check if x is unsigned or not
+  char *sp = nullptr;           // != 0 if string value assigned
+  uint len = 0;                 // string length; used only for ValueType::VT_STRING
+  bool string_owner = false;    // if true, destructor must deallocate sp
   bool null;
 };
 }  // namespace core
 }  // namespace Tianmu
 
-#endif  // TIANMU_CORE_VALUE_OR_NULL_H_
+#endif  // TIANMU_CORE_VALUE_OR_NULL_H_

storage/tianmu/optimizer/condition_encoder.cpp

@@ -238,6 +238,15 @@ void ConditionEncoder::TransformOtherThanINsOnNumerics() {
   bool v1_rounded = false, v2_rounded = false;
   static MIIterator mit(nullptr, pack_power);
 
+  // Will be used to construct value with signed/unsigned flag to valur_or_null later.
+  bool is_v1_unsigned = false, is_v2_unsigned = false;
+  if (nullptr != desc->val1.vc) {
+    is_v1_unsigned = desc->val1.vc->Type().GetUnsigned();
+  }
+  if (nullptr != desc->val2.vc) {
+    is_v2_unsigned = desc->val2.vc->Type().GetUnsigned();
+  }
+
   vcolumn::MultiValColumn *mvc = 0;
   if (desc->val1.vc && (desc->val1.vc)->IsMultival()) {
     mvc = static_cast<vcolumn::MultiValColumn *>(desc->val1.vc);
@@ -321,14 +330,20 @@ void ConditionEncoder::TransformOtherThanINsOnNumerics() {
     return;
   }
 
-  if (ISTypeOfEqualOperator(desc->op) || ISTypeOfNotEqualOperator(desc->op))
+  if (ISTypeOfEqualOperator(desc->op) || ISTypeOfNotEqualOperator(desc->op)) {
     v2 = v1;
+    is_v2_unsigned = is_v1_unsigned;
+  }
 
   if (ISTypeOfLessOperator(desc->op)) {
     if (!ATI::IsRealType(AttrTypeName())) {
       if (v1 > common::MINUS_INF_64)
         v2 = v1 - 1;
       v1 = common::MINUS_INF_64;
+      if (is_v1_unsigned && static_cast<uint64_t>(v1) > common::PLUS_INF_64) {  // like where a >= 18446744073709551599;
+        is_v2_unsigned = true;
+        is_v1_unsigned = false;  // as v1 = common::MINUS_INF_64
+      }
     } else {
       if (*(double *)&v1 > common::MINUS_INF_DBL)
         v2 = (AttrTypeName() == common::ColumnType::REAL) ? DoubleMinusEpsilon(v1) : FloatMinusEpsilon(v1);
@@ -350,9 +365,11 @@ void ConditionEncoder::TransformOtherThanINsOnNumerics() {
 
   if (ISTypeOfLessEqualOperator(desc->op)) {
     v2 = v1;
-    if (!ATI::IsRealType(AttrTypeName()))
+    is_v2_unsigned = is_v1_unsigned;  // set v2 unsigned flag be the same with v1
+    if (!ATI::IsRealType(AttrTypeName())) {
       v1 = common::MINUS_INF_64;
-    else
+      is_v1_unsigned = false;  // v1 unsigned flag maybe true in case: where x <= 18446744073709551599.
+    } else
       v1 = *(int64_t *)&common::MINUS_INF_DBL;
   }
 
@@ -364,20 +381,29 @@ void ConditionEncoder::TransformOtherThanINsOnNumerics() {
   }
 
   desc->sharp = false;
-  if (ISTypeOfNotEqualOperator(desc->op) || desc->op == common::Operator::O_NOT_BETWEEN)
+  if (ISTypeOfNotEqualOperator(desc->op) || desc->op == common::Operator::O_NOT_BETWEEN) {
     desc->op = common::Operator::O_NOT_BETWEEN;
-  else
+  } else {
     desc->op = common::Operator::O_BETWEEN;
+    //// After set between, check right args boundary, just like v1 above. deal with "where a between -22 and
+    /// 18446744073709551599"
+    if (is_v2_unsigned && static_cast<uint64_t>(v2) > common::PLUS_INF_64) {
+      is_v2_unsigned = false;
+      v2 = common::PLUS_INF_64;
+    }
+  }
 
   desc->val1 = CQTerm();
   desc->val1.vc = new vcolumn::ConstColumn(
-      ValueOrNull(types::TianmuNum(v1, attr->Type().GetScale(), ATI::IsRealType(AttrTypeName()), AttrTypeName())),
+      ValueOrNull(types::TianmuNum(v1, attr->Type().GetScale(), ATI::IsRealType(AttrTypeName()), AttrTypeName(),
+                                   is_v1_unsigned)),
       attr->Type());
   desc->val1.vc_id = desc->table->AddVirtColumn(desc->val1.vc);
 
   desc->val2 = CQTerm();
   desc->val2.vc = new vcolumn::ConstColumn(
-      ValueOrNull(types::TianmuNum(v2, attr->Type().GetScale(), ATI::IsRealType(AttrTypeName()), AttrTypeName())),
+      ValueOrNull(types::TianmuNum(v2, attr->Type().GetScale(), ATI::IsRealType(AttrTypeName()), AttrTypeName(),
+                                   is_v2_unsigned)),
       attr->Type());
   desc->val2.vc_id = desc->table->AddVirtColumn(desc->val2.vc);
 }

storage/tianmu/types/tianmu_num.cpp

@@ -29,14 +29,16 @@ namespace Tianmu {
 namespace types {
 
 TianmuNum::TianmuNum(common::ColumnType attrt)
-    : value_(0), scale_(0), is_double_(false), is_dot_(false), attr_type_(attrt) {}
+    : value_(0), scale_(0), is_double_(false), is_dot_(false), attr_type_(attrt), unsigned_flag_(false) {}
 
-TianmuNum::TianmuNum(int64_t value, short scale, bool is_double, common::ColumnType attrt) {
-  Assign(value, scale, is_double, attrt);
+TianmuNum::TianmuNum(int64_t value, short scale, bool is_double, common::ColumnType attrt, bool unsigned_flag) {
+  Assign(value, scale, is_double, attrt, unsigned_flag);
 }
 
 TianmuNum::TianmuNum(double value)
     : value_(*(int64_t *)&value), scale_(0), is_double_(true), is_dot_(false), attr_type_(common::ColumnType::REAL) {
+  // In tianmu, real type does not support unsigned well.
+  unsigned_flag_ = false;
   null_ = (value_ == NULL_VALUE_D ? true : false);
 }
 
@@ -46,17 +48,19 @@ TianmuNum::TianmuNum(const TianmuNum &tianmu_n)
       scale_(tianmu_n.scale_),
       is_double_(tianmu_n.is_double_),
       is_dot_(tianmu_n.is_dot_),
-      attr_type_(tianmu_n.attr_type_) {
+      attr_type_(tianmu_n.attr_type_),
+      unsigned_flag_(tianmu_n.unsigned_flag_) {
   null_ = tianmu_n.null_;
 }
 
 TianmuNum::~TianmuNum() {}
 
-TianmuNum &TianmuNum::Assign(int64_t value, short scale, bool is_double, common::ColumnType attrt) {
+TianmuNum &TianmuNum::Assign(int64_t value, short scale, bool is_double, common::ColumnType attrt, bool unsigned_flag) {
   this->value_ = value;
   this->scale_ = scale;
   this->is_double_ = is_double;
   this->attr_type_ = attrt;
+  this->unsigned_flag_ = unsigned_flag;
 
   if (scale != -1 &&
       !is_double_) {  // check if construct decimal, the UNK is used on temp_table.cpp: GetValueString(..)
@@ -84,6 +88,7 @@ TianmuNum &TianmuNum::Assign(double value) {
   this->is_double_ = true;
   this->is_dot_ = false;
   this->attr_type_ = common::ColumnType::REAL;
+  this->unsigned_flag_ = false;
   common::double_int_t v(value_);
   null_ = (v.i == common::NULL_VALUE_64 ? true : false);
   return *this;

storage/tianmu/types/tianmu_num.h

@@ -31,13 +31,14 @@ class TianmuNum : public ValueBasic<TianmuNum> {
 
  public:
   TianmuNum(common::ColumnType attrt = common::ColumnType::NUM);
-  TianmuNum(int64_t value, short scale = -1, bool dbl = false, common::ColumnType attrt = common::ColumnType::UNK);
+  TianmuNum(int64_t value, short scale = -1, bool dbl = false, common::ColumnType attrt = common::ColumnType::UNK,
+            bool unsigned_flag = false);
   TianmuNum(double value);
   TianmuNum(const TianmuNum &);
   ~TianmuNum();
 
   TianmuNum &Assign(int64_t value, short scale = -1, bool dbl = false,
-                    common::ColumnType attrt = common::ColumnType::UNK);
+                    common::ColumnType attrt = common::ColumnType::UNK, bool unsigned_flag = false);
   TianmuNum &Assign(double value);
 
   static common::ErrorCode Parse(const BString &tianmu_s, TianmuNum &tianmu_n,
@@ -82,6 +83,7 @@ class TianmuNum : public ValueBasic<TianmuNum> {
 
   short Scale() const { return scale_; }
   int64_t ValueInt() const { return value_; }
+  bool GetUnsignedFlag() const { return unsigned_flag_; }
   char *GetDataBytesPointer() const override { return (char *)&value_; }
   int64_t GetIntPart() const {
     return is_double_ ? (int64_t)GetIntPartAsDouble() : value_ / (int64_t)Uint64PowOfTen(scale_);
@@ -105,6 +107,7 @@ class TianmuNum : public ValueBasic<TianmuNum> {
  private:
   static constexpr int MAX_DEC_PRECISION = 18;
   int64_t value_;
+  bool unsigned_flag_ = false;
   ushort scale_;  // means 'scale' actually
   bool is_double_;
   bool is_dot_;

storage/tianmu/vc/const_column.h

@@ -58,6 +58,7 @@ class ConstColumn : public VirtualColumn {
   int64_t GetValueInt64Impl([[maybe_unused]] const core::MIIterator &mit) override {
     return value_or_null_.IsNull() ? common::NULL_VALUE_64 : value_or_null_.Get64();
   }
+  bool GetUnsignedFlagImpl() override { return value_or_null_.GetUnsignedFlag(); }
   bool IsNullImpl([[maybe_unused]] const core::MIIterator &mit) override { return value_or_null_.IsNull(); }
   void GetValueStringImpl(types::BString &s, const core::MIIterator &mit) override;
   double GetValueDoubleImpl(const core::MIIterator &mit) override;

storage/tianmu/vc/const_expr_column.h

@@ -96,6 +96,8 @@ class ConstExpressionColumn : public ExpressionColumn {
     return last_val_->IsNull() ? common::NULL_VALUE_64 : last_val_->Get64();
   }
 
+  bool GetUnsignedFlagImpl() override { return last_val_->GetUnsignedFlag(); }
+
   double GetValueDoubleImpl(const core::MIIterator &mit) override;
   int64_t GetMinInt64Impl([[maybe_unused]] const core::MIIterator &mit) override {
     return last_val_->IsNull() ? common::MINUS_INF_64 : last_val_->Get64();

storage/tianmu/vc/expr_column.h

@@ -83,6 +83,7 @@ class ExpressionColumn : public VirtualColumn {
   /////////////// Data access //////////////////////
  protected:
   int64_t GetValueInt64Impl(const core::MIIterator &) override;
+  bool GetUnsignedFlagImpl() override { return static_cast<bool>(expr_->GetItem()->unsigned_flag); }
   bool IsNullImpl(const core::MIIterator &) override;
   void GetValueStringImpl(types::BString &, const core::MIIterator &) override;
   double GetValueDoubleImpl(const core::MIIterator &) override;