diff --git a/clickhouse/columns/ip4.cpp b/clickhouse/columns/ip4.cpp index 78ec5bbe..616bf46c 100644 --- a/clickhouse/columns/ip4.cpp +++ b/clickhouse/columns/ip4.cpp @@ -23,7 +23,7 @@ ColumnIPv4::ColumnIPv4(std::vector&& data) : Column(Type::CreateIPv4()) { for (auto& addr : data) { - addr = htonl(addr); + addr = ntohl(addr); } data_ = std::make_shared(std::move(data)); } @@ -32,15 +32,15 @@ void ColumnIPv4::Append(const std::string& str) { uint32_t address; if (inet_pton(AF_INET, str.c_str(), &address) != 1) throw ValidationError("invalid IPv4 format, ip: " + str); - data_->Append(htonl(address)); + data_->Append(ntohl(address)); } void ColumnIPv4::Append(uint32_t ip) { - data_->Append(htonl(ip)); + data_->Append(ntohl(ip)); } void ColumnIPv4::Append(in_addr ip) { - data_->Append(htonl(ip.s_addr)); + data_->Append(ntohl(ip.s_addr)); } void ColumnIPv4::Clear() { @@ -49,13 +49,13 @@ void ColumnIPv4::Clear() { in_addr ColumnIPv4::At(size_t n) const { in_addr addr; - addr.s_addr = ntohl(data_->At(n)); + addr.s_addr = htonl(data_->At(n)); return addr; } in_addr ColumnIPv4::operator [] (size_t n) const { in_addr addr; - addr.s_addr = ntohl(data_->operator[](n)); + addr.s_addr = htonl(data_->operator[](n)); return addr; } diff --git a/clickhouse/columns/ip4.h b/clickhouse/columns/ip4.h index 2253e305..c0536ee7 100644 --- a/clickhouse/columns/ip4.h +++ b/clickhouse/columns/ip4.h @@ -19,15 +19,18 @@ class ColumnIPv4 : public Column { */ explicit ColumnIPv4(ColumnRef data); + /// Create column from numeric values in the same representation as `in_addr::s_addr` + /// (network byte order), i.e. `htonl(0xC0A80107)` for `192.168.1.7`. explicit ColumnIPv4(std::vector&& data); /// Appends one element to the column. void Append(const std::string& ip); - /// @params ip numeric value with host byte order. + /// Append numeric value in the same representation as `in_addr::s_addr` + /// (network byte order), i.e. `htonl(0xC0A80107)` for `192.168.1.7`. void Append(uint32_t ip); - /// + /// Append struct in_addr void Append(in_addr ip); /// Returns element at given row number. diff --git a/clickhouse/columns/lowcardinality.cpp b/clickhouse/columns/lowcardinality.cpp index 5a5d0298..22b1995b 100644 --- a/clickhouse/columns/lowcardinality.cpp +++ b/clickhouse/columns/lowcardinality.cpp @@ -233,9 +233,8 @@ inline void AppendToDictionary(Column& dictionary, const ItemView & item) { column_down_cast(dictionary).AppendRaw(item.get()); return; case Type::IPv4: - // ColumnIPv4::Append applies htonl, and GetItem returns the stored - // (already byte-swapped) value, so undo the swap to re-store as-is. - column_down_cast(dictionary).Append(ntohl(item.get())); + // GetItem returns the stored host-order value; Append(uint32_t) expects network order. + column_down_cast(dictionary).Append(htonl(item.get())); return; case Type::IPv6: { in6_addr addr; diff --git a/tests/simple/main.cpp b/tests/simple/main.cpp index 5e95433b..055566ce 100644 --- a/tests/simple/main.cpp +++ b/tests/simple/main.cpp @@ -517,7 +517,7 @@ inline void IPExample(Client &client) { auto v4 = std::make_shared(); v4->Append("127.0.0.1"); - v4->Append(3585395774); + v4->Append(0xD5B4CC3E); v4->Append(0); auto v6 = std::make_shared(); diff --git a/ut/client_ut.cpp b/ut/client_ut.cpp index 88ef5779..79ee9ca8 100644 --- a/ut/client_ut.cpp +++ b/ut/client_ut.cpp @@ -259,6 +259,92 @@ TEST_P(ClientCase, Time64) { EXPECT_EQ(total_rows, 1UL); } +TEST_P(ClientCase, IPv4RoundTrip) { + // Check that an inserted IP address matches representation created by ClickHouse's toString() + client_->Execute("DROP TEMPORARY TABLE IF EXISTS test_clickhouse_cpp_ipv4"); + client_->Execute("CREATE TEMPORARY TABLE test_clickhouse_cpp_ipv4 (id String, ip IPv4) ENGINE = Memory"); + + std::string address_string = "192.168.1.7"; + auto id = std::make_shared(); + auto ip = std::make_shared(); + id->Append("Append(const std::string&)"); + ip->Append(address_string); + + struct sockaddr_in sa{}; + ASSERT_EQ(inet_pton(AF_INET, address_string.c_str(), &sa.sin_addr), 1); + id->Append("Append(in_addr)"); + ip->Append(sa.sin_addr); + + // Append(uint32_t) expects the raw value as found in in_addr::s_addr (network byte order). + uint32_t network_order = htonl(0xC0A80107); + id->Append("Append(uint32_t)"); + ip->Append(network_order); + + Block b; + b.AppendColumn("id", id); + b.AppendColumn("ip", ip); + client_->Insert("test_clickhouse_cpp_ipv4", b); + + size_t total_rows = 0; + client_->Select("SELECT id, toString(ip) FROM test_clickhouse_cpp_ipv4 ORDER BY id", [&](const Block& block) { + total_rows += block.GetRowCount(); + for (size_t i = 0; i < block.GetRowCount(); ++i) { + const auto id = block[0]->AsStrict()->At(i); + const auto out_string = block[1]->AsStrict()->At(i); + EXPECT_EQ(out_string, address_string) << id; + } + }); + EXPECT_EQ(total_rows, ip->Size()); +} + +TEST_P(ClientCase, IPv6RoundTrip) { + // Check that an inserted IP address matches representation created by ClickHouse's toString(). + // The address has 16 distinct, monotonically increasing bytes (01 02 ... 10), so any + // byte-order or offset mistake on the way to the server produces a different string. + client_->Execute("DROP TEMPORARY TABLE IF EXISTS test_clickhouse_cpp_ipv6"); + client_->Execute("CREATE TEMPORARY TABLE test_clickhouse_cpp_ipv6 (id String, ip IPv6) ENGINE = Memory"); + + std::string address_string = "102:304:506:708:90a:b0c:d0e:f10"; + auto id = std::make_shared(); + auto ip = std::make_shared(); + id->Append("Append(const std::string_view&)"); + ip->Append(address_string); + + struct sockaddr_in6 sa{}; + ASSERT_EQ(inet_pton(AF_INET6, address_string.c_str(), &sa.sin6_addr), 1); + id->Append("Append(const in6_addr&)"); + ip->Append(sa.sin6_addr); + + id->Append("Append(const in6_addr*)"); + ip->Append(&sa.sin6_addr); + + // Bytes spelled out explicitly, bypassing inet_pton: s6_addr[0] is the most + // significant byte on the wire (network order). + const uint8_t raw_bytes[16] = {0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, + 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, 0x10}; + in6_addr explicit_bytes; + static_assert(sizeof(explicit_bytes) == sizeof(raw_bytes)); + memcpy(&explicit_bytes, raw_bytes, sizeof(raw_bytes)); + id->Append("Append(const in6_addr&) from explicit bytes"); + ip->Append(explicit_bytes); + + Block b; + b.AppendColumn("id", id); + b.AppendColumn("ip", ip); + client_->Insert("test_clickhouse_cpp_ipv6", b); + + size_t total_rows = 0; + client_->Select("SELECT id, toString(ip) FROM test_clickhouse_cpp_ipv6 ORDER BY id", [&](const Block& block) { + total_rows += block.GetRowCount(); + for (size_t i = 0; i < block.GetRowCount(); ++i) { + const auto id = block[0]->AsStrict()->At(i); + const auto out_string = block[1]->AsStrict()->At(i); + EXPECT_EQ(out_string, address_string) << id; + } + }); + EXPECT_EQ(total_rows, ip->Size()); +} + TEST_P(ClientCase, Date) { Block b; diff --git a/ut/columns_ut.cpp b/ut/columns_ut.cpp index be8e543a..57650ea9 100644 --- a/ut/columns_ut.cpp +++ b/ut/columns_ut.cpp @@ -954,7 +954,7 @@ TEST(ColumnsCase, ColumnIPv4) col.Append("255.255.255.255"); col.Append("127.0.0.1"); - col.Append(3585395774); + col.Append(0xD5B4CC3E); col.Append(0); const in_addr ip = MakeIPv4(0x12345678); col.Append(ip); @@ -962,7 +962,7 @@ TEST(ColumnsCase, ColumnIPv4) ASSERT_EQ(5u, col.Size()); EXPECT_EQ(MakeIPv4(0xffffffff), col.At(0)); EXPECT_EQ(MakeIPv4(0x0100007f), col.At(1)); - EXPECT_EQ(MakeIPv4(3585395774), col.At(2)); + EXPECT_EQ(MakeIPv4(0xD5B4CC3E), col.At(2)); EXPECT_EQ(MakeIPv4(0), col.At(3)); EXPECT_EQ(ip, col.At(4)); diff --git a/ut/utils.cpp b/ut/utils.cpp index 19cbaa10..6e164ca9 100644 --- a/ut/utils.cpp +++ b/ut/utils.cpp @@ -426,7 +426,7 @@ std::ostream& operator<<(std::ostream& ostr, const ItemView& item_view) { } case Type::IPv4: { in_addr addr; - addr.s_addr = ntohl(item_view.get()); + addr.s_addr = htonl(item_view.get()); ostr << addr; break; } diff --git a/ut/value_generators.cpp b/ut/value_generators.cpp index f47e8c34..89d3a3a7 100644 --- a/ut/value_generators.cpp +++ b/ut/value_generators.cpp @@ -186,11 +186,11 @@ std::string FooBarGenerator(size_t i) { std::vector MakeIPv4s() { return { - MakeIPv4(0x12345678), // 255.255.255.255 + MakeIPv4(0xFFFFFFFF), // 255.255.255.255 + MakeIPv4(0x12345678), // 120.86.52.18 MakeIPv4(0x0100007f), // 127.0.0.1 - MakeIPv4(3585395774), + MakeIPv4(0xD5B4CC3E), // 62.204.180.213 MakeIPv4(0), - MakeIPv4(0x12345678), }; }