Hash the type-derived parameter in the ALP, FPC and GCD codecs

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

The problem

In ClickHouse, certain compression codecs (ALP, FPC, GCD) derive internal parameters (like the width of a float or integer) from the column type, but did not include these parameters in their hash computation. In the "Compact" storage format, columns using the same codec hash share a compressed stream. If two columns with different widths (e.g., Float32 and Float64) used the same codec, they would incorrectly share a codec object, leading to errors like CANNOT_COMPRESS or suboptimal compression.

Why it's non-trivial

A naive fix might just check for mismatched types at the point of compression or try to avoid sharing streams, but that would only mask the underlying problem. The real issue is at the level of the cache key: the codec's hash must uniquely represent all parameters that affect its behavior, including those derived from the column type. Failing to do so means the same hash could represent incompatible codecs, leading to subtle and hard-to-debug bugs.

The approach

The fix ensures that all type-derived parameters are included in the codec hash. This is done by updating the updateHash method for each affected codec to include the relevant parameter.

ALP Codec:

 void CompressionCodecALP::updateHash(SipHash & hash) const
 {
     getCodecDesc()->updateTreeHash(hash, /* ignore_aliases */ true);
+    hash.update(float_width);
 }

This change adds the float_width (derived from the column type) to the hash for the ALP codec, ensuring that codecs for Float32 and Float64 columns are hashed differently.

FPC Codec:

 void CompressionCodecFPC::updateHash(SipHash & hash) const
 {
     getCodecDesc()->updateTreeHash(hash, /*ignore_aliases=*/ true);
+    hash.update(float_width);
 }

Similarly, this adds float_width to the hash for the FPC codec.

GCD Codec:

 void CompressionCodecGCD::updateHash(SipHash & hash) const
 {
     getCodecDesc()->updateTreeHash(hash, /*ignore_aliases=*/ true);
+    hash.update(gcd_bytes_size);
     hash.update(is_signed_type);
 }

Here, gcd_bytes_size (derived from the integer type) is added to the hash, along with is_signed_type.

Testing:

New SQL test files were added to check that:

• Inserts that previously failed (due to misaligned float widths) now succeed.
• Compression output is identical for mixed-width and single-width tables, regardless of column order.
• The bug is fixed for all three codecs.

Example from the test:

CREATE TABLE alp_reject (n UInt64, f64 Float64 CODEC(ALP), f32 Float32 CODEC(ALP)) ENGINE = MergeTree ORDER BY n ...
INSERT INTO alp_reject SELECT number, sin(number) * 1000, sin(number) * 1000 FROM numbers(101);
SELECT count() FROM alp_reject;

This would previously throw an error; with the fix, it succeeds.

From the review thread

groeneai:
The three byte-equality assertions compare exact sum(data_compressed_bytes) between a mixed-width table and two single-column tables. In a Compact part that sum is one data.bin, and the writer emits one compressed block per column per granule, so the granule count scales per-block framing overhead into the total. computeIndexGranularity derives that count from index_granularity_bytes and index_granularity, and clickhouse-test randomizes both, so a draw that splits the mixed table into a different granule sequence than the two singles made the assertions print 0 on a correct build: 0.0492% of slow-build draws, 0.3475% of release-range draws, about a 9.4% chance of a red on this PR's own flaky check and recurring afterwards. Fixed by pinning both settings, at their product defaults, on the seven size-compared tables; a no-random-merge-tree-settings tag was deliberately not used, since the per-setting pin is strictly narrower. Verified on one fixed binary: three forced witness draws each fail without the pin and pass with it, and the pinned test still reddens on unpatched master with all three equality assertions reddening independently.

groeneai:
All 8 codecs receiving a column type were audited. ALP and FPC both had the defect and both are fixed; SZ3, DoubleDelta, Gorilla, T64 and Delta already hash their type-derived state. GCD is the same defect but is owned by open #113798, so it is deliberately untouched here.

Takeaways

• Always include all type-derived parameters in cache or hash keys for objects whose behavior depends on those parameters.
  Omitting them can cause subtle bugs where different logical objects are treated as the same.

• Tests for data structure or cache correctness should control all relevant randomness (e.g., table settings) to avoid flaky results.
  Otherwise, correct code may appear to fail due to test instability.

• When fixing cache key or hashing bugs, ensure that the fix matches the root cause and does not just patch over symptoms at higher layers.
  The hash must represent all state that affects behavior, not just explicit configuration.