Merge pull request #46922 from ClickHouse/review-tavplubix

Code review from @tavplubix

src/AggregateFunctions/AggregateFunctionGroupArray.h

@@ -63,6 +63,9 @@ static constexpr const char * getNameByTrait()
 template <typename T>
 struct GroupArraySamplerData
 {
+    /// For easy serialization.
+    static_assert(std::has_unique_object_representations_v<T> || std::is_floating_point_v<T>);
+
     // Switch to ordinary Allocator after 4096 bytes to avoid fragmentation and trash in Arena
     using Allocator = MixedAlignedArenaAllocator<alignof(T), 4096>;
     using Array = PODArray<T, 32, Allocator>;
@@ -97,6 +100,9 @@ struct GroupArrayNumericData;
 template <typename T>
 struct GroupArrayNumericData<T, false>
 {
+    /// For easy serialization.
+    static_assert(std::has_unique_object_representations_v<T> || std::is_floating_point_v<T>);
+
     // Switch to ordinary Allocator after 4096 bytes to avoid fragmentation and trash in Arena
     using Allocator = MixedAlignedArenaAllocator<alignof(T), 4096>;
     using Array = PODArray<T, 32, Allocator>;

src/AggregateFunctions/AggregateFunctionGroupArrayMoving.h

@@ -32,6 +32,9 @@ namespace ErrorCodes
 template <typename T>
 struct MovingData
 {
+    /// For easy serialization.
+    static_assert(std::has_unique_object_representations_v<T> || std::is_floating_point_v<T>);
+
     using Accumulator = T;
 
     /// Switch to ordinary Allocator after 4096 bytes to avoid fragmentation and trash in Arena

src/AggregateFunctions/AggregateFunctionMaxIntersections.h

@@ -118,14 +118,19 @@ public:
         size_t size = value.size();
         writeVarUInt(size, buf);
 
-        for (size_t i = 0; i < size; ++i)
-        {
         /// In this version, pairs were serialized with padding.
         /// We must ensure that padding bytes are zero-filled.
-            char bytes[sizeof(value[0])]{};
-            unalignedStore<PointType>(&bytes[offsetof(typename MaxIntersectionsData<PointType>::Value, first)], value[i].first);
-            unalignedStore<Int64>(&bytes[offsetof(typename MaxIntersectionsData<PointType>::Value, second)], value[i].second);
-            buf.write(bytes, sizeof(value[0]));
+
+        static_assert(offsetof(typename MaxIntersectionsData<PointType>::Value, first) == 0);
+        static_assert(offsetof(typename MaxIntersectionsData<PointType>::Value, second) > 0);
+
+        char zero_padding[offsetof(typename MaxIntersectionsData<PointType>::Value, second) - sizeof(value[0].first)]{};
+
+        for (size_t i = 0; i < size; ++i)
+        {
+            writePODBinary(value[i].first, buf);
+            writePODBinary(zero_padding, buf);
+            writePODBinary(value[i].second, buf);
         }
     }
 

src/Storages/MergeTree/IMergeTreeDataPart.cpp

@@ -880,7 +880,7 @@ void IMergeTreeDataPart::writeMetadata(const String & filename, const WriteSetti
         }
         catch (...)
         {
-            tryLogCurrentException("DataPartStorageOnDiskFull");
+            tryLogCurrentException("IMergeTreeDataPart");
         }
 
         throw;