Skip to content

Commit 715306a

Browse files
abyss7abyss7
authored
Implement support for misplaced dot in appended decimal as string (ClickHouse#19)
Fixes ClickHouse#12 * Throw exceptions on overflow and unexpected symbols. * Implement support for misplaced dot in appended decimal
1 parent a510666 commit 715306a

File tree

3 files changed

+81
-14
lines changed

3 files changed

+81
-14
lines changed

‎clickhouse/CMakeLists.txt‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,7 @@ SET ( clickhouse-cpp-lib-src
2828
)
2929

3030
ADD_LIBRARY (clickhouse-cpp-lib SHARED ${clickhouse-cpp-lib-src})
31-
32-
SET_TARGET_PROPERTIES(clickhouse-cpp-lib
33-
PROPERTIES LINKER_LANGUAGE CXX)
34-
31+
SET_TARGET_PROPERTIES(clickhouse-cpp-lib PROPERTIES LINKER_LANGUAGE CXX)
3532
TARGET_LINK_LIBRARIES (clickhouse-cpp-lib
3633
cityhash-lib
3734
lz4-lib
@@ -43,6 +40,12 @@ TARGET_LINK_LIBRARIES (clickhouse-cpp-lib-static
4340
lz4-lib
4441
)
4542

43+
IF (CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
44+
set (CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} --rtlib=compiler-rt")
45+
TARGET_LINK_LIBRARIES (clickhouse-cpp-lib gcc_s)
46+
TARGET_LINK_LIBRARIES (clickhouse-cpp-lib-static gcc_s)
47+
ENDIF ()
48+
4649
INSTALL(TARGETS clickhouse-cpp-lib clickhouse-cpp-lib-static
4750
ARCHIVE DESTINATION lib
4851
LIBRARY DESTINATION lib

‎clickhouse/columns/decimal.cpp‎

Lines changed: 32 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
#include "decimal.h"
22

3+
#include <iostream>
4+
35
namespace clickhouse {
46

57
ColumnDecimal::ColumnDecimal(size_t precision, size_t scale)
@@ -32,26 +34,49 @@ void ColumnDecimal::Append(const Int128& value) {
3234
void ColumnDecimal::Append(const std::string& value) {
3335
Int128 int_value = 0;
3436
auto c = value.begin();
37+
auto end = value.end();
3538
bool sign = true;
39+
bool has_dot = false;
40+
41+
int zeros = 0;
3642

37-
while (c != value.end()) {
43+
while (c != end) {
3844
if (*c == '-') {
3945
sign = false;
4046
if (c != value.begin()) {
4147
break;
4248
}
43-
} else if (*c == '.') {
44-
// TODO: compare distance with `scale`
49+
} else if (*c == '.' && !has_dot) {
50+
size_t distance = std::distance(c, end) - 1;
51+
auto scale = type_->As<DecimalType>()->GetScale();
52+
53+
if (distance <= scale) {
54+
zeros = scale - distance;
55+
} else {
56+
std::advance(end, scale - distance);
57+
}
58+
59+
has_dot = true;
4560
} else if (*c >= '0' && *c <= '9') {
46-
int_value = int_value * 10 + (*c - '0');
61+
if (__builtin_mul_overflow(int_value, 10, &int_value) ||
62+
__builtin_add_overflow(int_value, *c - '0', &int_value)) {
63+
throw std::runtime_error("value is too big for 128-bit integer");
64+
}
4765
} else {
48-
// TODO: throw exception on unexpected symbol
66+
throw std::runtime_error(std::string("unexpected symbol '") + (*c) + "' in decimal value");
4967
}
5068
++c;
5169
}
5270

53-
if (c != value.end()) {
54-
// TODO: throw exception about symbols after 'minus'
71+
if (c != end) {
72+
throw std::runtime_error("unexpected symbol '-' in decimal value");
73+
}
74+
75+
while (zeros) {
76+
if (__builtin_mul_overflow(int_value, 10, &int_value)) {
77+
throw std::runtime_error("value is too big for 128-bit integer");
78+
}
79+
--zeros;
5580
}
5681

5782
Append(sign ? int_value : -int_value);

‎ut/client_ut.cpp‎

Lines changed: 42 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -376,6 +376,31 @@ TEST_P(ClientCase, Decimal) {
376376
auto d5 = std::make_shared<ColumnDecimal>(18, 9);
377377
auto d6 = std::make_shared<ColumnDecimal>(38, 19);
378378

379+
EXPECT_THROW(
380+
d1->Append("1234567890123456789012345678901234567890"),
381+
std::runtime_error
382+
);
383+
EXPECT_THROW(
384+
d1->Append("123456789012345678901234567890123456.7890"),
385+
std::runtime_error
386+
);
387+
EXPECT_THROW(
388+
d1->Append("-1234567890123456789012345678901234567890"),
389+
std::runtime_error
390+
);
391+
EXPECT_THROW(
392+
d1->Append("12345678901234567890123456789012345678a"),
393+
std::runtime_error
394+
);
395+
EXPECT_THROW(
396+
d1->Append("12345678901234567890123456789012345678-"),
397+
std::runtime_error
398+
);
399+
EXPECT_THROW(
400+
d1->Append("1234.12.1234"),
401+
std::runtime_error
402+
);
403+
379404
id->Append(1);
380405
d1->Append(123456789);
381406
d2->Append(123456789012345678);
@@ -401,7 +426,6 @@ TEST_P(ClientCase, Decimal) {
401426
d6->Append(-999999999999999999);
402427

403428
// Check strings with decimal point
404-
// TODO: check that exception is thrown if point doesn't match the `scale`
405429
id->Append(4);
406430
d1->Append("12345.6789");
407431
d2->Append("123456789.012345678");
@@ -419,6 +443,14 @@ TEST_P(ClientCase, Decimal) {
419443
d5->Append("-123456789012345678");
420444
d6->Append("-12345678901234567890123456789012345678");
421445

446+
id->Append(6);
447+
d1->Append("12345.678");
448+
d2->Append("123456789.0123456789");
449+
d3->Append("1234567890123456789.0123456789012345678");
450+
d4->Append("12345.6789");
451+
d5->Append("123456789.012345678");
452+
d6->Append("1234567890123456789.0123456789012345678");
453+
422454
b.AppendColumn("id", id);
423455
b.AppendColumn("d1", d1);
424456
b.AppendColumn("d2", d2);
@@ -435,7 +467,7 @@ TEST_P(ClientCase, Decimal) {
435467
return;
436468
}
437469

438-
ASSERT_EQ(5u, b.GetRowCount());
470+
ASSERT_EQ(6u, b.GetRowCount());
439471

440472
auto int128_to_string = [](Int128 value) {
441473
std::string result;
@@ -504,6 +536,14 @@ TEST_P(ClientCase, Decimal) {
504536
EXPECT_EQ("-123456789", int128_to_string(decimal(4, 4)));
505537
EXPECT_EQ("-123456789012345678", int128_to_string(decimal(5, 4)));
506538
EXPECT_EQ("-12345678901234567890123456789012345678", int128_to_string(decimal(6, 4)));
539+
540+
EXPECT_EQ(6u, b[0]->As<ColumnUInt64>()->At(5));
541+
EXPECT_EQ("123456780", int128_to_string(decimal(1, 5)));
542+
EXPECT_EQ("123456789012345678", int128_to_string(decimal(2, 5)));
543+
EXPECT_EQ("12345678901234567890123456789012345678", int128_to_string(decimal(3, 5)));
544+
EXPECT_EQ("123456789", int128_to_string(decimal(4, 5)));
545+
EXPECT_EQ("123456789012345678", int128_to_string(decimal(5, 5)));
546+
EXPECT_EQ("12345678901234567890123456789012345678", int128_to_string(decimal(6, 5)));
507547
});
508548
}
509549

@@ -518,4 +558,3 @@ INSTANTIATE_TEST_CASE_P(
518558
.SetPingBeforeQuery(false)
519559
.SetCompressionMethod(CompressionMethod::LZ4)
520560
));
521-

0 commit comments

Comments
 (0)