Skip to content

Commit 754bcce

Browse files
cpcloudwesm
authored andcommitted
ARROW-655: [C++/Python] Implement DecimalArray
Adds Decimal support for C++ and Python. TODOs: - [x] Tighten up some of the GIL acquisition. E.g., we may not need to hold it when importing the decimal module if we acquire it where we import the decimal module. - [x] Investigate FreeBSD issue (manifesting on OS X) where typeinfo symbols for `__int128_t` are not exported: https://bugs.llvm.org//show_bug.cgi?id=26156. - [x] See if there's a better way to visit scalar decimals, rather than keeping extra state on the class. Seems like an unacceptable hack. Author: Phillip Cloud <cpcloud@gmail.com> Closes apache#403 from cpcloud/decimal and squashes the following commits: e5470fd [Phillip Cloud] Remove unnecessary header in helpers.h 07713a7 [Phillip Cloud] Remove more boost leakage f764156 [Phillip Cloud] Revert "Transitively link static libs as well" a7109b2 [Phillip Cloud] Transitively link static libs as well bf2a7ea [Phillip Cloud] Move IsNegative to cc file cb2c1ac [Phillip Cloud] Do not link boost regex to jemalloc e63b766 [Phillip Cloud] Remove python extra cmake args 805bbac [Phillip Cloud] ARROW-655: [C++/Python] Implement DecimalArray
1 parent 449f991 commit 754bcce

40 files changed

Lines changed: 1497 additions & 80 deletions

.travis.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ addons:
1414
- valgrind
1515
- libboost-dev
1616
- libboost-filesystem-dev
17+
- libboost-regex-dev
1718
- libboost-system-dev
1819
- libjemalloc-dev
1920
- gtk-doc-tools

cpp/CMakeLists.txt

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -398,30 +398,36 @@ if (ARROW_BOOST_USE_SHARED)
398398
add_definitions(-DBOOST_ALL_DYN_LINK)
399399
endif()
400400

401-
find_package(Boost COMPONENTS system filesystem REQUIRED)
401+
find_package(Boost COMPONENTS system filesystem regex REQUIRED)
402402
if ("${CMAKE_BUILD_TYPE}" STREQUAL "DEBUG")
403403
set(BOOST_SHARED_SYSTEM_LIBRARY ${Boost_SYSTEM_LIBRARY_DEBUG})
404404
set(BOOST_SHARED_FILESYSTEM_LIBRARY ${Boost_FILESYSTEM_LIBRARY_DEBUG})
405+
set(BOOST_SHARED_REGEX_LIBRARY ${Boost_REGEX_LIBRARY_DEBUG})
405406
else()
406407
set(BOOST_SHARED_SYSTEM_LIBRARY ${Boost_SYSTEM_LIBRARY_RELEASE})
407408
set(BOOST_SHARED_FILESYSTEM_LIBRARY ${Boost_FILESYSTEM_LIBRARY_RELEASE})
409+
set(BOOST_SHARED_REGEX_LIBRARY ${Boost_REGEX_LIBRARY_RELEASE})
408410
endif()
409411
set(BOOST_SYSTEM_LIBRARY boost_system_shared)
410412
set(BOOST_FILESYSTEM_LIBRARY boost_filesystem_shared)
413+
set(BOOST_REGEX_LIBRARY boost_regex_shared)
411414
else()
412415
# Find static boost headers and libs
413416
# TODO Differentiate here between release and debug builds
414417
set(Boost_USE_STATIC_LIBS ON)
415-
find_package(Boost COMPONENTS system filesystem REQUIRED)
418+
find_package(Boost COMPONENTS system filesystem regex REQUIRED)
416419
if ("${CMAKE_BUILD_TYPE}" STREQUAL "DEBUG")
417420
set(BOOST_STATIC_SYSTEM_LIBRARY ${Boost_SYSTEM_LIBRARY_DEBUG})
418421
set(BOOST_STATIC_FILESYSTEM_LIBRARY ${Boost_FILESYSTEM_LIBRARY_DEBUG})
422+
set(BOOST_STATIC_REGEX_LIBRARY ${Boost_REGEX_LIBRARY_DEBUG})
419423
else()
420424
set(BOOST_STATIC_SYSTEM_LIBRARY ${Boost_SYSTEM_LIBRARY_RELEASE})
421425
set(BOOST_STATIC_FILESYSTEM_LIBRARY ${Boost_FILESYSTEM_LIBRARY_RELEASE})
426+
set(BOOST_STATIC_REGEX_LIBRARY ${Boost_REGEX_LIBRARY_RELEASE})
422427
endif()
423428
set(BOOST_SYSTEM_LIBRARY boost_system_static)
424429
set(BOOST_FILESYSTEM_LIBRARY boost_filesystem_static)
430+
set(BOOST_REGEX_LIBRARY boost_regex_static)
425431
endif()
426432

427433
message(STATUS "Boost include dir: " ${Boost_INCLUDE_DIRS})
@@ -435,7 +441,11 @@ ADD_THIRDPARTY_LIB(boost_filesystem
435441
STATIC_LIB "${BOOST_STATIC_FILESYSTEM_LIBRARY}"
436442
SHARED_LIB "${BOOST_SHARED_FILESYSTEM_LIBRARY}")
437443

438-
SET(ARROW_BOOST_LIBS boost_system boost_filesystem)
444+
ADD_THIRDPARTY_LIB(boost_regex
445+
STATIC_LIB "${BOOST_STATIC_REGEX_LIBRARY}"
446+
SHARED_LIB "${BOOST_SHARED_REGEX_LIBRARY}")
447+
448+
SET(ARROW_BOOST_LIBS boost_system boost_filesystem boost_regex)
439449

440450
include_directories(SYSTEM ${Boost_INCLUDE_DIR})
441451

@@ -695,14 +705,16 @@ endif()
695705
set(ARROW_MIN_TEST_LIBS
696706
arrow_static
697707
arrow_test_main
698-
${ARROW_BASE_LIBS})
708+
${ARROW_BASE_LIBS}
709+
${BOOST_REGEX_LIBRARY})
699710

700711
set(ARROW_TEST_LINK_LIBS ${ARROW_MIN_TEST_LIBS})
701712

702713
set(ARROW_BENCHMARK_LINK_LIBS
703714
arrow_static
704715
arrow_benchmark_main
705-
${ARROW_BASE_LIBS})
716+
${ARROW_BASE_LIBS}
717+
${BOOST_REGEX_LIBRARY})
706718

707719
############################################################
708720
# "make ctags" target
@@ -796,7 +808,7 @@ endif()
796808
############################################################
797809
798810
set(ARROW_LINK_LIBS
799-
)
811+
${BOOST_REGEX_LIBRARY})
800812
801813
set(ARROW_PRIVATE_LINK_LIBS
802814
)
@@ -816,6 +828,7 @@ set(ARROW_SRCS
816828
src/arrow/visitor.cc
817829
818830
src/arrow/util/bit-util.cc
831+
src/arrow/util/decimal.cc
819832
)
820833
821834
if(NOT APPLE AND NOT MSVC)
@@ -825,9 +838,11 @@ if(NOT APPLE AND NOT MSVC)
825838
set(ARROW_SHARED_LINK_FLAGS "-Wl,--version-script=${CMAKE_CURRENT_SOURCE_DIR}/src/arrow/symbols.map")
826839
endif()
827840
841+
828842
ADD_ARROW_LIB(arrow
829843
SOURCES ${ARROW_SRCS}
830844
SHARED_LINK_FLAGS ${ARROW_SHARED_LINK_FLAGS}
845+
SHARED_LINK_LIBS ${ARROW_LINK_LIBS}
831846
)
832847
833848
add_subdirectory(src/arrow)

cpp/cmake_modules/FindPythonLibsNew.cmake

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,8 @@ else()
175175
find_library(PYTHON_LIBRARY
176176
NAMES "python${PYTHON_LIBRARY_SUFFIX}"
177177
PATHS ${_PYTHON_LIBS_SEARCH}
178-
NO_SYSTEM_ENVIRONMENT_PATH)
178+
NO_SYSTEM_ENVIRONMENT_PATH
179+
NO_CMAKE_SYSTEM_PATH)
179180
message(STATUS "Found Python lib ${PYTHON_LIBRARY}")
180181
endif()
181182

cpp/src/arrow/array-decimal-test.cc

Lines changed: 192 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,20 +15,210 @@
1515
// specific language governing permissions and limitations
1616
// under the License.
1717

18+
#include "arrow/type.h"
1819
#include "gtest/gtest.h"
1920

20-
#include "arrow/type.h"
21+
#include "arrow/builder.h"
22+
#include "arrow/test-util.h"
23+
#include "arrow/util/decimal.h"
2124

2225
namespace arrow {
2326

24-
TEST(TypesTest, TestDecimalType) {
27+
TEST(TypesTest, TestDecimal32Type) {
2528
DecimalType t1(8, 4);
2629

2730
ASSERT_EQ(t1.type, Type::DECIMAL);
2831
ASSERT_EQ(t1.precision, 8);
2932
ASSERT_EQ(t1.scale, 4);
3033

3134
ASSERT_EQ(t1.ToString(), std::string("decimal(8, 4)"));
35+
36+
// Test properties
37+
ASSERT_EQ(t1.byte_width(), 4);
38+
ASSERT_EQ(t1.bit_width(), 32);
3239
}
3340

41+
TEST(TypesTest, TestDecimal64Type) {
42+
DecimalType t1(12, 5);
43+
44+
ASSERT_EQ(t1.type, Type::DECIMAL);
45+
ASSERT_EQ(t1.precision, 12);
46+
ASSERT_EQ(t1.scale, 5);
47+
48+
ASSERT_EQ(t1.ToString(), std::string("decimal(12, 5)"));
49+
50+
// Test properties
51+
ASSERT_EQ(t1.byte_width(), 8);
52+
ASSERT_EQ(t1.bit_width(), 64);
53+
}
54+
55+
TEST(TypesTest, TestDecimal128Type) {
56+
DecimalType t1(27, 7);
57+
58+
ASSERT_EQ(t1.type, Type::DECIMAL);
59+
ASSERT_EQ(t1.precision, 27);
60+
ASSERT_EQ(t1.scale, 7);
61+
62+
ASSERT_EQ(t1.ToString(), std::string("decimal(27, 7)"));
63+
64+
// Test properties
65+
ASSERT_EQ(t1.byte_width(), 16);
66+
ASSERT_EQ(t1.bit_width(), 128);
67+
}
68+
69+
template <typename T>
70+
class DecimalTestBase {
71+
public:
72+
virtual std::vector<uint8_t> data(
73+
const std::vector<T>& input, size_t byte_width) const = 0;
74+
75+
void test(int precision, const std::vector<T>& draw,
76+
const std::vector<uint8_t>& valid_bytes,
77+
const std::vector<uint8_t>& sign_bitmap = {}, int64_t offset = 0) const {
78+
auto type = std::make_shared<DecimalType>(precision, 4);
79+
int byte_width = type->byte_width();
80+
auto pool = default_memory_pool();
81+
auto builder = std::make_shared<DecimalBuilder>(pool, type);
82+
size_t null_count = 0;
83+
84+
size_t size = draw.size();
85+
builder->Reserve(size);
86+
87+
for (size_t i = 0; i < size; ++i) {
88+
if (valid_bytes[i]) {
89+
builder->Append(draw[i]);
90+
} else {
91+
builder->AppendNull();
92+
++null_count;
93+
}
94+
}
95+
96+
std::shared_ptr<Buffer> expected_sign_bitmap;
97+
if (!sign_bitmap.empty()) {
98+
BitUtil::BytesToBits(sign_bitmap, &expected_sign_bitmap);
99+
}
100+
101+
auto raw_bytes = data(draw, byte_width);
102+
auto expected_data = std::make_shared<Buffer>(raw_bytes.data(), size * byte_width);
103+
auto expected_null_bitmap = test::bytes_to_null_buffer(valid_bytes);
104+
int64_t expected_null_count = test::null_count(valid_bytes);
105+
auto expected = std::make_shared<DecimalArray>(type, size, expected_data,
106+
expected_null_bitmap, expected_null_count, offset, expected_sign_bitmap);
107+
108+
std::shared_ptr<Array> out;
109+
ASSERT_OK(builder->Finish(&out));
110+
ASSERT_TRUE(out->Equals(*expected));
111+
}
112+
};
113+
114+
template <typename T>
115+
class DecimalTest : public DecimalTestBase<T> {
116+
public:
117+
std::vector<uint8_t> data(
118+
const std::vector<T>& input, size_t byte_width) const override {
119+
std::vector<uint8_t> result;
120+
result.reserve(input.size() * byte_width);
121+
// TODO(phillipc): There's probably a better way to do this
122+
constexpr static const size_t bytes_per_element = sizeof(T);
123+
for (size_t i = 0, j = 0; i < input.size(); ++i, j += bytes_per_element) {
124+
*reinterpret_cast<typename T::value_type*>(&result[j]) = input[i].value;
125+
}
126+
return result;
127+
}
128+
};
129+
130+
template <>
131+
class DecimalTest<Decimal128> : public DecimalTestBase<Decimal128> {
132+
public:
133+
std::vector<uint8_t> data(
134+
const std::vector<Decimal128>& input, size_t byte_width) const override {
135+
std::vector<uint8_t> result;
136+
result.reserve(input.size() * byte_width);
137+
constexpr static const size_t bytes_per_element = 16;
138+
for (size_t i = 0; i < input.size(); ++i) {
139+
uint8_t stack_bytes[bytes_per_element] = {0};
140+
uint8_t* bytes = stack_bytes;
141+
bool is_negative;
142+
ToBytes(input[i], &bytes, &is_negative);
143+
144+
for (size_t i = 0; i < bytes_per_element; ++i) {
145+
result.push_back(bytes[i]);
146+
}
147+
}
148+
return result;
149+
}
150+
};
151+
152+
class Decimal32BuilderTest : public ::testing::TestWithParam<int>,
153+
public DecimalTest<Decimal32> {};
154+
155+
class Decimal64BuilderTest : public ::testing::TestWithParam<int>,
156+
public DecimalTest<Decimal64> {};
157+
158+
class Decimal128BuilderTest : public ::testing::TestWithParam<int>,
159+
public DecimalTest<Decimal128> {};
160+
161+
TEST_P(Decimal32BuilderTest, NoNulls) {
162+
int precision = GetParam();
163+
std::vector<Decimal32> draw = {
164+
Decimal32(1), Decimal32(2), Decimal32(2389), Decimal32(4), Decimal32(-12348)};
165+
std::vector<uint8_t> valid_bytes = {true, true, true, true, true};
166+
this->test(precision, draw, valid_bytes);
167+
}
168+
169+
TEST_P(Decimal64BuilderTest, NoNulls) {
170+
int precision = GetParam();
171+
std::vector<Decimal64> draw = {
172+
Decimal64(1), Decimal64(2), Decimal64(2389), Decimal64(4), Decimal64(-12348)};
173+
std::vector<uint8_t> valid_bytes = {true, true, true, true, true};
174+
this->test(precision, draw, valid_bytes);
175+
}
176+
177+
TEST_P(Decimal128BuilderTest, NoNulls) {
178+
int precision = GetParam();
179+
std::vector<Decimal128> draw = {
180+
Decimal128(1), Decimal128(-2), Decimal128(2389), Decimal128(4), Decimal128(-12348)};
181+
std::vector<uint8_t> valid_bytes = {true, true, true, true, true};
182+
std::vector<uint8_t> sign_bitmap = {false, true, false, false, true};
183+
this->test(precision, draw, valid_bytes, sign_bitmap);
184+
}
185+
186+
TEST_P(Decimal32BuilderTest, WithNulls) {
187+
int precision = GetParam();
188+
std::vector<Decimal32> draw = {
189+
Decimal32(1), Decimal32(2), Decimal32(-1), Decimal32(4), Decimal32(-1)};
190+
std::vector<uint8_t> valid_bytes = {true, true, false, true, false};
191+
this->test(precision, draw, valid_bytes);
192+
}
193+
194+
TEST_P(Decimal64BuilderTest, WithNulls) {
195+
int precision = GetParam();
196+
std::vector<Decimal64> draw = {
197+
Decimal64(-1), Decimal64(2), Decimal64(-1), Decimal64(4), Decimal64(-1)};
198+
std::vector<uint8_t> valid_bytes = {true, true, false, true, false};
199+
this->test(precision, draw, valid_bytes);
200+
}
201+
202+
TEST_P(Decimal128BuilderTest, WithNulls) {
203+
int precision = GetParam();
204+
std::vector<Decimal128> draw = {Decimal128(1), Decimal128(2), Decimal128(-1),
205+
Decimal128(4), Decimal128(-1), Decimal128(1), Decimal128(2),
206+
Decimal128("230342903942.234234"), Decimal128("-23049302932.235234")};
207+
std::vector<uint8_t> valid_bytes = {
208+
true, true, false, true, false, true, true, true, true};
209+
std::vector<uint8_t> sign_bitmap = {
210+
false, false, false, false, false, false, false, false, true};
211+
this->test(precision, draw, valid_bytes, sign_bitmap);
212+
}
213+
214+
INSTANTIATE_TEST_CASE_P(Decimal32BuilderTest, Decimal32BuilderTest,
215+
::testing::Range(
216+
DecimalPrecision<int32_t>::minimum, DecimalPrecision<int32_t>::maximum));
217+
INSTANTIATE_TEST_CASE_P(Decimal64BuilderTest, Decimal64BuilderTest,
218+
::testing::Range(
219+
DecimalPrecision<int64_t>::minimum, DecimalPrecision<int64_t>::maximum));
220+
INSTANTIATE_TEST_CASE_P(Decimal128BuilderTest, Decimal128BuilderTest,
221+
::testing::Range(
222+
DecimalPrecision<int128_t>::minimum, DecimalPrecision<int128_t>::maximum));
223+
34224
} // namespace arrow

0 commit comments

Comments
 (0)