Skip to content

Fix compressed spooling: compare decoded length against integer segment sizes#624

Open
arpitjain099 wants to merge 1 commit into
trinodb:masterfrom
arpitjain099:chore/compressed-spool-size-int
Open

Fix compressed spooling: compare decoded length against integer segment sizes#624
arpitjain099 wants to merge 1 commit into
trinodb:masterfrom
arpitjain099:chore/compressed-spool-size-int

Conversation

@arpitjain099

Copy link
Copy Markdown
Member

Description

CompressedQueryDataDecoder.decode guards the compressed spooling path by checking the segment length against the sizes the coordinator reports in the segment metadata. Those sizes arrive as strings (the _SegmentMetadataTO TypedDict types segmentSize and uncompressedSize as str, and the metadata built across the spooling tests uses string values like "10"), but the code compared them directly to len(data), which is an int. An int is never equal to a decimal string, so not len(data) == metadata["segmentSize"] is always true and every compressed segment raised RuntimeError before it was ever decompressed. The error text gives it away, it reads "Expected to read 29 bytes but got 29" and still raises.

This makes json+zstd and json+lz4 spooling unusable from the client even though both encodings are advertised in the X-Trino-Encoding header. The lz4 path already coerces the size with int(...) when calling lz4.block.decompress, so the two size checks here just needed the same treatment.

The fix wraps both metadata sizes in int() before comparing, matching the existing lz4 code. I also added a unit test that decodes a real zstd-compressed segment with the string-typed metadata the protocol actually sends. It fails against the old code with the "expected N, got N" RuntimeError and passes with the fix. The existing spooling tests missed this because they all decode with encoding="json" or mock the decoder, so the compressed decode path had no coverage.

Non-technical explanation

Compressed results returned through the spooling protocol were always rejected with an error. This fixes the size check so compressed segments decode correctly.

Release notes

(x) Release notes are required, with the following suggested text:

* Fix decoding of compressed segments in the spooling protocol, which previously failed with a size-mismatch error for every `json+zstd` and `json+lz4` segment.

Signed-off-by: Arpit Jain <arpitjain099@gmail.com>
@cla-bot cla-bot Bot added the cla-signed label Jul 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

1 participant