Skip to content

Commit 4180b58

Browse files
EnmkEnmk
authored
Merge pull request #183 from Enmk/test_refactoring
Tests for Column::GetItem
2 parents b4f016b + 8615012 commit 4180b58

File tree

11 files changed

+266
-164
lines changed

11 files changed

+266
-164
lines changed

‎clickhouse/columns/date.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ void ColumnDate::Swap(Column& other) {
5858
}
5959

6060
ItemView ColumnDate::GetItem(size_t index) const {
61-
return data_->GetItem(index);
61+
return ItemView(Type::Date, data_->GetItem(index));
6262
}
6363

6464

@@ -128,7 +128,7 @@ void ColumnDateTime::Swap(Column& other) {
128128
}
129129

130130
ItemView ColumnDateTime::GetItem(size_t index) const {
131-
return data_->GetItem(index);
131+
return ItemView(Type::DateTime, data_->GetItem(index));
132132
}
133133

134134
ColumnDateTime64::ColumnDateTime64(size_t precision)
@@ -186,7 +186,7 @@ size_t ColumnDateTime64::Size() const {
186186
}
187187

188188
ItemView ColumnDateTime64::GetItem(size_t index) const {
189-
return data_->GetItem(index);
189+
return ItemView(Type::DateTime64, data_->GetItem(index));
190190
}
191191

192192
void ColumnDateTime64::Swap(Column& other) {

‎clickhouse/columns/decimal.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,7 @@ void ColumnDecimal::Swap(Column& other) {
229229
}
230230

231231
ItemView ColumnDecimal::GetItem(size_t index) const {
232-
return data_->GetItem(index);
232+
return ItemView{GetType().GetCode(), data_->GetItem(index)};
233233
}
234234

235235
size_t ColumnDecimal::GetScale() const

‎clickhouse/columns/ip4.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ void ColumnIPv4::Swap(Column& other) {
9797
}
9898

9999
ItemView ColumnIPv4::GetItem(size_t index) const {
100-
return data_->GetItem(index);
100+
return ItemView(Type::IPv4, data_->GetItem(index));
101101
}
102102

103103
}

‎clickhouse/columns/ip6.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ void ColumnIPv6::Swap(Column& other) {
9797
}
9898

9999
ItemView ColumnIPv6::GetItem(size_t index) const {
100-
return data_->GetItem(index);
100+
return ItemView{Type::IPv6, data_->GetItem(index)};
101101
}
102102

103103
}

‎clickhouse/columns/itemview.cpp

Lines changed: 46 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,44 +1,71 @@
11
#include "../columns/itemview.h"
22

3+
#include <algorithm>
4+
#include <sstream>
5+
6+
namespace {
7+
8+
template <typename Container>
9+
std::string ContainerToString(Container container, const char * separator = ", ") {
10+
std::stringstream sstr;
11+
const auto end = std::end(container);
12+
for (auto i = std::begin(container); i != end; /*intentionally no ++i*/) {
13+
const auto & elem = *i;
14+
sstr << elem;
15+
16+
if (++i != end) {
17+
sstr << separator;
18+
}
19+
}
20+
21+
return sstr.str();
22+
}
23+
24+
}
25+
326
namespace clickhouse {
427

528
void ItemView::ValidateData(Type::Code type, DataType data) {
6-
int expected_size = 0;
29+
30+
auto AssertSize = [type, &data](std::initializer_list<int> allowed_sizes) -> void {
31+
const auto end = std::end(allowed_sizes);
32+
if (std::find(std::begin(allowed_sizes), end, static_cast<int>(data.size())) == end) {
33+
throw AssertionError(std::string("ItemView value size mismatch for ")
34+
+ Type::TypeName(type)
35+
+ " expected: " + ContainerToString(allowed_sizes, " or ")
36+
+ ", got: " + std::to_string(data.size()));
37+
}
38+
};
39+
740
switch (type) {
841
case Type::Code::Void:
9-
expected_size = 0;
10-
break;
42+
return AssertSize({0});
1143

1244
case Type::Code::Int8:
1345
case Type::Code::UInt8:
1446
case Type::Code::Enum8:
15-
expected_size = 1;
16-
break;
47+
return AssertSize({1});
1748

1849
case Type::Code::Int16:
1950
case Type::Code::UInt16:
2051
case Type::Code::Date:
2152
case Type::Code::Enum16:
22-
expected_size = 2;
23-
break;
53+
return AssertSize({2});
2454

2555
case Type::Code::Int32:
2656
case Type::Code::UInt32:
2757
case Type::Code::Float32:
2858
case Type::Code::DateTime:
2959
case Type::Code::IPv4:
3060
case Type::Code::Decimal32:
31-
expected_size = 4;
32-
break;
61+
return AssertSize({4});
3362

3463
case Type::Code::Int64:
3564
case Type::Code::UInt64:
3665
case Type::Code::Float64:
3766
case Type::Code::DateTime64:
38-
case Type::Code::IPv6:
3967
case Type::Code::Decimal64:
40-
expected_size = 8;
41-
break;
68+
return AssertSize({8});
4269

4370
case Type::Code::String:
4471
case Type::Code::FixedString:
@@ -49,24 +76,21 @@ void ItemView::ValidateData(Type::Code type, DataType data) {
4976
case Type::Code::Nullable:
5077
case Type::Code::Tuple:
5178
case Type::Code::LowCardinality:
52-
throw UnimplementedError("Unsupported type in ItemView: " + std::to_string(static_cast<int>(type)));
79+
throw AssertionError("Unsupported type in ItemView: " + std::string(Type::TypeName(type)));
5380

81+
case Type::Code::IPv6:
5482
case Type::Code::UUID:
5583
case Type::Code::Int128:
56-
case Type::Code::Decimal:
5784
case Type::Code::Decimal128:
58-
expected_size = 16;
59-
break;
85+
return AssertSize({16});
86+
87+
case Type::Code::Decimal:
88+
// Could be either Decimal32, Decimal64 or Decimal128
89+
return AssertSize({4, 8, 16});
6090

6191
default:
6292
throw UnimplementedError("Unknon type code:" + std::to_string(static_cast<int>(type)));
6393
}
64-
65-
if (expected_size != static_cast<int>(data.size())) {
66-
throw AssertionError("Value size mismatch for type "
67-
+ std::to_string(static_cast<int>(type)) + " expected: "
68-
+ std::to_string(expected_size) + ", got: " + std::to_string(data.size()));
69-
}
7094
}
7195

7296
}

‎clickhouse/columns/itemview.h

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
#include <string_view>
77
#include <stdexcept>
8+
#include <type_traits>
89

910
namespace clickhouse {
1011

@@ -43,8 +44,15 @@ struct ItemView {
4344
ValidateData(type, data);
4445
}
4546

47+
ItemView(Type::Code type, ItemView other)
48+
: type(type),
49+
data(other.data)
50+
{
51+
ValidateData(type, data);
52+
}
53+
4654
explicit ItemView()
47-
: ItemView(Type::Void, {})
55+
: ItemView(Type::Void, std::string_view{})
4856
{}
4957

5058
template <typename T>
@@ -53,11 +61,12 @@ struct ItemView {
5361
{}
5462

5563
template <typename T>
56-
T get() const {
57-
if constexpr (std::is_same_v<std::string_view, T> || std::is_same_v<std::string, T>) {
64+
auto get() const {
65+
using ValueType = std::remove_cv_t<std::decay_t<T>>;
66+
if constexpr (std::is_same_v<std::string_view, ValueType> || std::is_same_v<std::string, ValueType>) {
5867
return data;
59-
} else if constexpr (std::is_fundamental_v<T> || std::is_same_v<Int128, T>) {
60-
if (sizeof(T) == data.size()) {
68+
} else if constexpr (std::is_fundamental_v<ValueType> || std::is_same_v<Int128, ValueType>) {
69+
if (sizeof(ValueType) == data.size()) {
6170
return *reinterpret_cast<const T*>(data.data());
6271
} else {
6372
throw AssertionError("Incompatitable value type and size.");

‎clickhouse/columns/uuid.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,11 @@ void ColumnUUID::Swap(Column& other) {
7070
}
7171

7272
ItemView ColumnUUID::GetItem(size_t index) const {
73-
return data_->GetItem(index);
73+
// We know that ColumnUInt64 stores it's data in continius memory region,
74+
// and that every 2 values from data represent 1 UUID value.
75+
const auto data_item_view = data_->GetItem(index * 2);
76+
77+
return ItemView{Type::UUID, std::string_view{data_item_view.data.data(), data_item_view.data.size() * 2}};
7478
}
7579

7680
}

‎clickhouse/types/types.cpp

Lines changed: 41 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -13,50 +13,69 @@ Type::Type(const Code code)
1313
, type_unique_id_(0)
1414
{}
1515

16+
const char* Type::TypeName(Type::Code code) {
17+
switch (code) {
18+
case Type::Code::Void: return "Void";
19+
case Type::Code::Int8: return "Int8";
20+
case Type::Code::Int16: return "Int16";
21+
case Type::Code::Int32: return "Int32";
22+
case Type::Code::Int64: return "Int64";
23+
case Type::Code::UInt8: return "UInt8";
24+
case Type::Code::UInt16: return "UInt16";
25+
case Type::Code::UInt32: return "UInt32";
26+
case Type::Code::UInt64: return "UInt64";
27+
case Type::Code::Float32: return "Float32";
28+
case Type::Code::Float64: return "Float64";
29+
case Type::Code::String: return "String";
30+
case Type::Code::FixedString: return "FixedString";
31+
case Type::Code::DateTime: return "DateTime";
32+
case Type::Code::Date: return "Date";
33+
case Type::Code::Array: return "Array";
34+
case Type::Code::Nullable: return "Nullable";
35+
case Type::Code::Tuple: return "Tuple";
36+
case Type::Code::Enum8: return "Enum8";
37+
case Type::Code::Enum16: return "Enum16";
38+
case Type::Code::UUID: return "UUID";
39+
case Type::Code::IPv4: return "IPv4";
40+
case Type::Code::IPv6: return "IPv6";
41+
case Type::Code::Int128: return "Int128";
42+
case Type::Code::Decimal: return "Decimal";
43+
case Type::Code::Decimal32: return "Decimal32";
44+
case Type::Code::Decimal64: return "Decimal64";
45+
case Type::Code::Decimal128: return "Decimal128";
46+
case Type::Code::LowCardinality: return "LowCardinality";
47+
case Type::Code::DateTime64: return "DateTime64";
48+
}
49+
50+
return "Unknown type";
51+
}
52+
1653
std::string Type::GetName() const {
1754
switch (code_) {
1855
case Void:
19-
return "Void";
2056
case Int8:
21-
return "Int8";
2257
case Int16:
23-
return "Int16";
2458
case Int32:
25-
return "Int32";
2659
case Int64:
27-
return "Int64";
2860
case Int128:
29-
return "Int128";
3061
case UInt8:
31-
return "UInt8";
3262
case UInt16:
33-
return "UInt16";
3463
case UInt32:
35-
return "UInt32";
3664
case UInt64:
37-
return "UInt64";
3865
case UUID:
39-
return "UUID";
4066
case Float32:
41-
return "Float32";
4267
case Float64:
43-
return "Float64";
4468
case String:
45-
return "String";
46-
case FixedString:
47-
return As<FixedStringType>()->GetName();
4869
case IPv4:
49-
return "IPv4";
5070
case IPv6:
51-
return "IPv6";
71+
case Date:
72+
return TypeName(code_);
73+
case FixedString:
74+
return As<FixedStringType>()->GetName();
5275
case DateTime:
53-
{
5476
return As<DateTimeType>()->GetName();
55-
}
5677
case DateTime64:
5778
return As<DateTime64Type>()->GetName();
58-
case Date:
59-
return "Date";
6079
case Array:
6180
return As<ArrayType>()->GetName();
6281
case Nullable:

‎clickhouse/types/types.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,9 @@ class Type {
8383

8484
bool IsEqual(const TypeRef& other) const { return IsEqual(*other); }
8585

86+
/// Simple name, doesn't depend on parameters and\or nested types, caller MUST NOT free returned value.
87+
static const char* TypeName(Code);
88+
8689
public:
8790
static TypeRef CreateArray(TypeRef item_type);
8891

‎ut/Column_ut.cpp

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,45 @@ TYPED_TEST(GenericColumnTest, Append) {
150150
EXPECT_TRUE(CompareRecursive(values, *column));
151151
}
152152

153+
// To make some value types compatitable with Column::GetItem()
154+
template <typename ColumnType, typename ValueType>
155+
inline auto convertValueForGetItem(const ColumnType& col, ValueType&& t) {
156+
using T = std::remove_cv_t<std::decay_t<ValueType>>;
157+
158+
if constexpr (std::is_same_v<ColumnType, ColumnDecimal>) {
159+
// Since ColumnDecimal can hold 32, 64, 128-bit wide data and there is no way telling at run-time.
160+
const ItemView item = col.GetItem(0);
161+
return std::string_view(reinterpret_cast<const char*>(&t), item.data.size());
162+
} else if constexpr (std::is_same_v<T, clickhouse::UInt128>
163+
|| std::is_same_v<T, clickhouse::Int128>) {
164+
return std::string_view{reinterpret_cast<const char*>(&t), sizeof(T)};
165+
} else if constexpr (std::is_same_v<T, in_addr>) {
166+
return htonl(t.s_addr);
167+
} else if constexpr (std::is_same_v<T, in6_addr>) {
168+
return std::string_view(reinterpret_cast<const char*>(t.s6_addr), 16);
169+
} else if constexpr (std::is_same_v<ColumnType, ColumnDate>) {
170+
return static_cast<uint16_t>(t / std::time_t(86400));
171+
} else if constexpr (std::is_same_v<ColumnType, ColumnDateTime>) {
172+
return static_cast<uint32_t>(t);
173+
} else {
174+
return t;
175+
}
176+
}
177+
178+
TYPED_TEST(GenericColumnTest, GetItem) {
179+
auto [column, values] = this->MakeColumnWithValues(100);
180+
181+
ASSERT_EQ(values.size(), column->Size());
182+
ASSERT_EQ(column->GetItem(0).type, column->GetType().GetCode());
183+
184+
for (size_t i = 0; i < values.size(); ++i) {
185+
const auto v = convertValueForGetItem(*column, values[i]);
186+
const ItemView item = column->GetItem(i);
187+
188+
ASSERT_TRUE(CompareRecursive(item.get<decltype(v)>(), v));
189+
}
190+
}
191+
153192
TYPED_TEST(GenericColumnTest, Slice) {
154193
auto [column, values] = this->MakeColumnWithValues(100);
155194

0 commit comments

Comments
 (0)