Workflow [PR], commit [1ba225a]

Summary: ✅

My current hypothesis is that this behavior might be due to toIPv4OrDefault() / toIPv6OrDefault() reading from non–null-terminated buffers when operating on Dynamic values backed by FixedString.

This means they have a bug that must be fixed.

My current hypothesis is that this behavior might be due to toIPv4OrDefault() / toIPv6OrDefault() reading from non–null-terminated buffers when operating on Dynamic values backed by FixedString.

This means they have a bug that must be fixed.

This may also result from certain settings combinations I am testing out to reproduce locally.

The bug doesn't occur on the local machine, even without this change. I tested running the test 1000 times with different settings similar to those in failed CI jobs. The issue might be related to other factors such as architecture, OS, etc.

Simplify test, add debug information: select * from affected table to see original values

Flaky test 01872_functions_to_subcolumns_analyzer failure unrelated to this PR in https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=88269&sha=85c240b66a1e61a1f09d136c14e58451803b04f4&name_0=PR

@zlareb1 I think it might be an issue with the de-/serialization, at some points we use littleEndian and at other points we use POD-type (native format).

diff --git a/src/DataTypes/Serializations/SerializationIPv4andIPv6.cpp b/src/DataTypes/Serializations/SerializationIPv4andIPv6.cpp
index 115edacc6f7..9d3643492fe 100644
--- a/src/DataTypes/Serializations/SerializationIPv4andIPv6.cpp
+++ b/src/DataTypes/Serializations/SerializationIPv4andIPv6.cpp
@@ -128,20 +128,14 @@ template <typename IPv>
 void SerializationIP<IPv>::serializeBinary(const Field & field, WriteBuffer & ostr, const FormatSettings &) const
 {
     IPv x = field.safeGet<IPv>();
-    if constexpr (std::is_same_v<IPv, IPv6>)
-        writeBinary(x, ostr);
-    else
-        writeBinaryLittleEndian(x, ostr);
+    writeBinary(x, ostr);
 }
 
 template <typename IPv>
 void SerializationIP<IPv>::deserializeBinary(DB::Field & field, DB::ReadBuffer & istr, const DB::FormatSettings &) const
 {
     IPv x;
-    if constexpr (std::is_same_v<IPv, IPv6>)
-        readBinary(x, istr);
-    else
-        readBinaryLittleEndian(x, istr);
+    readBinary(x.toUnderType(), istr);
     field = NearestFieldType<IPv>(x);
 }

Here are 2 points where we use Little Endian for Ipv4. But then in deserializeBinary for example, we use readBinary which is not little endian necessarily.

@zlareb1 I think it might be an issue with the de-/serialization, at some points we use littleEndian and at other points we use POD-type (native format).

@raimannma Interesting. Would you like to take this on and make the necessary changes? Adding a brief reproducer test would also be helpful.

@zlareb1 I think it might be an issue with the de-/serialization, at some points we use littleEndian and at other points we use POD-type (native format).

@raimannma Interesting. Would you like to take this on and make the necessary changes? Adding a brief reproducer test would also be helpful.

raimannma@48c74e3
This might fix it, but I am not so sure what side effects it could have and if it is a breaking change.