Bugfix: set json read/write setting for binary mode too#282
Bugfix: set json read/write setting for binary mode too#282alex-clickhouse merged 2 commits intomainfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull request overview
This PR makes JSON read/write modes more explicit by always sending ClickHouse’s binary JSON format settings for Binary mode (instead of relying on server defaults), and adds integration tests to verify the settings behavior and override precedence.
Changes:
- Explicitly sets
output_format_binary_write_json_as_string=0whenJsonReadMode.Binary. - Explicitly sets
input_format_binary_read_json_as_string=0whenJsonWriteMode.Binary. - Adds tests to verify settings for
String/Binary/Nonemodes and thatQueryOptions.CustomSettingscan override the auto JSON setting.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| ClickHouse.Driver/Json/JsonWriteMode.cs | Updates enum docs to reflect explicit server setting behavior for Binary mode. |
| ClickHouse.Driver/Json/JsonReadMode.cs | Updates enum docs to reflect explicit server setting behavior for Binary mode. |
| ClickHouse.Driver/ClickHouseUriBuilder.cs | Injects explicit JSON format settings for both Binary and String modes, leaving None as “don’t send”. |
| ClickHouse.Driver.Tests/Types/JsonModeTests.cs | Adds coverage for setting injection/omission across JSON modes. |
| ClickHouse.Driver.Tests/ClickHouseClientQueryOptionsTests.cs | Adds test that per-query custom settings override the auto JSON setting. |
|
@codex[agent] review |
|
Codex Review: Didn't find any major issues. Swish! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Previously, setting JsonReadMode = String set the setting output_format_binary_write_json_as_string = 1. But JsonReadMode = Binary did not set it to 0, and relied on the server default to work. But the server setting is not necessarily 0, so this needs to be set explicitly.