Beefed up tests

Author: pmenon

src/function/numeric_functions.cpp

@@ -182,24 +182,6 @@ type::Value NumericFunctions::_Round(const std::vector<type::Value> &args) {
 
 namespace {
 
-/**
- * Skip all leading and trailing whitespace from the string bounded by the
- * provided pointers. This function will modify the input pointers to point to
- * the first non-whitespace space character at the start and end of the input
- * string.
- *
- * @param[in,out] left Pointer to the left-most character in the input string
- * @param[in,out] right Pointer to the right-most character in the input string
- */
-void TrimLeftRight(const char *&left, const char *&right) {
-  while (*left == ' ') {
-    left++;
-  }
-  while (right > left && *(right - 1) == ' ') {
-    right--;
-  }
-}
-
 /**
  * Convert the provided input string into an integral number. This function
  * handles leading whitespace and leading negative (-) or positive (+) signs.
@@ -216,16 +198,13 @@ T ParseInteger(const char *ptr, uint32_t len) {
   static_assert(std::is_integral<T>::value,
                 "Must provide integer-type when calling ParseInteger");
 
-  if (len == 0) {
-    codegen::RuntimeFunctions::ThrowInvalidInputStringException();
-    __builtin_unreachable();
-  }
-
   const char *start = ptr;
   const char *end = start + len;
 
-  // Trim leading and trailing whitespace
-  TrimLeftRight(start, end);
+  // Trim leading whitespace
+  while (start < end && *start == ' ') {
+    start++;
+  }
 
   // Check negative or positive sign
   bool negative = false;
@@ -238,18 +217,26 @@ T ParseInteger(const char *ptr, uint32_t len) {
 
   // Convert
   int64_t num = 0;
-  while (start != end) {
+  while (start < end) {
     if (*start < '0' || *start > '9') {
-      codegen::RuntimeFunctions::ThrowInvalidInputStringException();
-      __builtin_unreachable();
+      break;
     }
 
     num = (num * 10) + (*start - '0');
 
     start++;
   }
 
-  PELOTON_ASSERT(start == end);
+  // Trim trailing whitespace
+  while (start < end && *start == ' ') {
+    start++;
+  }
+
+  // If we haven't consumed everything at this point, it was an invalid input
+  if (start < end) {
+    codegen::RuntimeFunctions::ThrowInvalidInputStringException();
+    __builtin_unreachable();
+  }
 
   // Negate number if we need to
   if (negative) {
@@ -279,10 +266,13 @@ bool NumericFunctions::InputBoolean(
     __builtin_unreachable();
   }
 
-  const char *start = ptr, *end = ptr + len;
+  const char *start = ptr;
+  const char *end = ptr + len;
 
-  // Trim leading and trailing whitespace
-  TrimLeftRight(start, end);
+  // Trim leading whitespace
+  while (start < end && *start == ' ') {
+    start++;
+  }
 
   //
   uint64_t trimmed_len = end - start;
@@ -393,13 +383,16 @@ double NumericFunctions::InputDecimal(
     __builtin_unreachable();
   }
 
+  const char *start = ptr;
+  const char *end = ptr + len;
+
   // We don't trim because std::strtod() does the trimming for us
 
   // TODO(pmenon): Optimize me later
-  char *end = nullptr;
-  double ret = std::strtod(ptr, &end);
+  char *consumed_ptr = nullptr;
+  double ret = std::strtod(ptr, &consumed_ptr);
 
-  if (unlikely_branch(end == ptr)) {
+  if (unlikely_branch(consumed_ptr == start)) {
     if (errno == ERANGE) {
       codegen::RuntimeFunctions::ThrowOverflowException();
       __builtin_unreachable();
@@ -409,6 +402,17 @@ double NumericFunctions::InputDecimal(
     }
   }
 
+  // Eat the rest
+  while (consumed_ptr < end && *consumed_ptr == ' ') {
+    consumed_ptr++;
+  }
+
+  // If we haven't consumed everything at this point, it was an invalid input
+  if (consumed_ptr < end) {
+    codegen::RuntimeFunctions::ThrowInvalidInputStringException();
+    __builtin_unreachable();
+  }
+
   // Done
   return ret;
 }

src/include/index/bwtree.h

@@ -7585,7 +7585,7 @@ class BwTree : public BwTreeBase {
       // would always fail, until we have cleaned all epoch nodes
       current_epoch_p = nullptr;
 
-      LOG_TRACE("Clearing the epoch in ~EpochManager()...");
+      LOG_TRACE("Clearing the epoch in ~EpochManager() ...");
 
       // If all threads has exited then all thread counts are
       // 0, and therefore this should proceed way to the end

test/codegen/value_integrity_test.cpp

@@ -6,7 +6,7 @@
 //
 // Identification: test/codegen/value_integrity_test.cpp
 //
-// Copyright (c) 2015-17, Carnegie Mellon University Database Group
+// Copyright (c) 2015-2018, Carnegie Mellon University Database Group
 //
 //===----------------------------------------------------------------------===//
 
@@ -190,8 +190,9 @@ void TestInputIntegral(
                      extra_valid_tests.end());
 
   // Default invalid tests
-  std::vector<std::string> invalid_tests = {"a",   "-b",  "+c",  " 1c",
-                                            "2d ", "3 3", "-4 4"};
+  std::vector<std::string> invalid_tests = {"a",       "-b",    "+c",   " 1c",
+                                            "2d ",     "3 3",   "-4 4", "-5 a ",
+                                            "  -6  a", "  c 7 "};
   invalid_tests.insert(invalid_tests.end(), extra_invalid_tests.begin(),
                        extra_invalid_tests.end());
 
@@ -205,19 +206,25 @@ void TestInputIntegral(
   for (const auto &test : valid_tests) {
     auto *ptr = test.first.data();
     auto len = static_cast<uint32_t>(test.first.length());
-    EXPECT_EQ(test.second, TestFunc(type, ptr, len));
+    try {
+      EXPECT_EQ(test.second, TestFunc(type, ptr, len));
+    } catch (std::exception &e) {
+      EXPECT_TRUE(false) << "Valid input '" << test.first << "' threw an error";
+    }
   }
 
   for (const auto &test : invalid_tests) {
     auto *ptr = test.data();
     auto len = static_cast<uint32_t>(test.length());
-    EXPECT_THROW(TestFunc(type, ptr, len), std::runtime_error);
+    EXPECT_THROW(TestFunc(type, ptr, len), std::runtime_error)
+        << "Input '" << test << "' was expected to throw an error, but did not";
   }
 
   for (const auto &test : overflow_tests) {
     auto *ptr = test.data();
     auto len = static_cast<uint32_t>(test.length());
-    EXPECT_THROW(TestFunc(type, ptr, len), std::overflow_error);
+    EXPECT_THROW(TestFunc(type, ptr, len), std::overflow_error)
+        << "Input '" << test << "' expected to overflow, but did not";
   }
 }
 }  // namespace
@@ -238,5 +245,59 @@ TEST_F(ValueIntegrityTest, InputIntegralTypesTest) {
   TestInputIntegral<int64_t>(bigint, function::NumericFunctions::InputBigInt);
 }
 
+TEST_F(ValueIntegrityTest, InputDecimalTypesTest) {
+  codegen::type::Type decimal{type::TypeId::DECIMAL, false};
+
+  // First check some valid cases
+  std::vector<std::pair<std::string, double>> valid_tests = {
+      {"0.0", 0.0},
+      {"-1.0", -1.0},
+      {"2.0", 2.0},
+      {"+3.0", 3.0},
+      {"  4.0", 4.0},
+      {"  -5.0", -5.0},
+      {"  +6.0", 6.0},
+      {"7.0  ", 7.0},
+      {"-8.0  ", -8.0},
+      {"  9.0  ", 9.0},
+      {"  -10.0  ", -10.0},
+      {"  +11.0  ", 11.0}};
+
+  for (const auto &test_case : valid_tests) {
+    auto *ptr = test_case.first.data();
+    auto len = static_cast<uint32_t>(test_case.first.length());
+    EXPECT_EQ(test_case.second,
+              function::NumericFunctions::InputDecimal(decimal, ptr, len));
+  }
+
+  // Now let's try some invalid ones. Take each valid test and randomly insert
+  // a character somewhere.
+  std::vector<std::string> invalid_tests;
+
+  std::random_device rd;
+  std::mt19937 rng(rd());
+
+  for (const auto &valid_test : valid_tests) {
+    auto orig = valid_test.first;
+
+    std::uniform_int_distribution<> dist(0, orig.length());
+    auto pos = dist(rng);
+
+    auto invalid_num = orig.substr(0, pos) + "aa" + orig.substr(pos);
+
+    invalid_tests.push_back(invalid_num);
+  }
+
+  // Now check that each test throws an invalid string error
+  for (const auto &invalid_test : invalid_tests) {
+    auto *ptr = invalid_test.data();
+    auto len = static_cast<uint32_t>(invalid_test.length());
+    EXPECT_THROW(function::NumericFunctions::InputDecimal(decimal, ptr, len),
+                 std::runtime_error)
+        << "Input '" << invalid_test
+        << "' expected to throw error, but passed parsing logic";
+  }
+}
+
 }  // namespace test
 }  // namespace peloton