Skip to content

Commit 3188d70

Browse files
cpcloudwesm
cpcloud
authored and
wesm
committed
ARROW-1716: [Format/JSON] Use string integer value for Decimals in JSON
Author: Phillip Cloud <cpcloud@gmail.com> Closes apache#1267 from cpcloud/ARROW-1716 and squashes the following commits: b4f3aed [Phillip Cloud] Add cases for every valid precision f8d4391 [Phillip Cloud] Use the full range of decimal values in integration tests 6fef5f7 [Phillip Cloud] ARROW-1716: [Format/JSON] Use string integer value for Decimals in JSON
1 parent e631119 commit 3188d70

7 files changed

Lines changed: 165 additions & 75 deletions

File tree

‎cpp/src/arrow/ipc/ipc-read-write-test.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -727,7 +727,7 @@ TEST_F(TestTensorRoundTrip, BasicRoundtrip) {
727727
int64_t size = 24;
728728

729729
std::vector<int64_t> values;
730-
test::randint<int64_t>(size, 0, 100, &values);
730+
test::randint(size, 0, 100, &values);
731731

732732
auto data = test::GetBufferFromVector(values);
733733

@@ -748,7 +748,7 @@ TEST_F(TestTensorRoundTrip, NonContiguous) {
748748
ASSERT_OK(io::MemoryMapFixture::InitMemoryMap(kBufferSize, path, &mmap_));
749749

750750
std::vector<int64_t> values;
751-
test::randint<int64_t>(24, 0, 100, &values);
751+
test::randint(24, 0, 100, &values);
752752

753753
auto data = test::GetBufferFromVector(values);
754754
Tensor tensor(int64(), data, {4, 3}, {48, 16});

‎cpp/src/arrow/ipc/json-internal.cc‎

Lines changed: 55 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
#include "arrow/type.h"
3434
#include "arrow/type_traits.h"
3535
#include "arrow/util/bit-util.h"
36+
#include "arrow/util/decimal.h"
3637
#include "arrow/util/logging.h"
3738
#include "arrow/util/string.h"
3839
#include "arrow/visitor_inline.h"
@@ -448,14 +449,22 @@ class ArrayWriter {
448449
}
449450

450451
void WriteDataValues(const FixedSizeBinaryArray& arr) {
451-
int32_t width = arr.byte_width();
452+
const int32_t width = arr.byte_width();
453+
452454
for (int64_t i = 0; i < arr.length(); ++i) {
453455
const uint8_t* buf = arr.GetValue(i);
454456
std::string encoded = HexEncode(buf, width);
455457
writer_->String(encoded);
456458
}
457459
}
458460

461+
void WriteDataValues(const DecimalArray& arr) {
462+
for (int64_t i = 0; i < arr.length(); ++i) {
463+
const Decimal128 value(arr.GetValue(i));
464+
writer_->String(value.ToIntegerString());
465+
}
466+
}
467+
459468
void WriteDataValues(const BooleanArray& arr) {
460469
for (int i = 0; i < arr.length(); ++i) {
461470
writer_->Bool(arr.Value(i));
@@ -1053,7 +1062,9 @@ class ArrayReader {
10531062
}
10541063

10551064
template <typename T>
1056-
typename std::enable_if<std::is_base_of<FixedSizeBinaryType, T>::value, Status>::type
1065+
typename std::enable_if<std::is_base_of<FixedSizeBinaryType, T>::value &&
1066+
!std::is_base_of<DecimalType, T>::value,
1067+
Status>::type
10571068
Visit(const T& type) {
10581069
typename TypeTraits<T>::BuilderType builder(type_, pool_);
10591070

@@ -1073,22 +1084,52 @@ class ArrayReader {
10731084
for (int i = 0; i < length_; ++i) {
10741085
if (!is_valid_[i]) {
10751086
RETURN_NOT_OK(builder.AppendNull());
1076-
continue;
1077-
}
1087+
} else {
1088+
const rj::Value& val = json_data_arr[i];
1089+
DCHECK(val.IsString())
1090+
<< "Found non-string JSON value when parsing FixedSizeBinary value";
1091+
std::string hex_string = val.GetString();
1092+
if (static_cast<int32_t>(hex_string.size()) != byte_width * 2) {
1093+
DCHECK(false) << "Expected size: " << byte_width * 2
1094+
<< " got: " << hex_string.size();
1095+
}
1096+
const char* hex_data = hex_string.c_str();
10781097

1079-
const rj::Value& val = json_data_arr[i];
1080-
DCHECK(val.IsString());
1081-
std::string hex_string = val.GetString();
1082-
if (static_cast<int32_t>(hex_string.size()) != byte_width * 2) {
1083-
DCHECK(false) << "Expected size: " << byte_width * 2
1084-
<< " got: " << hex_string.size();
1098+
for (int32_t j = 0; j < byte_width; ++j) {
1099+
RETURN_NOT_OK(ParseHexValue(hex_data + j * 2, &byte_buffer_data[j]));
1100+
}
1101+
RETURN_NOT_OK(builder.Append(byte_buffer_data));
10851102
}
1086-
const char* hex_data = hex_string.c_str();
1103+
}
1104+
return builder.Finish(&result_);
1105+
}
1106+
1107+
template <typename T>
1108+
typename std::enable_if<std::is_base_of<DecimalType, T>::value, Status>::type Visit(
1109+
const T& type) {
1110+
typename TypeTraits<T>::BuilderType builder(type_, pool_);
1111+
1112+
const auto& json_data = obj_->FindMember("DATA");
1113+
RETURN_NOT_ARRAY("DATA", json_data, *obj_);
10871114

1088-
for (int32_t j = 0; j < byte_width; ++j) {
1089-
RETURN_NOT_OK(ParseHexValue(hex_data + j * 2, &byte_buffer_data[j]));
1115+
const auto& json_data_arr = json_data->value.GetArray();
1116+
1117+
DCHECK_EQ(static_cast<int32_t>(json_data_arr.Size()), length_);
1118+
1119+
for (int i = 0; i < length_; ++i) {
1120+
if (!is_valid_[i]) {
1121+
RETURN_NOT_OK(builder.AppendNull());
1122+
} else {
1123+
const rj::Value& val = json_data_arr[i];
1124+
DCHECK(val.IsString())
1125+
<< "Found non-string JSON value when parsing Decimal128 value";
1126+
DCHECK_GT(val.GetStringLength(), 0)
1127+
<< "Empty string found when parsing Decimal128 value";
1128+
1129+
Decimal128 value;
1130+
RETURN_NOT_OK(Decimal128::FromString(val.GetString(), &value));
1131+
RETURN_NOT_OK(builder.Append(value));
10901132
}
1091-
RETURN_NOT_OK(builder.Append(byte_buffer_data));
10921133
}
10931134
return builder.Finish(&result_);
10941135
}

‎cpp/src/arrow/ipc/test-common.h‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -671,8 +671,11 @@ Status MakeFWBinary(std::shared_ptr<RecordBatch>* out) {
671671
}
672672

673673
Status MakeDecimal(std::shared_ptr<RecordBatch>* out) {
674-
auto f0 = field("f0", decimal(19, 4));
675-
auto schema = ::arrow::schema({f0, f0});
674+
constexpr int kDecimalPrecision = 38;
675+
auto type = decimal(kDecimalPrecision, 4);
676+
auto f0 = field("f0", type);
677+
auto f1 = field("f1", type);
678+
auto schema = ::arrow::schema({f0, f1});
676679

677680
constexpr int kDecimalSize = 16;
678681
constexpr int length = 10;
@@ -682,18 +685,18 @@ Status MakeDecimal(std::shared_ptr<RecordBatch>* out) {
682685

683686
RETURN_NOT_OK(AllocateBuffer(default_memory_pool(), kDecimalSize * length, &data));
684687

685-
test::random_bytes(kDecimalSize * length, 0, data->mutable_data());
688+
test::random_decimals(length, 1, kDecimalPrecision, data->mutable_data());
686689
test::random_null_bytes(length, 0.1, is_valid_bytes.data());
687690

688691
RETURN_NOT_OK(BitUtil::BytesToBits(is_valid_bytes, default_memory_pool(), &is_valid));
689692

690693
auto a1 = std::make_shared<DecimalArray>(f0->type(), length, data, is_valid,
691694
kUnknownNullCount);
692695

693-
auto a2 = std::make_shared<DecimalArray>(f0->type(), length, data);
696+
auto a2 = std::make_shared<DecimalArray>(f1->type(), length, data);
694697

695698
ArrayVector arrays = {a1, a2};
696-
*out = std::make_shared<RecordBatch>(schema, a1->length(), arrays);
699+
*out = std::make_shared<RecordBatch>(schema, length, arrays);
697700
return Status::OK();
698701
}
699702

0 commit comments

Comments
 (0)