Fix potential crash caused by concurrent mutation of underlying const PREWHERE columns

azat · azat · commit 25675b2c7079 · 2025-10-16T20:29:23.000+02:00
The issue was that in MergeTreeReadTask we may have column that has
other references, usually it is a constant column that created during
analysis (that is not a constant anymore where we call shrink, i.e.
after `materialize()`), and we do not need to mutate such column anyway.

v2: move code from MergeTreeSplitPrewhereIntoReadSteps.cpp::addClonedDAGToDAG() into MergeTreeReadTask
diff --git a/src/Storages/MergeTree/MergeTreeReadTask.cpp b/src/Storages/MergeTree/MergeTreeReadTask.cpp
@@ -351,7 +351,12 @@ MergeTreeReadTask::BlockAndProgress MergeTreeReadTask::read()
     if (read_result.num_rows != 0)
     {
         for (const auto & column : read_result.columns)
-            column->assumeMutableRef().shrinkToFit();
+        {
+            /// We may have columns that has other references, usually it is a constant column that has been created during analysis
+            /// (that will not be const here anymore, i.e. after materialize()), and we do not need to shrink it anyway.
+            if (column->use_count() == 1)
+                column->assumeMutableRef().shrinkToFit();
+        }
         block = sample_block.cloneWithColumns(read_result.columns);
     }
 
diff --git a/tests/queries/0_stateless/03680_mergetree_shrink_const_from_prewhere.reference b/tests/queries/0_stateless/03680_mergetree_shrink_const_from_prewhere.reference
@@ -0,0 +1,3 @@
+1
+2
+3
diff --git a/tests/queries/0_stateless/03680_mergetree_shrink_const_from_prewhere.sql b/tests/queries/0_stateless/03680_mergetree_shrink_const_from_prewhere.sql
@@ -0,0 +1,9 @@
+DROP TABLE IF EXISTS const_node;
+CREATE TABLE const_node (`v` Nullable(UInt8)) ENGINE = MergeTree ORDER BY tuple();
+SYSTEM STOP MERGES const_node;
+INSERT INTO const_node VALUES (1);
+INSERT INTO const_node VALUES (2);
+INSERT INTO const_node VALUES (3);
+-- Here we have condition with a constant "materialize(255)", for which convertToFullColumnIfConst() will return underlying column w/o copying,
+-- and later shrinkToFit() will be called from multiple threads on this column, and leads to UB
+SELECT v FROM const_node PREWHERE and(materialize(255), *) ORDER BY v;

Original file line number	Diff line number	Diff line change
`@@ -351,7 +351,12 @@ MergeTreeReadTask::BlockAndProgress MergeTreeReadTask::read()`
`351`	`351`	`if (read_result.num_rows != 0)`
`352`	`352`	`{`
`353`	`353`	`for (const auto & column : read_result.columns)`
`354`		`- column->assumeMutableRef().shrinkToFit();`
	`354`	`+ {`
	`355`	`+ /// We may have columns that has other references, usually it is a constant column that has been created during analysis`
	`356`	`+ /// (that will not be const here anymore, i.e. after materialize()), and we do not need to shrink it anyway.`
	`357`	`+ if (column->use_count() == 1)`
	`358`	`+ column->assumeMutableRef().shrinkToFit();`
	`359`	`+ }`
`355`	`360`	`block = sample_block.cloneWithColumns(read_result.columns);`
`356`	`361`	`}`
`357`	`362`