Backport #72392 to 24.11: Fix data race in Squashing with LowCardinality

robot-clickhouse · robot-clickhouse · commit e6104bc5a3ee · 2024-11-26T13:14:53.000Z
diff --git a/src/Columns/ColumnLowCardinality.h b/src/Columns/ColumnLowCardinality.h
@@ -190,6 +190,26 @@ class ColumnLowCardinality final : public COWHelper<IColumnHelper<ColumnLowCardi
             callback(dictionary.getColumnUniquePtr());
     }
 
+    void forEachSubcolumnRecursively(RecursiveColumnCallback callback) const override
+    {
+        /** It is important to have both const and non-const versions here.
+          * The behavior of ColumnUnique::forEachSubcolumnRecursively differs between const and non-const versions.
+          * The non-const version will update a field in ColumnUnique.
+          * In the meantime, the default implementation IColumn::forEachSubcolumnRecursively uses const_cast,
+          * so when the const version is called, the field will still be mutated.
+          * This can lead to a data race if constness is expected.
+          */
+        callback(*idx.getPositionsPtr());
+        idx.getPositionsPtr()->forEachSubcolumnRecursively(callback);
+
+        /// Column doesn't own dictionary if it's shared.
+        if (!dictionary.isShared())
+        {
+            callback(*dictionary.getColumnUniquePtr());
+            dictionary.getColumnUniquePtr()->forEachSubcolumnRecursively(callback);
+        }
+    }
+
     void forEachSubcolumnRecursively(RecursiveMutableColumnCallback callback) override
     {
         callback(*idx.getPositionsPtr());
