Check column structure against the declared type in collectOffsetsColumns
Project: ClickHouse/ClickHouse | Language: C++ | Concepts: error-handling, data-structures | Difficulty: senior
Link: https://github.com/ClickHouse/ClickHouse/pull/114087
The problem
When reading from a MergeTree table in ClickHouse, it's possible for a column's declared type (from table metadata) and its actual data (from a data part written before a type-altering ALTER TABLE) to diverge. Previously, this mismatch could cause:
- In debug/sanitizer builds: an abort with a generic "Bad cast from type A to B" error, not naming the column or giving useful context.
- In release builds: silent, unsafe reinterpretation of memory, leading to undefined behavior.
This made debugging and diagnosing such issues difficult and dangerous.
Why it's non-trivial
A naive fix might simply check the top-level type name, but ClickHouse columns can be deeply nested and wrapped (e.g., Array, Nullable, Tuple, Map, LowCardinality, etc.), and the type mismatch can occur at any nesting level. The serialization logic expects the structure of the column to match the declared type exactly at each wrapper, not just at the leaf. A robust solution must recursively validate the structure, matching the logic of the serialization code itself.
The approach
The PR introduces a structural check, columnMatchesTypeStructure, that recursively verifies the column's structure matches the declared type at every serialization-relevant wrapper before any unchecked cast occurs. If a mismatch is detected, it throws a clear exception naming the column and both structures.
Key code snippets and their roles:
-
Recursive structure check function:
+static bool columnMatchesTypeStructure(const IColumn & column, const IDataType & type) +{
- /// A lazily replicated column describes data of its nested column.
- if (const auto * column_replicated = typeid_cast<const ColumnReplicated *>(&column))
-
return columnMatchesTypeStructure(*column_replicated->getNestedColumn(), type); - ...
- if (const auto * type_array = typeid_cast<const DataTypeArray *>(&type))
- {
-
const auto * column_array = typeid_cast<const ColumnArray *>(&column); -
return column_array && columnMatchesTypeStructure(column_array->getData(), *type_array->getNestedType()); - }
- ...
- if (const auto * type_tuple = typeid_cast<const DataTypeTuple *>(&type))
- {
-
const auto * column_tuple = typeid_cast<const ColumnTuple *>(&column); -
if (!column_tuple || column_tuple->tupleSize() != type_tuple->getElements().size()) -
return false; -
for (size_t i = 0; i < column_tuple->tupleSize(); ++i) -
if (!columnMatchesTypeStructure(column_tuple->getColumn(i), *type_tuple->getElement(i))) -
return false; -
return true; - }
- ...
- if (typeid_cast<const DataTypeDynamic *>(&type))
-
return typeid_cast<const ColumnDynamic *>(&column) != nullptr; - return true; +}
**What it does:**
This function recursively checks that the structure of the column matches the declared type for all wrappers (`Array`, `Nullable`, `Tuple`, `Map`, `LowCardinality`, `Variant`, `Object`, `QBit`, `Dynamic`, etc.). For each supported wrapper type, it checks both the type and the corresponding column class, and recurses into nested elements. For leaf types and cases where divergence is legitimate (e.g., a part storing `UInt32` for a column widened to `UInt64`), it returns `true`.
2. **Structural check before serialization:**
```cpp
+ if (!columnMatchesTypeStructure(*res_columns[i], *available_column->type))
+ throw Exception(ErrorCodes::LOGICAL_ERROR,
+ "Column {} is listed with type {} among available columns, but its data has "
+ "incompatible structure {}. It is likely that a type resolved from the table's "
+ "metadata was combined with a column read from a data part with an older type",
+ backQuote(available_column->name), available_column->type->getName(), res_columns[i]->dumpStructure());
What it does:
Before attempting to enumerate streams (which would assert-cast the column to the expected class), this code checks the structure. If the check fails, it throws a detailed exception, naming the column, the expected type, and the actual structure. This prevents both silent memory bugs and unhelpful aborts.
-
Test coverage for edge cases:
Example (from
04904_low_cardinality_missing_column_read.sql):
+-- Keep a part written before LowCardinality was introduced, then read it together with an
+-- Array column missing from that part. The missing column makes fillMissingColumns collect
+-- offsets from c; the reader converts the old nested String data to a low-cardinality
+-- dictionary, exercising the DataTypeLowCardinality branch of
+-- columnMatchesTypeStructure.
**What it does:**
The PR adds tests that simulate reading columns with changed types and missing columns, ensuring the new structural check does not produce false positives and covers all tricky cases (e.g., sparse columns, low cardinality, etc.).
## From the review thread
No notable review discussion on this PR.
## Takeaways
- **Validate data structure at every wrapper, not just at the top level.**
When dealing with nested or wrapped data types, always check the structure recursively to prevent subtle bugs from type mismatches.
- **Fail early and clearly on structural mismatches.**
Throwing a detailed exception before attempting unchecked casts prevents both unsafe behavior and hard-to-debug errors.
- **Mirror validation logic to serialization logic.**
Structure checks should follow the same paths and expectations as the code that will later operate on the data; this ensures consistency and prevents accidental divergence.