From 88b96a1c6d5b4f8f5cec5771e857aa620fc13415 Mon Sep 17 00:00:00 2001 From: Vasily Nemkov Date: Wed, 2 Feb 2022 17:03:50 +0200 Subject: [PATCH 1/2] Fixed ColumnIPv4i::Append() Added tests, simplified implementation + few extra minor touches --- clickhouse/columns/ip4.cpp | 22 ++++----- clickhouse/columns/ip4.h | 6 ++- clickhouse/columns/ip6.cpp | 9 +++- clickhouse/columns/ip6.h | 5 +- clickhouse/columns/nothing.h | 1 + tests/simple/main.cpp | 1 + ut/columns_ut.cpp | 94 ++++++++++++++++++++++++++++++++++-- ut/performance_tests.cpp | 2 + 8 files changed, 121 insertions(+), 19 deletions(-) diff --git a/clickhouse/columns/ip4.cpp b/clickhouse/columns/ip4.cpp index 70a81d75..106aa414 100644 --- a/clickhouse/columns/ip4.cpp +++ b/clickhouse/columns/ip4.cpp @@ -1,12 +1,8 @@ - #include "ip4.h" +#include "../base/socket.h" // for platform-specific IPv4-related functions #include -#if defined(_win_) -using in_addr_t = unsigned long; -#endif - namespace clickhouse { ColumnIPv4::ColumnIPv4() @@ -25,29 +21,33 @@ ColumnIPv4::ColumnIPv4(ColumnRef data) } void ColumnIPv4::Append(const std::string& str) { - in_addr_t addr = inet_addr(str.c_str()); - if (addr == INADDR_NONE) { + uint32_t address; + if (inet_pton(AF_INET, str.c_str(), &address) != 1) throw std::runtime_error("invalid IPv4 format, ip: " + str); - } - data_->Append(htonl(addr)); + data_->Append(htonl(address)); } void ColumnIPv4::Append(uint32_t ip) { data_->Append(htonl(ip)); } +void ColumnIPv4::Append(in_addr ip) +{ + data_->Append(htonl(ip.s_addr)); +} + void ColumnIPv4::Clear() { data_->Clear(); } in_addr ColumnIPv4::At(size_t n) const { - struct in_addr addr; + in_addr addr; addr.s_addr = ntohl(data_->At(n)); return addr; } in_addr ColumnIPv4::operator [] (size_t n) const { - struct in_addr addr; + in_addr addr; addr.s_addr = ntohl(data_->operator[](n)); return addr; } diff --git a/clickhouse/columns/ip4.h b/clickhouse/columns/ip4.h index ec6e51ba..0e66bf3a 100644 --- a/clickhouse/columns/ip4.h +++ b/clickhouse/columns/ip4.h @@ -1,7 +1,8 @@ #pragma once #include "numeric.h" -#include "../base/socket.h" + +struct in_addr; namespace clickhouse { @@ -16,6 +17,9 @@ class ColumnIPv4 : public Column { /// @params ip numeric value with host byte order. void Append(uint32_t ip); + /// + void Append(in_addr ip); + /// Returns element at given row number. in_addr At(size_t n) const; diff --git a/clickhouse/columns/ip6.cpp b/clickhouse/columns/ip6.cpp index 6b532d25..11f2030f 100644 --- a/clickhouse/columns/ip6.cpp +++ b/clickhouse/columns/ip6.cpp @@ -1,5 +1,6 @@ #include "ip6.h" +#include "../base/socket.h" // for IPv6 platform-specific stuff #include @@ -27,11 +28,15 @@ void ColumnIPv6::Append(const std::string& ip) { if (inet_pton(AF_INET6, ip.c_str(), buf) != 1) { throw std::runtime_error("invalid IPv6 format, ip: " + ip); } - data_->Append(std::string((const char*)buf, 16)); + data_->Append(std::string_view((const char*)buf, 16)); } void ColumnIPv6::Append(const in6_addr* addr) { - data_->Append(std::string((const char*)addr->s6_addr, 16)); + data_->Append(std::string_view((const char*)addr->s6_addr, 16)); +} + +void ColumnIPv6::Append(const in6_addr& addr) { + Append(&addr); } void ColumnIPv6::Clear() { diff --git a/clickhouse/columns/ip6.h b/clickhouse/columns/ip6.h index e620ef90..84231db4 100644 --- a/clickhouse/columns/ip6.h +++ b/clickhouse/columns/ip6.h @@ -1,7 +1,9 @@ #pragma once #include "string.h" -#include "../base/socket.h" +#include + +struct in6_addr; namespace clickhouse { @@ -14,6 +16,7 @@ class ColumnIPv6 : public Column{ void Append(const std::string& str); void Append(const in6_addr* addr); + void Append(const in6_addr& addr); /// Returns element at given row number. in6_addr At(size_t n) const; diff --git a/clickhouse/columns/nothing.h b/clickhouse/columns/nothing.h index 591c1ecd..ef7004b5 100644 --- a/clickhouse/columns/nothing.h +++ b/clickhouse/columns/nothing.h @@ -2,6 +2,7 @@ #pragma once #include "column.h" +#include "../base/input.h" #include #include diff --git a/tests/simple/main.cpp b/tests/simple/main.cpp index 10fb353d..10e30bb7 100644 --- a/tests/simple/main.cpp +++ b/tests/simple/main.cpp @@ -1,6 +1,7 @@ #include #include #include +#include #include #include diff --git a/ut/columns_ut.cpp b/ut/columns_ut.cpp index 3a14c16f..e177cd2d 100644 --- a/ut/columns_ut.cpp +++ b/ut/columns_ut.cpp @@ -8,6 +8,8 @@ #include #include #include +#include +#include #include #include @@ -16,6 +18,28 @@ #include +#include // for ipv4-ipv6 platform-specific stuff + +bool operator==(const in_addr & left, const in_addr & right) +{ + return memcmp(&left, &right, sizeof(left)) == 0; +} + +bool operator==(const in6_addr & left, const in6_addr & right) +{ + return memcmp(&left, &right, sizeof(left)) == 0; +} + +std::ostream& operator<<(std::ostream& ostr, const in6_addr & addr) +{ + char buf[INET6_ADDRSTRLEN]; + const char* ip_str = inet_ntop(AF_INET6, &addr, buf, INET6_ADDRSTRLEN); + + if (!ip_str) + return ostr << ""; + + return ostr << ip_str; +} namespace { @@ -216,8 +240,8 @@ TEST(ColumnsCase, ArrayAppend) { auto col = arr1->GetAsColumn(1); ASSERT_EQ(arr1->Size(), 2u); - //ASSERT_EQ(col->As()->At(0), 1u); - //ASSERT_EQ(col->As()->At(1), 3u); + ASSERT_EQ(col->As()->At(0), 1u); + ASSERT_EQ(col->As()->At(1), 3u); } TEST(ColumnsCase, TupleAppend){ @@ -253,6 +277,7 @@ TEST(ColumnsCase, TupleSlice){ ASSERT_EQ((*tuple2)[1]->As()->At(0), "3"); } + TEST(ColumnsCase, DateAppend) { auto col1 = std::make_shared(); auto col2 = std::make_shared(); @@ -505,6 +530,68 @@ TEST(ColumnsCase, Int128) { EXPECT_EQ(0, col->At(4)); } +TEST(ColumnsCase, ColumnIPv4) +{ + // TODO: split into proper method-level unit-tests + auto col = ColumnIPv4(); + + col.Append("255.255.255.255"); + col.Append("127.0.0.1"); + col.Append(3585395774); + col.Append(0); + const in_addr ip{0x12345678}; + col.Append(ip); + + ASSERT_EQ(5u, col.Size()); + EXPECT_EQ(in_addr{0xffffffff}, col.At(0)); + EXPECT_EQ(in_addr{0x0100007f}, col.At(1)); + EXPECT_EQ(in_addr{3585395774}, col.At(2)); + EXPECT_EQ(in_addr{0}, col.At(3)); + EXPECT_EQ(ip, col.At(4)); + + EXPECT_EQ("255.255.255.255", col.AsString(0)); + EXPECT_EQ("127.0.0.1", col.AsString(1)); + EXPECT_EQ("62.204.180.213", col.AsString(2)); + EXPECT_EQ("0.0.0.0", col.AsString(3)); + EXPECT_EQ("120.86.52.18", col.AsString(4)); + + col.Clear(); + EXPECT_EQ(0u, col.Size()); +} + +TEST(ColumnsCase, ColumnIPv6) +{ + // TODO: split into proper method-level unit-tests + auto col = ColumnIPv6(); + col.Append("0:0:0:0:0:0:0:1"); + col.Append("::"); + col.Append("::FFFF:204.152.189.116"); + + const auto ipv6 = in6_addr{{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15}}}; + col.Append(ipv6); + + ASSERT_EQ(4u, col.Size()); + const auto first_val = in6_addr{{{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1}}}; + EXPECT_EQ(first_val, col.At(0)); + + const auto second_val = in6_addr{{{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}}}; + EXPECT_EQ(second_val, col.At(1)); + + const auto third_val = in6_addr{{{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0xff, 0xff, 204, 152, 189, 116}}}; + EXPECT_EQ(third_val, col.At(2)); + + EXPECT_EQ(ipv6, col.At(3)); + + + EXPECT_EQ("::1", col.AsString(0)); + EXPECT_EQ("::", col.AsString(1)); + EXPECT_EQ("::ffff:204.152.189.116", col.AsString(2)); + EXPECT_EQ("1:203:405:607:809:a0b:c0d:e0f", col.AsString(3)); + + col.Clear(); + EXPECT_EQ(0u, col.Size()); +} + TEST(ColumnsCase, ColumnDecimal128_from_string) { auto col = std::make_shared(38, 0); @@ -516,8 +603,7 @@ TEST(ColumnsCase, ColumnDecimal128_from_string) { std::numeric_limits::max(), }; - for (size_t i = 0; i < values.size(); ++i) - { + for (size_t i = 0; i < values.size(); ++i) { const auto value = values.begin()[i]; SCOPED_TRACE(::testing::Message() << "# index: " << i << " Int128 value: " << value); diff --git a/ut/performance_tests.cpp b/ut/performance_tests.cpp index f6883f35..d772644f 100644 --- a/ut/performance_tests.cpp +++ b/ut/performance_tests.cpp @@ -7,6 +7,8 @@ #include #include #include +#include +#include #include From 60c1f66cc27ef2d8d4dcdcc6b0b7b002f0f92e18 Mon Sep 17 00:00:00 2001 From: Vasily Nemkov Date: Wed, 2 Feb 2022 17:41:22 +0200 Subject: [PATCH 2/2] Fixed tests under Windows --- ut/columns_ut.cpp | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/ut/columns_ut.cpp b/ut/columns_ut.cpp index e177cd2d..7aa12083 100644 --- a/ut/columns_ut.cpp +++ b/ut/columns_ut.cpp @@ -30,6 +30,15 @@ bool operator==(const in6_addr & left, const in6_addr & right) return memcmp(&left, &right, sizeof(left)) == 0; } +in_addr make_IPv4(uint32_t ip) +{ + static_assert(sizeof(in_addr) == sizeof(ip)); + in_addr result; + memcpy(&result, &ip, sizeof(ip)); + + return result; +} + std::ostream& operator<<(std::ostream& ostr, const in6_addr & addr) { char buf[INET6_ADDRSTRLEN]; @@ -539,14 +548,14 @@ TEST(ColumnsCase, ColumnIPv4) col.Append("127.0.0.1"); col.Append(3585395774); col.Append(0); - const in_addr ip{0x12345678}; + const in_addr ip = make_IPv4(0x12345678); col.Append(ip); ASSERT_EQ(5u, col.Size()); - EXPECT_EQ(in_addr{0xffffffff}, col.At(0)); - EXPECT_EQ(in_addr{0x0100007f}, col.At(1)); - EXPECT_EQ(in_addr{3585395774}, col.At(2)); - EXPECT_EQ(in_addr{0}, col.At(3)); + EXPECT_EQ(make_IPv4(0xffffffff), col.At(0)); + EXPECT_EQ(make_IPv4(0x0100007f), col.At(1)); + EXPECT_EQ(make_IPv4(3585395774), col.At(2)); + EXPECT_EQ(make_IPv4(0), col.At(3)); EXPECT_EQ(ip, col.At(4)); EXPECT_EQ("255.255.255.255", col.AsString(0));