Fix ColumnIPv4 byte-order documentation and add IP round-trip tests

clickhouse/columns/ip4.cpp

@@ -23,7 +23,7 @@ ColumnIPv4::ColumnIPv4(std::vector<uint32_t>&& data)
     : Column(Type::CreateIPv4())
 {
     for (auto& addr : data) {
-        addr = htonl(addr);
+        addr = ntohl(addr);
     }
     data_ = std::make_shared<ColumnUInt32>(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;
 }
 

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<uint32_t>&& 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.

clickhouse/columns/lowcardinality.cpp

@@ -233,9 +233,8 @@ inline void AppendToDictionary(Column& dictionary, const ItemView & item) {
             column_down_cast<ColumnDateTime>(dictionary).AppendRaw(item.get<uint32_t>());
             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<ColumnIPv4>(dictionary).Append(ntohl(item.get<uint32_t>()));
+            // GetItem returns the stored host-order value; Append(uint32_t) expects network order.
+            column_down_cast<ColumnIPv4>(dictionary).Append(htonl(item.get<uint32_t>()));
             return;
         case Type::IPv6: {
             in6_addr addr;

tests/simple/main.cpp

@@ -517,7 +517,7 @@ inline void IPExample(Client &client) {
 
         auto v4 = std::make_shared<ColumnIPv4>();
         v4->Append("127.0.0.1");
-        v4->Append(3585395774);
+        v4->Append(0xD5B4CC3E);
         v4->Append(0);
 
         auto v6 = std::make_shared<ColumnIPv6>();

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<ColumnString>();
+    auto ip = std::make_shared<ColumnIPv4>();
+    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<ColumnString>()->At(i);
+            const auto out_string = block[1]->AsStrict<ColumnString>()->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<ColumnString>();
+    auto ip = std::make_shared<ColumnIPv6>();
+    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<ColumnString>()->At(i);
+            const auto out_string = block[1]->AsStrict<ColumnString>()->At(i);
+            EXPECT_EQ(out_string, address_string) << id;
+        }
+    });
+    EXPECT_EQ(total_rows, ip->Size());
+}
+
 TEST_P(ClientCase, Date) {
     Block b;
 

ut/columns_ut.cpp

@@ -954,15 +954,15 @@ 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);
 
     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));
 

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<uint32_t>());
+            addr.s_addr = htonl(item_view.get<uint32_t>());
             ostr << addr;
             break;
         }

ut/value_generators.cpp

@@ -186,11 +186,11 @@ std::string FooBarGenerator(size_t i) {
 
 std::vector<in_addr> 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),
     };
 }