Merge pull request ClickHouse#86719 from ClickHouse/backport/25.8/86410

Avogar · web-flow · commit 8950b791e4e0 · 2025-09-09T13:07:14.000+02:00
Backport ClickHouse#86410 to 25.8: Fix GROUP BY Nullable(JSON)
diff --git a/src/Columns/ColumnArray.cpp b/src/Columns/ColumnArray.cpp
@@ -255,6 +255,25 @@ char * ColumnArray::serializeValueIntoMemory(size_t n, char * memory) const
     return memory;
 }
 
+std::optional<size_t> ColumnArray::getSerializedValueSize(size_t n) const
+{
+    const auto & offsets_data = getOffsets();
+
+    size_t pos = offsets_data[n - 1];
+    size_t end = offsets_data[n];
+
+    size_t res = sizeof(offsets_data[0]);
+    for (; pos < end; ++pos)
+    {
+        auto element_size = getData().getSerializedValueSize(pos);
+        if (!element_size)
+            return std::nullopt;
+        res += *element_size;
+    }
+
+    return res;
+}
+
 
 const char * ColumnArray::deserializeAndInsertFromArena(const char * pos)
 {
diff --git a/src/Columns/ColumnArray.h b/src/Columns/ColumnArray.h
@@ -80,6 +80,7 @@ class ColumnArray final : public COWHelper<IColumnHelper<ColumnArray>, ColumnArr
     void insertData(const char * pos, size_t length) override;
     StringRef serializeValueIntoArena(size_t n, Arena & arena, char const *& begin) const override;
     char * serializeValueIntoMemory(size_t, char * memory) const override;
+    std::optional<size_t> getSerializedValueSize(size_t n) const override;
     const char * deserializeAndInsertFromArena(const char * pos) override;
     const char * skipSerializedInArena(const char * pos) const override;
     void updateHashWithValue(size_t n, SipHash & hash) const override;
diff --git a/src/Columns/ColumnDynamic.h b/src/Columns/ColumnDynamic.h
@@ -196,6 +196,7 @@ class ColumnDynamic final : public COWHelper<IColumnHelper<ColumnDynamic>, Colum
     StringRef serializeValueIntoArena(size_t n, Arena & arena, char const *& begin) const override;
     const char * deserializeAndInsertFromArena(const char * pos) override;
     const char * skipSerializedInArena(const char * pos) const override;
+    std::optional<size_t> getSerializedValueSize(size_t) const override { return std::nullopt; }
 
     void updateHashWithValue(size_t n, SipHash & hash) const override;
 
diff --git a/src/Columns/ColumnMap.cpp b/src/Columns/ColumnMap.cpp
@@ -160,6 +160,11 @@ char * ColumnMap::serializeValueIntoMemory(size_t n, char * memory) const
     return nested->serializeValueIntoMemory(n, memory);
 }
 
+std::optional<size_t> ColumnMap::getSerializedValueSize(size_t n) const
+{
+    return nested->getSerializedValueSize(n);
+}
+
 const char * ColumnMap::deserializeAndInsertFromArena(const char * pos)
 {
     return nested->deserializeAndInsertFromArena(pos);
diff --git a/src/Columns/ColumnMap.h b/src/Columns/ColumnMap.h
@@ -58,6 +58,7 @@ class ColumnMap final : public COWHelper<IColumnHelper<ColumnMap>, ColumnMap>
     void popBack(size_t n) override;
     StringRef serializeValueIntoArena(size_t n, Arena & arena, char const *& begin) const override;
     char * serializeValueIntoMemory(size_t n, char * memory) const override;
+    std::optional<size_t> getSerializedValueSize(size_t n) const override;
     const char * deserializeAndInsertFromArena(const char * pos) override;
     const char * skipSerializedInArena(const char * pos) const override;
     void updateHashWithValue(size_t n, SipHash & hash) const override;
diff --git a/src/Columns/ColumnNullable.cpp b/src/Columns/ColumnNullable.cpp
@@ -198,6 +198,14 @@ char * ColumnNullable::serializeValueIntoMemory(size_t n, char * memory) const
     return getNestedColumn().serializeValueIntoMemory(n, memory);
 }
 
+std::optional<size_t> ColumnNullable::getSerializedValueSize(size_t n) const
+{
+    auto nested_size = getNestedColumn().getSerializedValueSize(n);
+    if (!nested_size)
+        return std::nullopt;
+    return 1 + *nested_size; /// +1 for null mask byte.
+}
+
 const char * ColumnNullable::deserializeAndInsertFromArena(const char * pos)
 {
     UInt8 val = unalignedLoad<UInt8>(pos);
diff --git a/src/Columns/ColumnNullable.h b/src/Columns/ColumnNullable.h
@@ -68,6 +68,7 @@ class ColumnNullable final : public COWHelper<IColumnHelper<ColumnNullable>, Col
     void insertData(const char * pos, size_t length) override;
     StringRef serializeValueIntoArena(size_t n, Arena & arena, char const *& begin) const override;
     char * serializeValueIntoMemory(size_t n, char * memory) const override;
+    std::optional<size_t> getSerializedValueSize(size_t n) const override;
     const char * deserializeAndInsertFromArena(const char * pos) override;
     const char * skipSerializedInArena(const char * pos) const override;
 #if !defined(DEBUG_OR_SANITIZER_BUILD)
diff --git a/src/Columns/ColumnObject.cpp b/src/Columns/ColumnObject.cpp
@@ -896,7 +896,7 @@ StringRef ColumnObject::serializeValueIntoArena(size_t n, Arena & arena, const c
         res.size += data_ref.size;
     }
 
-    /// Second, serialize paths and values in bunary format from dynamic paths and shared data in sorted by path order.
+    /// Second, serialize paths and values in binary format from dynamic paths and shared data in sorted by path order.
     /// Calculate total number of paths to serialize and write it.
     const auto & shared_data_offsets = getSharedDataOffsets();
     size_t offset = shared_data_offsets[static_cast<ssize_t>(n) - 1];
diff --git a/src/Columns/ColumnObject.h b/src/Columns/ColumnObject.h
@@ -146,6 +146,7 @@ class ColumnObject final : public COWHelper<IColumnHelper<ColumnObject>, ColumnO
     StringRef serializeValueIntoArena(size_t n, Arena & arena, char const *& begin) const override;
     const char * deserializeAndInsertFromArena(const char * pos) override;
     const char * skipSerializedInArena(const char * pos) const override;
+    std::optional<size_t> getSerializedValueSize(size_t) const override { return std::nullopt; }
 
     void updateHashWithValue(size_t n, SipHash & hash) const override;
     WeakHash32 getWeakHash32() const override;
diff --git a/src/Columns/ColumnSparse.cpp b/src/Columns/ColumnSparse.cpp
@@ -168,6 +168,11 @@ char * ColumnSparse::serializeValueIntoMemory(size_t n, char * memory) const
     return values->serializeValueIntoMemory(getValueIndex(n), memory);
 }
 
+std::optional<size_t> ColumnSparse::getSerializedValueSize(size_t n) const
+{
+    return values->getSerializedValueSize(getValueIndex(n));
+}
+
 const char * ColumnSparse::deserializeAndInsertFromArena(const char * pos)
 {
     const char * res = nullptr;
diff --git a/src/Columns/ColumnSparse.h b/src/Columns/ColumnSparse.h
@@ -80,6 +80,7 @@ class ColumnSparse final : public COWHelper<IColumnHelper<ColumnSparse>, ColumnS
     void insertData(const char * pos, size_t length) override;
     StringRef serializeValueIntoArena(size_t n, Arena & arena, char const *& begin) const override;
     char * serializeValueIntoMemory(size_t n, char * memory) const override;
+    std::optional<size_t> getSerializedValueSize(size_t n) const override;
     const char * deserializeAndInsertFromArena(const char * pos) override;
     const char * skipSerializedInArena(const char *) const override;
 #if !defined(DEBUG_OR_SANITIZER_BUILD)
diff --git a/src/Columns/ColumnTuple.cpp b/src/Columns/ColumnTuple.cpp
@@ -337,6 +337,21 @@ char * ColumnTuple::serializeValueIntoMemory(size_t n, char * memory) const
     return memory;
 }
 
+std::optional<size_t> ColumnTuple::getSerializedValueSize(size_t n) const
+{
+    size_t res = 0;
+    for (const auto & column : columns)
+    {
+        auto element_size = column->getSerializedValueSize(n);
+        if (!element_size)
+            return std::nullopt;
+        res += *element_size;
+    }
+
+    return res;
+}
+
+
 const char * ColumnTuple::deserializeAndInsertFromArena(const char * pos)
 {
     ++column_length;
diff --git a/src/Columns/ColumnTuple.h b/src/Columns/ColumnTuple.h
@@ -79,6 +79,7 @@ class ColumnTuple final : public COWHelper<IColumnHelper<ColumnTuple>, ColumnTup
     void popBack(size_t n) override;
     StringRef serializeValueIntoArena(size_t n, Arena & arena, char const *& begin) const override;
     char * serializeValueIntoMemory(size_t n, char * memory) const override;
+    std::optional<size_t> getSerializedValueSize(size_t n) const override;
     const char * deserializeAndInsertFromArena(const char * pos) override;
     const char * skipSerializedInArena(const char * pos) const override;
     void updateHashWithValue(size_t n, SipHash & hash) const override;
diff --git a/src/Columns/ColumnVariant.cpp b/src/Columns/ColumnVariant.cpp
@@ -839,6 +839,31 @@ const char * ColumnVariant::skipSerializedInArena(const char * pos) const
     return variants[localDiscriminatorByGlobal(global_discr)]->skipSerializedInArena(pos);
 }
 
+char * ColumnVariant::serializeValueIntoMemory(size_t n, char * memory) const
+{
+    Discriminator global_discr = globalDiscriminatorAt(n);
+    memcpy(memory, &global_discr, sizeof(global_discr));
+    memory += sizeof(global_discr);
+    if (global_discr == NULL_DISCRIMINATOR)
+        return memory;
+
+    return variants[localDiscriminatorByGlobal(global_discr)]->serializeValueIntoMemory(offsetAt(n), memory);
+}
+
+std::optional<size_t> ColumnVariant::getSerializedValueSize(size_t n) const
+{
+    size_t res = sizeof(Discriminator);
+    Discriminator global_discr = globalDiscriminatorAt(n);
+    if (global_discr == NULL_DISCRIMINATOR)
+        return res;
+
+    auto variant_size = variants[localDiscriminatorByGlobal(global_discr)]->getSerializedValueSize(offsetAt(n));
+    if (!variant_size)
+        return std::nullopt;
+
+    return res + *variant_size;
+}
+
 void ColumnVariant::updateHashWithValue(size_t n, SipHash & hash) const
 {
     Discriminator global_discr = globalDiscriminatorAt(n);
diff --git a/src/Columns/ColumnVariant.h b/src/Columns/ColumnVariant.h
@@ -217,6 +217,9 @@ class ColumnVariant final : public COWHelper<IColumnHelper<ColumnVariant>, Colum
     const char * deserializeAndInsertFromArena(const char * pos) override;
     const char * deserializeVariantAndInsertFromArena(Discriminator global_discr, const char * pos);
     const char * skipSerializedInArena(const char * pos) const override;
+    char * serializeValueIntoMemory(size_t n, char * memory) const override;
+    std::optional<size_t> getSerializedValueSize(size_t n) const override;
+
     void updateHashWithValue(size_t n, SipHash & hash) const override;
     WeakHash32 getWeakHash32() const override;
     void updateHashFast(SipHash & hash) const override;
diff --git a/src/Columns/IColumn.cpp b/src/Columns/IColumn.cpp
@@ -561,11 +561,20 @@ IColumnHelper<Derived, Parent>::serializeValueIntoArenaWithNull(size_t n, Arena
             return {memory, 1};
         }
 
-        size_t sz = self.byteSizeAt(n) + 1 /* null map byte */;
-        memory = arena.allocContinue(sz, begin);
+        auto serialized_value_size = self.getSerializedValueSize(n);
+        if (serialized_value_size)
+        {
+            size_t total_size = *serialized_value_size + 1 /* null map byte */;
+            memory = arena.allocContinue(total_size, begin);
+            *memory = 0;
+            self.serializeValueIntoMemory(n, memory + 1);
+            return {memory, total_size};
+        }
+
+        memory = arena.allocContinue(1, begin);
         *memory = 0;
-        self.serializeValueIntoMemory(n, memory + 1);
-        return {memory, sz};
+        auto res = self.serializeValueIntoArena(n, arena, begin);
+        return StringRef(res.data - 1, res.size + 1);
     }
 
     return self.serializeValueIntoArena(n, arena, begin);
diff --git a/src/Columns/IColumn.h b/src/Columns/IColumn.h
@@ -276,6 +276,11 @@ class IColumn : public COW<IColumn>
     /// Return pointer to the end of the serialization data.
     virtual char * serializeValueIntoMemory(size_t /* n */, char * /* memory */) const;
 
+    /// Returns size in bytes required to serialize value into memory using the previous method.
+    /// If size cannot be calculated in advance, return nullopt. In this case serializeValueIntoMemory
+    /// cannot be used and serializeValueIntoArena should be used instead,
+    virtual std::optional<size_t> getSerializedValueSize(size_t n) const { return byteSizeAt(n); }
+
     virtual void batchSerializeValueIntoMemory(std::vector<char *> & /* memories */) const;
 
     /// Nullable variant to avoid calling virtualized method inside ColumnNullable.
diff --git a/tests/queries/0_stateless/03606_nullable_json_group_by.reference b/tests/queries/0_stateless/03606_nullable_json_group_by.reference
@@ -0,0 +1,6 @@
+{"a":42}
+{"a":42}
+\N
+\N
+\N
+{"a":42}
diff --git a/tests/queries/0_stateless/03606_nullable_json_group_by.sql b/tests/queries/0_stateless/03606_nullable_json_group_by.sql
@@ -0,0 +1,6 @@
+select '{"a" : 42}'::Nullable(JSON) group by 1;
+select materialize('{"a" : 42}')::Nullable(JSON) group by 1;
+select null::Nullable(JSON) group by 1;
+select materialize(null)::Nullable(JSON) group by 1;
+select (number % 2 ? null : '{"a" : 42}')::Nullable(JSON) from numbers(4) group by 1;
+

Original file line number	Diff line number	Diff line change
`@@ -160,6 +160,11 @@ char * ColumnMap::serializeValueIntoMemory(size_t n, char * memory) const`
`160`	`160`	`return nested->serializeValueIntoMemory(n, memory);`
`161`	`161`	`}`
`162`	`162`
	`163`	`+std::optional<size_t> ColumnMap::getSerializedValueSize(size_t n) const`
	`164`	`+{`
	`165`	`+ return nested->getSerializedValueSize(n);`
	`166`	`+}`
	`167`	`+`
`163`	`168`	`const char * ColumnMap::deserializeAndInsertFromArena(const char * pos)`
`164`	`169`	`{`
`165`	`170`	`return nested->deserializeAndInsertFromArena(pos);`
Original file line number	Diff line number	Diff line change
`@@ -198,6 +198,14 @@ char * ColumnNullable::serializeValueIntoMemory(size_t n, char * memory) const`
`198`	`198`	`return getNestedColumn().serializeValueIntoMemory(n, memory);`
`199`	`199`	`}`
`200`	`200`
	`201`	`+std::optional<size_t> ColumnNullable::getSerializedValueSize(size_t n) const`
	`202`	`+{`
	`203`	`+ auto nested_size = getNestedColumn().getSerializedValueSize(n);`
	`204`	`+ if (!nested_size)`
	`205`	`+ return std::nullopt;`
	`206`	`+ return 1 + *nested_size; /// +1 for null mask byte.`
	`207`	`+}`
	`208`	`+`
`201`	`209`	`const char * ColumnNullable::deserializeAndInsertFromArena(const char * pos)`
`202`	`210`	`{`
`203`	`211`	`UInt8 val = unalignedLoad<UInt8>(pos);`
Original file line number	Diff line number	Diff line change
`@@ -896,7 +896,7 @@ StringRef ColumnObject::serializeValueIntoArena(size_t n, Arena & arena, const c`
`896`	`896`	`res.size += data_ref.size;`
`897`	`897`	`}`
`898`	`898`
`899`		`- /// Second, serialize paths and values in bunary format from dynamic paths and shared data in sorted by path order.`
	`899`	`+ /// Second, serialize paths and values in binary format from dynamic paths and shared data in sorted by path order.`
`900`	`900`	`/// Calculate total number of paths to serialize and write it.`
`901`	`901`	`const auto & shared_data_offsets = getSharedDataOffsets();`
`902`	`902`	`size_t offset = shared_data_offsets[static_cast<ssize_t>(n) - 1];`
Original file line number	Diff line number	Diff line change
`@@ -168,6 +168,11 @@ char * ColumnSparse::serializeValueIntoMemory(size_t n, char * memory) const`
`168`	`168`	`return values->serializeValueIntoMemory(getValueIndex(n), memory);`
`169`	`169`	`}`
`170`	`170`
	`171`	`+std::optional<size_t> ColumnSparse::getSerializedValueSize(size_t n) const`
	`172`	`+{`
	`173`	`+ return values->getSerializedValueSize(getValueIndex(n));`
	`174`	`+}`
	`175`	`+`
`171`	`176`	`const char * ColumnSparse::deserializeAndInsertFromArena(const char * pos)`
`172`	`177`	`{`
`173`	`178`	`const char * res = nullptr;`