Additional changes for #1031, support abs SQL function

- Return type based upon input type (for integer types) - Fill in codegen implementation (initial code from Prashant) - Fix tests to handle integer types - Apply suggested fix for OSX Travis builds (not tested on OSX) Changes tested on Ubuntu 14.04, using a debug build
cmu-db · Jan 11, 2018 · 32548ac · 32548ac
1 parent 73c87c5
commit 32548ac
Show file tree

Hide file tree

Showing 4 changed files with 65 additions and 40 deletions.
diff --git a/src/catalog/catalog.cpp b/src/catalog/catalog.cpp
@@ -1001,29 +1001,29 @@ void Catalog::InitializeFunctions() {
        * integer functions
        */
       AddBuiltinFunction(
-          "abs", {type::TypeId::TINYINT}, type::TypeId::DECIMAL, internal_lang,
-          "Abs",
+          "abs", {type::TypeId::TINYINT}, type::TypeId::TINYINT, 
+          internal_lang, "Abs",
           function::BuiltInFuncType{OperatorId::Abs,
                                     function::DecimalFunctions::_Abs},
           txn);
 
       AddBuiltinFunction(
-          "abs", {type::TypeId::SMALLINT}, type::TypeId::DECIMAL, internal_lang,
-          "Abs",
+          "abs", {type::TypeId::SMALLINT}, type::TypeId::SMALLINT, 
+          internal_lang, "Abs",
           function::BuiltInFuncType{OperatorId::Abs,
                                     function::DecimalFunctions::_Abs},
           txn);
 
       AddBuiltinFunction(
-          "abs", {type::TypeId::INTEGER}, type::TypeId::DECIMAL, internal_lang,
-          "Abs",
+          "abs", {type::TypeId::INTEGER}, type::TypeId::INTEGER, 
+          internal_lang, "Abs",
           function::BuiltInFuncType{OperatorId::Abs,
                                     function::DecimalFunctions::_Abs},
           txn);
 
       AddBuiltinFunction(
-          "abs", {type::TypeId::BIGINT}, type::TypeId::DECIMAL, internal_lang,
-          "Abs",
+          "abs", {type::TypeId::BIGINT}, type::TypeId::BIGINT, 
+          internal_lang, "Abs",
           function::BuiltInFuncType{OperatorId::Abs,
                                     function::DecimalFunctions::_Abs},
           txn);

diff --git a/src/codegen/type/integer_type.cpp b/src/codegen/type/integer_type.cpp
@@ -189,23 +189,6 @@ struct Floor : public TypeSystem::UnaryOperatorHandleNull {
   }
 };
 
-// Abs
-struct Abs : public TypeSystem::UnaryOperatorHandleNull {
-  bool SupportsType(const Type &type) const override {
-    return type.GetSqlType() == Integer::Instance();
-  }
-
-  Type ResultType(UNUSED_ATTRIBUTE const Type &val_type) const override {
-    return Type{Integer::Instance()};
-  }
-
-  Value Impl(UNUSED_ATTRIBUTE CodeGen &codegen,
-               const Value &val) const override {
-    PL_ASSERT(SupportsType(val.GetType()));
-    return val;
-  }
-};
-
 // Ceiling
 struct Ceil : public TypeSystem::UnaryOperatorHandleNull {
   bool SupportsType(const Type &type) const override {
@@ -428,6 +411,30 @@ struct Modulo : public TypeSystem::BinaryOperatorHandleNull {
   }
 };
 
+// Abs
+struct Abs : public TypeSystem::UnaryOperatorHandleNull {
+  bool SupportsType(const Type &type) const override {
+    return type.GetSqlType() == Integer::Instance();
+  }
+
+  Type ResultType(UNUSED_ATTRIBUTE const Type &val_type) const override {
+    return Type{Integer::Instance()};
+  }
+
+  Value Impl(CodeGen &codegen, const Value &val) const override {
+    // The integer subtraction implementation
+    Sub sub;
+    // Zero place-holder
+    auto zero = codegen::Value{type::Integer::Instance(), codegen.Const32(0)};
+
+    // We want: raw_ret = (val < 0 ? 0 - val : val)
+    auto sub_result = sub.Impl(codegen, zero, val, OnError::Exception);
+    auto *lt_zero = codegen->CreateICmpSLT(val.GetValue(), zero.GetValue());
+    auto *raw_ret = codegen->CreateSelect(lt_zero, sub_result.GetValue(), val.GetValue());
+    return Value{Integer::Instance(), raw_ret};
+  }
+};
+
 //===----------------------------------------------------------------------===//
 // TYPE SYSTEM CONSTRUCTION
 //===----------------------------------------------------------------------===//

diff --git a/src/function/decimal_functions.cpp b/src/function/decimal_functions.cpp
@@ -31,27 +31,45 @@ type::Value DecimalFunctions::_Abs(const std::vector<type::Value> &args) {
   if (args[0].IsNull()) {
     return type::ValueFactory::GetNullValueByType(type::TypeId::DECIMAL);
   }
-  double result;
   switch (args[0].GetElementType()) {
     case type::TypeId::DECIMAL:
-      result = Abs(args[0].GetAs<double>());
+      {
+        double result;
+        result = Abs(args[0].GetAs<double>());
+        return type::ValueFactory::GetDecimalValue(result);
+      }
       break;
     case type::TypeId::INTEGER:
-      result = abs(args[0].GetAs<int32_t>());
-      break;
+      {
+        int32_t result;
+        result = abs(args[0].GetAs<int32_t>());
+        return type::ValueFactory::GetIntegerValue(result);
+        break;
+      }
     case type::TypeId::BIGINT:
-      result = abs(args[0].GetAs<int64_t>());
+      {
+        int64_t result;
+        result = std::abs(args[0].GetAs<int64_t>());
+        return type::ValueFactory::GetBigIntValue(result);
+      }
       break;
     case type::TypeId::SMALLINT:
-      result = abs(args[0].GetAs<int16_t>());
+      {
+        int16_t result;
+        result = abs(args[0].GetAs<int16_t>());
+        return type::ValueFactory::GetSmallIntValue(result);
+      }
       break;
     case type::TypeId::TINYINT:
-      result = abs(args[0].GetAs<int8_t>());
+      {
+        int8_t result;
+        result = abs(args[0].GetAs<int8_t>());
+        return type::ValueFactory::GetTinyIntValue(result);
+      }
       break;
     default:
       return type::ValueFactory::GetNullValueByType(type::TypeId::DECIMAL);
   }
-  return type::ValueFactory::GetDecimalValue(result);
 }
 
 double DecimalFunctions::Abs(const double args) { return fabs(args); }

diff --git a/test/function/decimal_functions_test.cpp b/test/function/decimal_functions_test.cpp
@@ -133,28 +133,28 @@ TEST_F(DecimalFunctionsTests, AbsTestInt) {
     args = {type::ValueFactory::GetIntegerValue(in)};
     auto result = function::DecimalFunctions::_Abs(args);
     EXPECT_FALSE(result.IsNull());
-    EXPECT_EQ(abs(in), result.GetAs<double>());
+    EXPECT_EQ(abs(in), result.GetAs<int64_t>());
   }
 
-  for (int in: intTestInputs) {
+  for (int32_t in: intTestInputs) {
     args = {type::ValueFactory::GetIntegerValue(in)};
     auto result = function::DecimalFunctions::_Abs(args);
     EXPECT_FALSE(result.IsNull());
-    EXPECT_EQ(abs(in), result.GetAs<double>());
+    EXPECT_EQ(abs(in), result.GetAs<int32_t>());
   }
 
-  for (int in: smallIntTestInputs) {
+  for (int16_t in: smallIntTestInputs) {
     args = {type::ValueFactory::GetIntegerValue(in)};
     auto result = function::DecimalFunctions::_Abs(args);
     EXPECT_FALSE(result.IsNull());
-    EXPECT_EQ(abs(in), result.GetAs<double>());
+    EXPECT_EQ(abs(in), result.GetAs<int16_t>());
   }
 
-  for (int in: tinyIntTestInputs) {
+  for (int8_t in: tinyIntTestInputs) {
     args = {type::ValueFactory::GetIntegerValue(in)};
     auto result = function::DecimalFunctions::_Abs(args);
     EXPECT_FALSE(result.IsNull());
-    EXPECT_EQ(abs(in), result.GetAs<double>());
+    EXPECT_EQ(abs(in), result.GetAs<int8_t>());
   }
 }