unlock unlimited code runs for $79, limited time →
NOSSLETTER.techLIVE
back to OSSpedia
error-handlingtesting-strategy

Supply _sample_factor to the per-part read path used by mutations and merges

Project: ClickHouse/ClickHouse | Language: C++ | Concepts: error-handling, testing-strategy | Difficulty: mid
Link: https://github.com/ClickHouse/ClickHouse/pull/109813

The problem

Mutations (like ALTER ... UPDATE or DELETE) that reference the virtual column _sample_factor would fail mid-execution with an error:
Unexpected const virtual column: _sample_factor. This left the mutation stuck and unable to complete. The root cause was that the per-part read path used by mutations and merges did not supply a value for _sample_factor, even though the SELECT path did.

Why it's non-trivial

A naive fix might try to always supply _sample_factor or patch only the error message. However, _sample_factor is a virtual column only materialized in SELECT queries with or without a SAMPLE clause, and its value must be consistent and safe for replication. The mutation read path is different from the SELECT path, and simply copying logic could break invariants or introduce subtle bugs, especially with other virtual columns.

The approach

The fix seeds the _sample_factor field in the per-part mutation/merge read path, mirroring what the SELECT path does when no SAMPLE clause is present. The value is always set to 1.0, which is the correct default when no sampling is requested.

Key code diff:

diff --git a/src/Storages/MergeTree/MergeTreeSequentialSource.cpp b/src/Storages/MergeTree/MergeTreeSequentialSource.cpp
index 7cd580d1bd0e..c63b461a735f 100644
--- a/src/Storages/MergeTree/MergeTreeSequentialSource.cpp
+++ b/src/Storages/MergeTree/MergeTreeSequentialSource.cpp
@@ -330,6 +330,9 @@ Pipe createMergeTreeSequentialSource(
     info->part_starting_offset_in_query = data_part.part_starting_offset_in_query;
     info->const_virtual_fields.emplace("_part_index", info->part_index_in_query);
     info->const_virtual_fields.emplace("_part_starting_offset", info->part_starting_offset_in_query);
+    /// No `SAMPLE` clause reaches this path, so the sample factor is 1 - the same value
+    /// `ReadFromMergeTree` publishes for a query without `SAMPLE`.
+    info->const_virtual_fields.emplace("_sample_factor", 1.0);

What this does:

  • Adds _sample_factor to the set of constant virtual fields for each part when reading for mutations/merges.
  • Sets its value to 1.0, which matches the value used in SELECTs without a SAMPLE clause.
  • The comment clarifies that this is correct because the mutation path never sees a SAMPLE clause.

Test coverage:

New regression tests were added to verify the fix and prevent regressions:

diff --git a/tests/queries/0_stateless/04510_mutation_query_plan_only_virtual_columns.sql b/tests/queries/0_stateless/04510_mutation_query_plan_only_virtual_columns.sql
new file mode 100644
index 000000000000..2bce644b80ea
--- /dev/null
+++ b/tests/queries/0_stateless/04510_mutation_query_plan_only_virtual_columns.sql
@@ -0,0 +1,57 @@
+-- Tests that _sample_factor is materialized by the per-part read path. See issue #78465.
+
+DROP TABLE IF EXISTS t_mut_sf;
+CREATE TABLE t_mut_sf (c0 UInt32, u Float64) ENGINE = MergeTree ORDER BY c0 SAMPLE BY c0;
+INSERT INTO t_mut_sf VALUES (1, 0), (5, 0);
+
+SELECT 'select', _sample_factor FROM t_mut_sf ORDER BY c0 LIMIT 1;
+
+SET mutations_sync = 2;
+
+ALTER TABLE t_mut_sf UPDATE u = _sample_factor WHERE c0 < 2;
+SELECT 'update rhs', u FROM t_mut_sf ORDER BY c0;
+
+ALTER TABLE t_mut_sf UPDATE u = 7 WHERE _sample_factor = 1 AND c0 > 4;
+SELECT 'update where', u FROM t_mut_sf ORDER BY c0;
+
+ALTER TABLE t_mut_sf DELETE WHERE _sample_factor = 1 AND c0 < 2;
+SELECT 'alter delete', count() FROM t_mut_sf;
+
+DELETE FROM t_mut_sf WHERE _sample_factor = 1 AND c0 > 4;
+SELECT 'lightweight delete', count() FROM t_mut_sf;
+
+DROP TABLE t_mut_sf;
...

What this does:

  • Creates tables and runs mutations using _sample_factor in both the assignment and predicate.
  • Verifies that the value is as expected and that no errors are thrown.
  • Additional checks ensure that similar names in tuple subcolumns, lambda parameters, and real columns do not interfere.

From the review thread

No notable review discussion on this PR.

Takeaways

  • When supporting virtual columns in multiple code paths, ensure all relevant execution paths (not just SELECT) materialize required virtuals, or else mid-execution errors can occur.
  • Always add regression tests for previously missing or broken behaviors, especially when the bug only appears in non-SELECT operations.
  • Comments in code should clarify why a value is set a certain way, especially for subtle invariants (e.g., why _sample_factor is always 1.0 for mutations).