Skip to content

Commit 25c7634

Browse files
committed
Added tests for GetItem;
Fixed size checks in ItemView Fixed GetItem type/value for ColumnDate, ColumnDecimal, ColumnIPv4, ColumnIPv6, ColumnUUID
1 parent fa9a93a commit 25c7634

File tree

8 files changed

+62
-14
lines changed

8 files changed

+62
-14
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: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,13 +35,13 @@ void ItemView::ValidateData(Type::Code type, DataType data) {
3535
case Type::Code::UInt64:
3636
case Type::Code::Float64:
3737
case Type::Code::DateTime64:
38-
case Type::Code::IPv6:
3938
case Type::Code::Decimal64:
4039
expected_size = 8;
4140
break;
4241

4342
case Type::Code::String:
4443
case Type::Code::FixedString:
44+
case Type::Code::Decimal:
4545
// value can be of any size
4646
return;
4747

@@ -51,9 +51,9 @@ void ItemView::ValidateData(Type::Code type, DataType data) {
5151
case Type::Code::LowCardinality:
5252
throw UnimplementedError("Unsupported type in ItemView: " + std::to_string(static_cast<int>(type)));
5353

54+
case Type::Code::IPv6:
5455
case Type::Code::UUID:
5556
case Type::Code::Int128:
56-
case Type::Code::Decimal:
5757
case Type::Code::Decimal128:
5858
expected_size = 16;
5959
break;

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
}

ut/Column_ut.cpp

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,41 @@ 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(ValueType&& t) {
156+
using T = std::remove_cv_t<std::decay_t<ValueType>>;
157+
158+
if constexpr (std::is_same_v<T, clickhouse::UInt128>
159+
|| std::is_same_v<T, clickhouse::Int128>) {
160+
return std::string_view{reinterpret_cast<const char*>(&t), sizeof(T)};
161+
} else if constexpr (std::is_same_v<T, in_addr>) {
162+
return htonl(t.s_addr);
163+
} else if constexpr (std::is_same_v<T, in6_addr>) {
164+
return std::string_view(reinterpret_cast<const char*>(t.s6_addr), 16);
165+
} else if constexpr (std::is_same_v<ColumnType, ColumnDate>) {
166+
return static_cast<uint16_t>(t / std::time_t(86400));
167+
} else if constexpr (std::is_same_v<ColumnType, ColumnDateTime>) {
168+
return static_cast<uint32_t>(t);
169+
} else {
170+
return t;
171+
}
172+
}
173+
174+
TYPED_TEST(GenericColumnTest, GetItem) {
175+
auto [column, values] = this->MakeColumnWithValues(100);
176+
177+
ASSERT_EQ(values.size(), column->Size());
178+
ASSERT_EQ(column->GetItem(0).type, column->GetType().GetCode());
179+
180+
for (size_t i = 0; i < values.size(); ++i) {
181+
const auto v = convertValueForGetItem<typename TestFixture::ColumnType>(values[i]);
182+
const ItemView item = column->GetItem(i);
183+
184+
ASSERT_TRUE(CompareRecursive(item.get<decltype(v)>(), v));
185+
}
186+
}
187+
153188
TYPED_TEST(GenericColumnTest, Slice) {
154189
auto [column, values] = this->MakeColumnWithValues(100);
155190

0 commit comments

Comments
 (0)