You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by aw...@apache.org on 2019/08/24 00:59:13 UTC

[kudu] 04/05: [util] Add ExtractFloat method to JsonReader and improve ExtractDouble method

This is an automated email from the ASF dual-hosted git repository.

awong pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git

commit 81353eb38d86694eefeb5fbacef300079f2f58c6
Author: zhangyifan27 <ch...@163.com>
AuthorDate: Mon Aug 19 17:51:42 2019 +0800

    [util] Add ExtractFloat method to JsonReader and improve ExtractDouble method
    
    ExtractDouble method should not only parse double values, int values
    that can be losslessly converted to double should also be parsed.
    
    Though there is already IsLosslessFloat() in rapidjson library,
    the implementation has some precision issues.
    This patch re-implement the method.
    
    Change-Id: Ica0a04fb21daceaa44233995a42a60686f59361b
    Reviewed-on: http://gerrit.cloudera.org:8080/14114
    Tested-by: Kudu Jenkins
    Reviewed-by: Andrew Wong <aw...@cloudera.com>
---
 src/kudu/tools/tool_action_table.cc |  6 ++--
 src/kudu/util/jsonreader-test.cc    | 58 +++++++++++++++++++++++++++++++++++--
 src/kudu/util/jsonreader.cc         | 46 ++++++++++++++++++++++++-----
 src/kudu/util/jsonreader.h          |  6 ++++
 4 files changed, 103 insertions(+), 13 deletions(-)

diff --git a/src/kudu/tools/tool_action_table.cc b/src/kudu/tools/tool_action_table.cc
index 5739186..6fcd83c 100644
--- a/src/kudu/tools/tool_action_table.cc
+++ b/src/kudu/tools/tool_action_table.cc
@@ -708,10 +708,10 @@ Status ParseValueOfType(const string& default_value,
       break;
     }
     case KuduColumnSchema::DataType::FLOAT: {
-      double double_value;
+      float float_value;
       RETURN_NOT_OK_PREPEND(
-        reader.ExtractDouble(values[0], /*field=*/nullptr, &double_value), msg);
-      *value = KuduValue::FromFloat(double_value);
+        reader.ExtractFloat(values[0], /*field=*/nullptr, &float_value), msg);
+      *value = KuduValue::FromFloat(float_value);
       break;
     }
     case KuduColumnSchema::DataType::DOUBLE: {
diff --git a/src/kudu/util/jsonreader-test.cc b/src/kudu/util/jsonreader-test.cc
index b321538..0b4a58c 100644
--- a/src/kudu/util/jsonreader-test.cc
+++ b/src/kudu/util/jsonreader-test.cc
@@ -59,6 +59,7 @@ TEST(JsonReaderTest, Empty) {
   ASSERT_TRUE(r.ExtractUint32(r.root(), "foo", nullptr).IsNotFound());
   ASSERT_TRUE(r.ExtractUint64(r.root(), "foo", nullptr).IsNotFound());
   ASSERT_TRUE(r.ExtractDouble(r.root(), "foo", nullptr).IsNotFound());
+  ASSERT_TRUE(r.ExtractFloat(r.root(), "foo", nullptr).IsNotFound());
   ASSERT_TRUE(r.ExtractString(r.root(), "foo", nullptr).IsNotFound());
   ASSERT_TRUE(r.ExtractObject(r.root(), "foo", nullptr).IsNotFound());
   ASSERT_TRUE(r.ExtractObjectArray(r.root(), "foo", nullptr).IsNotFound());
@@ -78,6 +79,7 @@ TEST(JsonReaderTest, Basic) {
   ASSERT_TRUE(r.ExtractUint32(r.root(), "foo", nullptr).IsInvalidArgument());
   ASSERT_TRUE(r.ExtractUint64(r.root(), "foo", nullptr).IsInvalidArgument());
   ASSERT_TRUE(r.ExtractDouble(r.root(), "foo", nullptr).IsInvalidArgument());
+  ASSERT_TRUE(r.ExtractFloat(r.root(), "foo", nullptr).IsInvalidArgument());
   ASSERT_TRUE(r.ExtractObject(r.root(), "foo", nullptr).IsInvalidArgument());
   ASSERT_TRUE(r.ExtractObjectArray(r.root(), "foo", nullptr).IsInvalidArgument());
 }
@@ -121,6 +123,7 @@ TEST(JsonReaderTest, LessBasic) {
   ASSERT_TRUE(r.ExtractUint32(r.root(), "null", nullptr).IsInvalidArgument());
   ASSERT_TRUE(r.ExtractUint64(r.root(), "null", nullptr).IsInvalidArgument());
   ASSERT_TRUE(r.ExtractDouble(r.root(), "null", nullptr).IsInvalidArgument());
+  ASSERT_TRUE(r.ExtractFloat(r.root(), "null", nullptr).IsInvalidArgument());
   ASSERT_TRUE(r.ExtractObject(r.root(), "null", nullptr).IsInvalidArgument());
   ASSERT_TRUE(r.ExtractObjectArray(r.root(), "null", nullptr).IsInvalidArgument());
 
@@ -130,6 +133,7 @@ TEST(JsonReaderTest, LessBasic) {
   ASSERT_TRUE(r.ExtractUint32(r.root(), "empty", nullptr).IsInvalidArgument());
   ASSERT_TRUE(r.ExtractUint64(r.root(), "empty", nullptr).IsInvalidArgument());
   ASSERT_TRUE(r.ExtractDouble(r.root(), "empty", nullptr).IsInvalidArgument());
+  ASSERT_TRUE(r.ExtractFloat(r.root(), "empty", nullptr).IsInvalidArgument());
   ASSERT_TRUE(r.ExtractObject(r.root(), "empty", nullptr).IsInvalidArgument());
   ASSERT_TRUE(r.ExtractObjectArray(r.root(), "empty", nullptr).IsInvalidArgument());
 
@@ -138,6 +142,7 @@ TEST(JsonReaderTest, LessBasic) {
   ASSERT_TRUE(r.ExtractUint32(r.root(), "bool", nullptr).IsInvalidArgument());
   ASSERT_TRUE(r.ExtractUint64(r.root(), "bool", nullptr).IsInvalidArgument());
   ASSERT_TRUE(r.ExtractDouble(r.root(), "bool", nullptr).IsInvalidArgument());
+  ASSERT_TRUE(r.ExtractFloat(r.root(), "bool", nullptr).IsInvalidArgument());
   ASSERT_TRUE(r.ExtractString(r.root(), "bool", nullptr).IsInvalidArgument());
   ASSERT_TRUE(r.ExtractObject(r.root(), "bool", nullptr).IsInvalidArgument());
   ASSERT_TRUE(r.ExtractObjectArray(r.root(), "bool", nullptr).IsInvalidArgument());
@@ -168,6 +173,12 @@ TEST(JsonReaderTest, SignedAndUnsignedInts) {
   ASSERT_EQ(-1, negative64);
   ASSERT_TRUE(r.ExtractUint32(r.root(), negative, nullptr).IsInvalidArgument());
   ASSERT_TRUE(r.ExtractUint64(r.root(), negative, nullptr).IsInvalidArgument());
+  double negative_double;
+  ASSERT_OK(r.ExtractDouble(r.root(), negative, &negative_double));
+  ASSERT_EQ(-1, negative_double);
+  float negative_float;
+  ASSERT_OK(r.ExtractFloat(r.root(), negative, &negative_float));
+  ASSERT_EQ(-1, negative_float);
 
   // Max signed 32-bit integer.
   const char* const signed_big32 = "signed_big32";
@@ -183,6 +194,10 @@ TEST(JsonReaderTest, SignedAndUnsignedInts) {
   uint64_t signed_big32_uint64;
   ASSERT_OK(r.ExtractUint64(r.root(), signed_big32, &signed_big32_uint64));
   ASSERT_EQ(kMaxInt32, signed_big32_uint64);
+  double signed_big32_double;
+  ASSERT_OK(r.ExtractDouble(r.root(), signed_big32, &signed_big32_double));
+  ASSERT_EQ(kMaxInt32, signed_big32_double);
+  ASSERT_TRUE(r.ExtractFloat(r.root(), signed_big32, nullptr).IsInvalidArgument());
 
   // Max signed 64-bit integer.
   const char* const signed_big64 = "signed_big64";
@@ -195,6 +210,7 @@ TEST(JsonReaderTest, SignedAndUnsignedInts) {
   ASSERT_OK(r.ExtractUint64(r.root(), signed_big64, &signed_big64_uint64));
   ASSERT_EQ(kMaxInt64, signed_big64_uint64);
   ASSERT_TRUE(r.ExtractDouble(r.root(), signed_big64, nullptr).IsInvalidArgument());
+  ASSERT_TRUE(r.ExtractFloat(r.root(), signed_big64, nullptr).IsInvalidArgument());
 
   // Max unsigned 32-bit integer.
   const char* const unsigned_big32 = "unsigned_big32";
@@ -208,6 +224,10 @@ TEST(JsonReaderTest, SignedAndUnsignedInts) {
   uint64_t unsigned_big32_uint64;
   ASSERT_OK(r.ExtractUint64(r.root(), unsigned_big32, &unsigned_big32_uint64));
   ASSERT_EQ(kMaxUint32, unsigned_big32_uint64);
+  double unsigned_big32_double;
+  ASSERT_OK(r.ExtractDouble(r.root(), unsigned_big32, &unsigned_big32_double));
+  ASSERT_EQ(kMaxUint32, unsigned_big32_double);
+  ASSERT_TRUE(r.ExtractFloat(r.root(), unsigned_big32, nullptr).IsInvalidArgument());
 
   // Max unsigned 64-bit integer.
   const char* const unsigned_big64 = "unsigned_big64";
@@ -218,6 +238,7 @@ TEST(JsonReaderTest, SignedAndUnsignedInts) {
   ASSERT_OK(r.ExtractUint64(r.root(), unsigned_big64, &unsigned_big64_uint64));
   ASSERT_EQ(kMaxUint64, unsigned_big64_uint64);
   ASSERT_TRUE(r.ExtractDouble(r.root(), unsigned_big64, nullptr).IsInvalidArgument());
+  ASSERT_TRUE(r.ExtractFloat(r.root(), unsigned_big64, nullptr).IsInvalidArgument());
 
   // Min signed 32-bit integer.
   const char* const signed_small32 = "signed_small32";
@@ -229,8 +250,14 @@ TEST(JsonReaderTest, SignedAndUnsignedInts) {
   ASSERT_EQ(kMinInt32, small32_int64);
   ASSERT_TRUE(r.ExtractUint32(r.root(), signed_small32, nullptr).IsInvalidArgument());
   ASSERT_TRUE(r.ExtractUint64(r.root(), signed_small32, nullptr).IsInvalidArgument());
-
-  // Min signed 32-bit integer.
+  double small32_double;
+  ASSERT_OK(r.ExtractDouble(r.root(), signed_small32, &small32_double));
+  ASSERT_EQ(kMinInt32, small32_double);
+  float small32_float;
+  ASSERT_OK(r.ExtractFloat(r.root(), signed_small32, &small32_float));
+  ASSERT_EQ(kMinInt32, small32_float);
+
+  // Min signed 64-bit integer.
   const char* const signed_small64 = "signed_small64";
   ASSERT_TRUE(r.ExtractInt32(r.root(), signed_small64, nullptr).IsInvalidArgument());
   int64_t small64_int64;
@@ -238,6 +265,12 @@ TEST(JsonReaderTest, SignedAndUnsignedInts) {
   ASSERT_EQ(kMinInt64, small64_int64);
   ASSERT_TRUE(r.ExtractUint32(r.root(), signed_small64, nullptr).IsInvalidArgument());
   ASSERT_TRUE(r.ExtractUint64(r.root(), signed_small64, nullptr).IsInvalidArgument());
+  double small64_double;
+  ASSERT_OK(r.ExtractDouble(r.root(), signed_small64, &small64_double));
+  ASSERT_EQ(kMinInt64, small64_double);
+  float small64_float;
+  ASSERT_OK(r.ExtractFloat(r.root(), signed_small64, &small64_float));
+  ASSERT_EQ(kMinInt64, small64_float);
 }
 
 TEST(JsonReaderTest, Doubles) {
@@ -259,6 +292,25 @@ TEST(JsonReaderTest, Doubles) {
   ASSERT_TRUE(r.ExtractObjectArray(r.root(), "foo", nullptr).IsInvalidArgument());
 }
 
+TEST(JsonReaderTest, Floats) {
+  JsonReader r("{ \"foo\" : 5.125 }");
+  ASSERT_OK(r.Init());
+
+  float foo;
+  ASSERT_OK(r.ExtractFloat(r.root(), "foo", &foo));
+  ASSERT_EQ(5.125, foo);
+
+  // Bad types.
+  ASSERT_TRUE(r.ExtractBool(r.root(), "foo", nullptr).IsInvalidArgument());
+  ASSERT_TRUE(r.ExtractInt32(r.root(), "foo", nullptr).IsInvalidArgument());
+  ASSERT_TRUE(r.ExtractInt64(r.root(), "foo", nullptr).IsInvalidArgument());
+  ASSERT_TRUE(r.ExtractUint32(r.root(), "foo", nullptr).IsInvalidArgument());
+  ASSERT_TRUE(r.ExtractUint64(r.root(), "foo", nullptr).IsInvalidArgument());
+  ASSERT_TRUE(r.ExtractString(r.root(), "foo", nullptr).IsInvalidArgument());
+  ASSERT_TRUE(r.ExtractObject(r.root(), "foo", nullptr).IsInvalidArgument());
+  ASSERT_TRUE(r.ExtractObjectArray(r.root(), "foo", nullptr).IsInvalidArgument());
+}
+
 TEST(JsonReaderTest, Objects) {
   JsonReader r("{ \"foo\" : { \"1\" : 1 } }");
   ASSERT_OK(r.Init());
@@ -278,6 +330,7 @@ TEST(JsonReaderTest, Objects) {
   ASSERT_TRUE(r.ExtractUint32(r.root(), "foo", nullptr).IsInvalidArgument());
   ASSERT_TRUE(r.ExtractUint64(r.root(), "foo", nullptr).IsInvalidArgument());
   ASSERT_TRUE(r.ExtractDouble(r.root(), "foo", nullptr).IsInvalidArgument());
+  ASSERT_TRUE(r.ExtractFloat(r.root(), "foo", nullptr).IsInvalidArgument());
   ASSERT_TRUE(r.ExtractString(r.root(), "foo", nullptr).IsInvalidArgument());
   ASSERT_TRUE(r.ExtractObjectArray(r.root(), "foo", nullptr).IsInvalidArgument());
 }
@@ -302,6 +355,7 @@ TEST(JsonReaderTest, TopLevelArray) {
   ASSERT_TRUE(r.ExtractUint32(r.root(), nullptr, nullptr).IsInvalidArgument());
   ASSERT_TRUE(r.ExtractUint64(r.root(), nullptr, nullptr).IsInvalidArgument());
   ASSERT_TRUE(r.ExtractDouble(r.root(), nullptr, nullptr).IsInvalidArgument());
+  ASSERT_TRUE(r.ExtractFloat(r.root(), nullptr, nullptr).IsInvalidArgument());
   ASSERT_TRUE(r.ExtractString(r.root(), nullptr, nullptr).IsInvalidArgument());
   ASSERT_TRUE(r.ExtractObject(r.root(), nullptr, nullptr).IsInvalidArgument());
 }
diff --git a/src/kudu/util/jsonreader.cc b/src/kudu/util/jsonreader.cc
index 05cde80..d6f233a 100644
--- a/src/kudu/util/jsonreader.cc
+++ b/src/kudu/util/jsonreader.cc
@@ -20,6 +20,7 @@
 #include <ostream>
 #include <utility>
 
+#include <cmath>
 #include <glog/logging.h>
 #include <rapidjson/error/en.h>
 #include <rapidjson/rapidjson.h>
@@ -121,7 +122,8 @@ Status JsonReader::ExtractInt64(const Value* object,
   if (PREDICT_FALSE(!val->IsInt64())) {
     return Status::InvalidArgument(Substitute(
         "wrong type during field extraction: expected int64 but got $0",
-        TypeToString(val->GetType())));  }
+        TypeToString(val->GetType())));
+  }
   *result = val->GetInt64();
   return Status::OK();
 }
@@ -134,7 +136,8 @@ Status JsonReader::ExtractUint64(const Value* object,
   if (PREDICT_FALSE(!val->IsUint64())) {
     return Status::InvalidArgument(Substitute(
         "wrong type during field extraction: expected uint64 but got $0",
-        TypeToString(val->GetType())));  }
+        TypeToString(val->GetType())));
+  }
   *result = val->GetUint64();
   return Status::OK();
 }
@@ -144,14 +147,29 @@ Status JsonReader::ExtractDouble(const Value* object,
                                  double* result) const {
   const Value* val;
   RETURN_NOT_OK(ExtractField(object, field, &val));
-  if (PREDICT_FALSE(!val->IsDouble())) {
+  if (PREDICT_FALSE(!val->IsLosslessDouble())) {
     return Status::InvalidArgument(Substitute(
-        "wrong type during field extraction: expected double but got $0",
-        TypeToString(val->GetType())));  }
+        "wrong type during field extraction: expected a lossless double type but got $0",
+        TypeToString(val->GetType())));
+  }
   *result = val->GetDouble();
   return Status::OK();
 }
 
+Status JsonReader::ExtractFloat(const Value* object,
+                                const char* field,
+                                float* result) const {
+  const Value* val;
+  RETURN_NOT_OK(ExtractField(object, field, &val));
+  if (PREDICT_FALSE(!IsLosslessFloatValue(val))) {
+    return Status::InvalidArgument(Substitute(
+        "wrong type during field extraction: expected a lossless float type but got $0",
+        TypeToString(val->GetType())));
+  }
+  *result = val->GetFloat();
+  return Status::OK();
+}
+
 Status JsonReader::ExtractString(const Value* object,
                                  const char* field,
                                  string* result) const {
@@ -164,7 +182,8 @@ Status JsonReader::ExtractString(const Value* object,
     }
     return Status::InvalidArgument(Substitute(
         "wrong type during field extraction: expected string but got $0",
-        TypeToString(val->GetType())));  }
+        TypeToString(val->GetType())));
+  }
   result->assign(val->GetString());
   return Status::OK();
 }
@@ -177,7 +196,8 @@ Status JsonReader::ExtractObject(const Value* object,
   if (PREDICT_FALSE(!val->IsObject())) {
     return Status::InvalidArgument(Substitute(
         "wrong type during field extraction: expected object but got $0",
-        TypeToString(val->GetType())));  }
+        TypeToString(val->GetType())));
+  }
   *result = val;
   return Status::OK();
 }
@@ -190,7 +210,8 @@ Status JsonReader::ExtractObjectArray(const Value* object,
   if (PREDICT_FALSE(!val->IsArray())) {
     return Status::InvalidArgument(Substitute(
         "wrong type during field extraction: expected object array but got $0",
-        TypeToString(val->GetType())));  }
+        TypeToString(val->GetType())));
+  }
   for (Value::ConstValueIterator iter = val->Begin(); iter != val->End(); ++iter) {
     result->push_back(iter);
   }
@@ -207,4 +228,13 @@ Status JsonReader::ExtractField(const Value* object,
   return Status::OK();
 }
 
+bool JsonReader::IsLosslessFloatValue(const Value* value) const {
+  if (!value->IsLosslessDouble()) {
+    return false;
+  }
+  double a = value->GetDouble();
+  double b = static_cast<double>(static_cast<float>(a));
+  return fabs(a-b) <= 1e-7;
+}
+
 } // namespace kudu
diff --git a/src/kudu/util/jsonreader.h b/src/kudu/util/jsonreader.h
index be2412b..125b762 100644
--- a/src/kudu/util/jsonreader.h
+++ b/src/kudu/util/jsonreader.h
@@ -76,6 +76,10 @@ class JsonReader {
                        const char* field,
                        double* result) const;
 
+  Status ExtractFloat(const rapidjson::Value* object,
+                      const char* field,
+                      float* result) const;
+
   // 'result' is only valid for as long as JsonReader is alive.
   Status ExtractObject(const rapidjson::Value* object,
                        const char* field,
@@ -93,6 +97,8 @@ class JsonReader {
                       const char* field,
                       const rapidjson::Value** result) const;
 
+  bool IsLosslessFloatValue(const rapidjson::Value* value) const;
+
   std::string text_;
   rapidjson::Document document_;