You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by we...@apache.org on 2019/06/13 14:03:36 UTC

[arrow] branch master updated: ARROW-5514: [C++] Fix pretty-printing uint64 values

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

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


The following commit(s) were added to refs/heads/master by this push:
     new 35e0c7c  ARROW-5514: [C++] Fix pretty-printing uint64 values
35e0c7c is described below

commit 35e0c7cf4cf0811b4a2daf63ffd0227e356db53a
Author: Antoine Pitrou <an...@python.org>
AuthorDate: Thu Jun 13 09:03:24 2019 -0500

    ARROW-5514: [C++] Fix pretty-printing uint64 values
    
    uint64 values above 2**63 would be printed as negative.
    
    Author: Antoine Pitrou <an...@python.org>
    
    Closes #4545 from pitrou/ARROW-5514-pprint-uint64 and squashes the following commits:
    
    b0c8f692a <Antoine Pitrou> ARROW-5514:  Fix pretty-printing uint64 values
---
 cpp/src/arrow/pretty_print-test.cc | 39 ++++++++++++++++++++++++++++++++++++++
 cpp/src/arrow/pretty_print.cc      |  4 +++-
 cpp/src/arrow/util/int-util.h      | 16 ++++++++++++++++
 3 files changed, 58 insertions(+), 1 deletion(-)

diff --git a/cpp/src/arrow/pretty_print-test.cc b/cpp/src/arrow/pretty_print-test.cc
index 8171798..1aed4d7 100644
--- a/cpp/src/arrow/pretty_print-test.cc
+++ b/cpp/src/arrow/pretty_print-test.cc
@@ -165,6 +165,45 @@ TEST_F(TestPrettyPrint, PrimitiveType) {
   CheckPrimitive<StringType, std::string>({2, 10}, is_valid, values3, ex3_in2);
 }
 
+TEST_F(TestPrettyPrint, Int8) {
+  static const char* expected = R"expected([
+  0,
+  127,
+  -128
+])expected";
+  CheckPrimitive<Int8Type, int8_t>({0, 10}, {true, true, true}, {0, 127, -128}, expected);
+}
+
+TEST_F(TestPrettyPrint, UInt8) {
+  static const char* expected = R"expected([
+  0,
+  255
+])expected";
+  CheckPrimitive<UInt8Type, uint8_t>({0, 10}, {true, true}, {0, 255}, expected);
+}
+
+TEST_F(TestPrettyPrint, Int64) {
+  static const char* expected = R"expected([
+  0,
+  9223372036854775807,
+  -9223372036854775808
+])expected";
+  CheckPrimitive<Int64Type, int64_t>(
+      {0, 10}, {true, true, true}, {0, 9223372036854775807LL, -9223372036854775807LL - 1},
+      expected);
+}
+
+TEST_F(TestPrettyPrint, UInt64) {
+  static const char* expected = R"expected([
+  0,
+  9223372036854775803,
+  18446744073709551615
+])expected";
+  CheckPrimitive<UInt64Type, uint64_t>(
+      {0, 10}, {true, true, true}, {0, 9223372036854775803ULL, 18446744073709551615ULL},
+      expected);
+}
+
 TEST_F(TestPrettyPrint, DateTimeTypes) {
   std::vector<bool> is_valid = {true, true, false, true, false};
 
diff --git a/cpp/src/arrow/pretty_print.cc b/cpp/src/arrow/pretty_print.cc
index 695abc1..175514b 100644
--- a/cpp/src/arrow/pretty_print.cc
+++ b/cpp/src/arrow/pretty_print.cc
@@ -33,6 +33,7 @@
 #include "arrow/type.h"
 #include "arrow/type_traits.h"
 #include "arrow/util/checked_cast.h"
+#include "arrow/util/int-util.h"
 #include "arrow/util/string.h"
 #include "arrow/vendored/datetime.h"
 #include "arrow/visitor_inline.h"
@@ -151,7 +152,8 @@ class ArrayPrinter : public PrettyPrinter {
                                  Status>::type
   WriteDataValues(const T& array) {
     const auto data = array.raw_values();
-    WriteValues(array, [&](int64_t i) { (*sink_) << static_cast<int64_t>(data[i]); });
+    // Need to upcast integers to avoid selecting operator<<(char)
+    WriteValues(array, [&](int64_t i) { (*sink_) << internal::UpcastInt(data[i]); });
     return Status::OK();
   }
 
diff --git a/cpp/src/arrow/util/int-util.h b/cpp/src/arrow/util/int-util.h
index d3ae09f..da45473 100644
--- a/cpp/src/arrow/util/int-util.h
+++ b/cpp/src/arrow/util/int-util.h
@@ -83,6 +83,22 @@ SignedInt SafeLeftShift(SignedInt u, Shift shift) {
   return static_cast<SignedInt>(static_cast<UnsignedInt>(u) << shift);
 }
 
+/// Upcast an integer to the largest possible width (currently 64 bits)
+
+template <typename Integer>
+typename std::enable_if<
+    std::is_integral<Integer>::value && std::is_signed<Integer>::value, int64_t>::type
+UpcastInt(Integer v) {
+  return v;
+}
+
+template <typename Integer>
+typename std::enable_if<
+    std::is_integral<Integer>::value && std::is_unsigned<Integer>::value, uint64_t>::type
+UpcastInt(Integer v) {
+  return v;
+}
+
 }  // namespace internal
 }  // namespace arrow