Skip to content

Fix remaining_length accounting after Huffman decode in regression/composed predictors - #2

Merged
alexey-milovidov merged 1 commit into
ClickHouse:ClickHouse/v3.3.2from
groeneai:fix-sz3-lorenzo-reg-remaining-length
Jul 17, 2026
Merged

Fix remaining_length accounting after Huffman decode in regression/composed predictors#2
alexey-milovidov merged 1 commit into
ClickHouse:ClickHouse/v3.3.2from
groeneai:fix-sz3-lorenzo-reg-remaining-length

Conversation

@groeneai

Copy link
Copy Markdown

Fixes a remaining_length byte-accounting bug that causes silent data loss for the ALGO_LORENZO_REG compression algorithm when used via the ClickHouse SZ3 codec (ClickHouse/ClickHouse#110336).

Root cause

HuffmanEncoder::decode() advances the read pointer past the encoded stream but does not update the caller-owned remaining_length bound. Callers must account for the consumed bytes themselves.

RegressionPredictor::load() did this incorrectly:

regression_coeff_quant_inds = encoder.decode(c, coeff_size);
encoder.postprocess_decode();
remaining_length -= coeff_size * sizeof(int);   // WRONG

coeff_size * sizeof(int) is the uncompressed index count in bytes, but decode() only advanced c by the Huffman-compressed stream size (sizeof(size_t) + encodedLength). Since the compressed stream is smaller, this over-subtracts and understates remaining_length for the following main-quantizer encoder.load(), whose tree bound check tree_bytes > remaining_length then spuriously throws SZ3 Huffman: tree exceeds compressed buffer.

RegressionPredictor is only used by ALGO_LORENZO_REG, so ALGO_INTERP/ALGO_INTERP_LORENZO are unaffected. Small element counts fail deterministically; large counts happen to leave enough slack in the main stream to survive the understated bound (in ClickHouse this shows up as: n=500 fails, n=5000 works).

Fix

Account for exactly the bytes decode() consumed, via pointer difference:

const uchar *decode_start = c;
regression_coeff_quant_inds = encoder.decode(c, coeff_size);
encoder.postprocess_decode();
remaining_length -= static_cast<size_t>(c - decode_start);

ComposedPredictor::load() had the mirror defect (it decoded its selection stream but never decremented remaining_length at all, leaving a loose bound). Tightened the same way.

…mposed predictors

RegressionPredictor::load decremented remaining_length by the uncompressed
index count (coeff_size * sizeof(int)) after Huffman decode(), but decode()
advances the read pointer only by the compressed stream size. The uncompressed
count overshoots the compressed stream, understating remaining_length for the
subsequent quantizer encoder.load(), whose bound check then spuriously rejects
valid data with "SZ3 Huffman: tree exceeds compressed buffer".

RegressionPredictor is only used by ALGO_LORENZO_REG, so a column compressed
with CODEC(SZ3('ALGO_LORENZO_REG', ...)) could be written on insert but fail
every subsequent read with CORRUPTED_DATA (effective data loss). Small element
counts are affected; large counts happen to leave enough slack to survive the
understated bound.

Account for exactly the bytes decode() consumed via pointer difference.
ComposedPredictor::load had the mirror defect (it never decremented
remaining_length for its selection stream); tightened the same way.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
2 out of 5 committers have signed the CLA.

✅ alexey-milovidov
✅ groeneai
❌ ayzk
❌ gsylvand
❌ guoxiliu
You have signed the CLA already but the status is still pending? Let us recheck it.

@groeneai

Copy link
Copy Markdown
Author

cc @alexey-milovidov @scanhex12 — could you review? This fixes the ALGO_LORENZO_REG data-loss reported in ClickHouse/ClickHouse#110336 (small element counts fail every read with SZ3 Huffman: tree exceeds compressed buffer). The remaining_length bound was decremented by the uncompressed index count instead of the compressed stream size after Huffman decode. Once merged I'll bump the ClickHouse submodule and add a regression test.

@alexey-milovidov
alexey-milovidov changed the base branch from master to ClickHouse/v3.3.2 July 17, 2026 02:46
@alexey-milovidov
alexey-milovidov merged commit ff760cc into ClickHouse:ClickHouse/v3.3.2 Jul 17, 2026
alexey-milovidov added a commit to ClickHouse/ClickHouse that referenced this pull request Jul 17, 2026
Advance the `contrib/sz3` submodule from `dda0caee` to `ff760cc`, the current
tip of the `ClickHouse/v3.3.2` branch. In addition to the quantizer NaN cast
fix already pinned, this brings in the fix for `remaining_length` accounting
after Huffman `decode` in `RegressionPredictor`/`ComposedPredictor`
(ClickHouse/SZ3#2), which otherwise made a column compressed with
`CODEC(SZ3('ALGO_LORENZO_REG', ...))` fail to decompress after being written.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants