Skip to content

Honor selected compression method for native TCP responses - #568

Open
beejmaxx wants to merge 3 commits into
ClickHouse:masterfrom
beejmaxx:fix/native-response-compression
Open

beejmaxx wants to merge 3 commits into
ClickHouse:masterfrom
beejmaxx:fix/native-response-compression

Conversation

@beejmaxx

@beejmaxx beejmaxx commented Sep 25, 2026 •

Copy link
Copy Markdown

Problem

SetCompressionMethod(CompressionMethod::ZSTD) compresses blocks sent by the client with ZSTD, but the native Query packet carries only a boolean compression flag, not a codec name. The server chooses its response codec from network_compression_method, which this client does not send. A server configured for LZ4 can therefore return LZ4 blocks to a client configured for ZSTD. The client decodes them successfully, so the mismatch is silent.

Change

For LZ4 or ZSTD, send the corresponding network_compression_method with each query when the caller has not set it explicitly. Preserve the caller's setting and flags. Do not add a setting when compression is disabled or the server revision is too old for string-serialized settings.

Compatibility

This changes behavior for users whose server constrains network_compression_method. For example, a readonly user whose permitted server codec differs from the configured client codec can now receive a READONLY error for a query that previously succeeded with the server's codec. The client propagates the server error. The caller can choose a permitted codec, disable compression, or supply an explicit permitted per-query setting where the API allows one.

Verification

  • Before the fix, regression tests failed for both mismatched cases: client ZSTD with server LZ4, and client LZ4 with server ZSTD.
  • After the fix, 169 protocol, stream, client, and roundtrip tests passed on macOS arm64 with AppleClang 21 and ClickHouse 25.12.1.649; 2 existing tests were disabled.
  • Captured server responses for both codecs and both explicit cross-codec overrides. Each case returned three checksum-validated frames using the expected codec, and the client decoded all 100 result rows.
  • A scripted older server test inspects the emitted query bytes to verify that no automatic string setting is sent. A readonly integration test covers the rejection and a permitted explicit override.
  • git diff --check passed. Bazel and Linux/Windows CI have not been run locally.

Fixes #556.

@CLAassistant

CLAassistant commented Sep 25, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@beejmaxx
beejmaxx force-pushed the fix/native-response-compression branch from 476a4c4 to 25ee5f9 Compare September 25, 2026 10:52
@beejmaxx
beejmaxx marked this pull request as ready for review September 25, 2026 10:57
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.

Native TCP: CompressionMethod::ZSTD is not honored for server -> client blocks (network_compression_method never sent)

2 participants