fix: honor CompressionZSTD for native block compression

[must108]

Summary

This PR fixes HTTP native block compression settings so CompressionZSTD and CompressionLZ4 no longer behave the same.

Fixes #1772

Checklist

Delete items not relevant to your PR:

• Unit and integration tests covering the common scenarios were added

[must108]

@chernser @kavirajk bumping, would love for you guys to take a look!

[must108]

@chernser @kavirajk bumping, would love for you guys to take a look!

[github-actions]

Summary

This PR fixes a real bug: previously, both CompressionZSTD and CompressionLZ4 for HTTP native block compression only set compress=1/decompress=1 without telling the server which codec to use, so the server effectively defaulted to LZ4 regardless of the client's selection. The fix factors a helper applyHTTPNativeCompressionSettings that additionally sets network_compression_method and (for ZSTD) network_zstd_compression_level, and routes both the query and batch paths through it. The approach is sound and the refactor avoids drift between the two call sites.

Should fix

• clickhouse_options.go:73 — the doc comment on Compression.Level says "this only applies to lz4, lz4hc, zlib, and brotli compression algorithms". With this PR, ZSTD now also honors Level (via network_zstd_compression_level). Update the comment so users discover the new behavior.
• The new test only exercises the read path (SELECT getSetting(...)) through httpConnect.query. The fix in httpBatch.Send() shares the helper but is never directly executed by the test — a regression that broke only the batch branch would not be caught. Consider extending the table to also do a PrepareBatch / Append / Send round-trip with CompressionZSTD and verify the server accepts the ZSTD-compressed request body.
• network_compression_method is now set via query.Set in createRequest (conn_http.go:645), which overrides any user-provided value in Options.Settings. A caller who deliberately set network_compression_method in their own settings will now have it silently overridden. Either honor a user-supplied value or document this precedence in the README block you already updated.

Nits

• nit: applyHTTPNativeCompressionSettings's early-return guard if method != CompressionLZ4 && method != CompressionZSTD duplicates the switch filter both callers already do. Not harmful, but redundant — happy either way.
• nit: conn_http_batch.go:243 — the b.conn.opt != nil check is unreachable; dialHttp requires a non-nil opt. Only the opt.Compression != nil check is meaningful.
• nit: a one-line doc comment on applyHTTPNativeCompressionSettings ("sets the ClickHouse HTTP query parameters for native block compression") would help future readers grasp intent without tracing call sites.
• nit: CompressionLZ4HC is not handled here (consistent with prior behavior), but worth a quick thought — should a user who selected LZ4HC have network_compression_method=LZ4 set on their behalf? Probably out of scope for this PR; flagging in case it's worth a follow-up.

Verdict

Approve once the stale Compression.Level doc comment is updated. The other items are non-blocking improvements.

[must108]

@mshustov good to check again!

[github-actions]

Review

Summary

This PR fixes #1772, where the HTTP native block compression path set compress=1/decompress=1 but never told the server which codec to use — so CompressionZSTD silently behaved like CompressionLZ4. The fix introduces applyHTTPNativeCompressionSettings, which additionally sets network_compression_method (and network_zstd_compression_level for ZSTD when a level is configured) on both the query and batch paths. The approach is correct: settings are serialized to HTTP query params via fmt.Sprint in conn_http.go, so the int level value is encoded fine, and the helper preserves the prior compress/decompress semantics for each direction (request body compressed only on the batch path). README and the Compression.Level doc comment are updated accordingly. Core logic looks good.

Should fix

• Batch path is untested. The new test exercises only the query path: QueryRow("SELECT getSetting('network_compression_method')") flows through conn_http_query.go. The matching change in conn_http_batch.go (Send) has no coverage. Consider a round-trip insert under CompressionZSTD (or asserting the codec server-side after a batch) so a regression in the batch branch is caught.
• std/database/sql (OpenDB) path not covered. Per the review checklist, coverage is expected for both the native and std APIs. The fix lives below both surfaces in conn_http.go, so risk is low, but a std-path assertion would close the gap the checklist calls out.

Nits

• nit: The level-extraction block

  compressionLevel := 0
  if h.opt != nil && h.opt.Compression != nil {
      compressionLevel = h.opt.Compression.Level
  }

  is duplicated verbatim in conn_http_query.go and conn_http_batch.go. Passing *Compression (or *Options) into the helper would remove the repetition and the redundant nil guards (opt is always set in dialHttp, and Compression is non-nil whenever compression is LZ4/ZSTD).
• nit: settings["network_zstd_compression_level"] = level stores an int while every other setting in the map is a string. It works (serialized via fmt.Sprint), but strconv.Itoa(level) would keep the map value types consistent.

Verdict

Approve — the fix is correct and has a test for the query path. Adding batch-path (and ideally std-path) coverage before merge would be good but is non-blocking.

[github-actions]

🤖 Claude review

Fixes #1772: the HTTP native block compression path set compress=1/decompress=1 but never told the server which codec to use, so CompressionZSTD silently behaved like CompressionLZ4 for server→client blocks. The new applyHTTPNativeCompressionSettings helper additionally sends network_compression_method (and network_zstd_compression_level when a level is set) on both the query and batch paths. The fix is correct and well-scoped — approve.

What checks out:

• Client→server blocks were already encoded with the right codec/level (blockCompressor built from Method+Level at dial); the missing piece was exactly the server-side setting this PR adds.
• Per-request settings serialize via fmt.Sprint (the int level is fine) and take precedence over connection-level URL params, and options.settings is never nil.
• The regression test self-verifies via getSetting that both settings reach the server, and the insert/select round trip exercises ZSTD compression in both directions. Naming and structure match tests/issues/ conventions.

Minor points (non-blocking): the helper unconditionally overwrites a user-supplied network_compression_method (see inline), and the native TCP path retains the same server→client codec asymmetry (see general note).

Blind spots:

• One CI matrix job failed (Go 1.26 / ClickHouse 26.3); I could not fetch its logs from this environment to rule out a relation to the new test — the other nine matrix jobs passed.
• No database/sql variant of the test, though the std driver over HTTP shares the exact same httpConnect query/batch code paths, so coverage is effectively shared.
• Server-side acceptance of out-of-range ZSTD levels was not validated against a live server.

Verdict: ✅ Approve

General findings

• 💡 Nit — nit: native TCP path has the same server→client codec asymmetry
  Over the native TCP protocol the server also picks the response-block codec from network_compression_method (default LZ4), and clickhouse-go never sends that setting on the TCP path either — so with CompressionZSTD over TCP, client→server blocks are ZSTD but server→client blocks stay LZ4, the same asymmetry Setting CompressionZSTD has no effect #1772 reported for HTTP. The client decodes either transparently, so this is invisible except in bandwidth/CPU. Fine to keep this PR scoped to HTTP, but a follow-up applying the same setting on the TCP path would give full parity.

_{Inline comments are attached to the relevant lines. This summary updates in place on re-review.}

[kavirajk]

@must108 do you think we should tackle TCP side also in this PR? Also works if you can create a an issue to tack the TCP.

[kavirajk]

I see we pass the network_compression_method in HTTP requests correctly now 👍

But I don't see we fix the places where we choose compression reader/writer to correctly decode the blocks? e.g: now we send non-default compression say zstd, and blocks are now compressed with zstd instead of lz4 (default). how does our query path decodes it correctly? I don't see relevant code is touched in this PR.

Just wondering how it was working and how it will work now?