perf: move query params to POST body (fix #454) - #455
Open
cjrh wants to merge 5 commits into
Open
Conversation
cjrh
requested review from
abonander,
joe-clickhouse,
kavirajk and
peter-leonov-ch
as code owners
August 7, 2026 14:08
cjrh
force-pushed
the
perf-move-query-params-post-body
branch
from
August 7, 2026 23:49
d4cb50d to
620d8bd
Compare
There was a problem hiding this comment.
Pull request overview
This PR changes how ClickHouse server-side query parameters (param_*, including those created via Query::param()) are transported: instead of being sent as URI query parameters, they are now sent as multipart/form-data fields in the POST request body to avoid URI length limits. It also fixes RowBinary serde for Option<Vec<Uuid>> behind the uuid feature, and strengthens CI/test coverage around these behaviors.
Changes:
- Send
param_*settings as multipart form fields in the POST body (while keeping ordinary settings/roles in the URI query string), and control framing headers for multipart requests. - Add unit/integration tests for multipart request construction, header handling, and large parameter payloads.
- Fix
uuid_vec::optionRowBinary serde soOption<Vec<Uuid>>correctly preserves the Nullable marker and uses the UUID adapter on deserialization (plus regression tests).
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/it/query.rs | Adds integration coverage for ordinary settings + params, and for large parameter payloads that would previously risk URI-length issues. |
| src/test/handlers.rs | Adds a test-only handler to record full HTTP requests for transport-level assertions. |
| src/serde.rs | Fixes uuid_vec::option serde to preserve Nullable encoding and use the crate’s UUID-vector adapter. |
| src/rowbinary/tests.rs | Adds regression tests for RowBinary round-tripping of Nullable(Array(UUID)) via Option<Vec<Uuid>>. |
| src/request_body.rs | Makes RequestBody::full accept Into<Bytes> to support multipart bodies efficiently. |
| src/query.rs | Implements multipart request-body construction for param_*, validates names, selects a non-colliding boundary, and adds transport/unit tests. |
| src/lib.rs | Updates Client::with_header docs to reflect new header framing behavior. |
| docs/index.mdx | Removes outdated documentation claiming server-side parameter binding is unsupported. |
| .github/workflows/ci.yml | Expands CI coverage (clippy with uuid feature) and tests against an additional minimum ClickHouse version. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
cjrh
force-pushed
the
perf-move-query-params-post-body
branch
from
September 4, 2026 14:57
620d8bd to
08a5fff
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #454.
Server-side query parameters supplied with [
Query::param()] are now sent asmultipart/form-datafields in the POST request body instead of URI query parameters. This avoids URI-length limits when queries bind many parameters or large values.Candidate CHANGELOG entry
Note the point about the HTTP headers. For users setting custom headers, this is a breaking change.
Implementation Notes
Query::param()continues to serialize values with the existing parameter serializer and records them asparam_<name>settings. While building a query request, clickhouse-rs now separates thoseparam_*settings from ordinary ClickHouse settings:multipart/form-dataPOST body containing aqueryfield followed by one field perparam_<name>.Content-Typeand removes caller-suppliedContent-LengthandTransfer-Encoding, allowing the HTTP body implementation to frame the request correctly.Content-Lengthbehavior.Unit tests cover multipart field construction, boundary collisions, parameter validation, header handling, compression, and preservation of ordinary settings. Integration coverage includes parameterized queries with large values that would otherwise approach URI-length limits.
The branch also fixes the opt-in
uuidRowBinary adapter forOption<Vec<Uuid>>. Its optional wrapper now delegatesSomevalues through the crate's UUID-vector serializer/deserializer, preserving ClickHouse's binary UUID representation. Regression tests cover both nullable states.On the UUID RowBinary change
I use clickhouse-rs via diesel-clickhouse, and I use UUIDs a lot which is how I found this.
This is a targeted fix for structs that map a Rust
Option<Vec<uuid::Uuid>>to a ClickHouseNullable(Array(UUID))column using#[serde(with = "clickhouse::serde::uuid_vec::option")].For RowBinary, a non-null
Nullable(...)value starts with a0byte;NULLis represented by1. A presentArray(UUID)then contains its length, followed by each UUID encoded as two little-endianu64values.Previously, serializing
Some(vec)delegated directly to the UUID-vector adapter. That adapter correctly encoded the array and UUID elements, but bypassed Serde's outerOptionhandling, so it omitted the required leading0non-null marker. The generated bytes were therefore not validNullable(Array(UUID))RowBinary.The old deserializer had the inverse problem: it handled the outer
Optionbut delegated a present vector to Serde's genericVec<Uuid>implementation, rather than the crate's UUID-vector adapter. It consequently could not deserialize valid non-nullNullable(Array(UUID))bytes.The fix preserves the outer
Optionlayer and delegates only its present inner value to the UUID-vector adapter. The regression test verifies exact bytes and round-trips bothSome(vec![...])andNone.Nonewas already encoded correctly. This change is behind the opt-inuuidfeature and is independent of the multipart query-parameter change. I do need it for diesel-clickhouse though.Checklist
Delete items not relevant to your PR: