Add JsonReadMode and JsonWriteMode settings for configurable JSON handling#182
Add JsonReadMode and JsonWriteMode settings for configurable JSON handling#182alex-clickhouse merged 7 commits intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds configurable JSON handling modes for ClickHouse JSON type operations through new JsonReadMode and JsonWriteMode settings. The default write behavior has changed from binary encoding to string serialization to minimize upgrade impact.
Changes:
- Introduces
JsonReadMode(Binary/String) andJsonWriteMode(Binary/String) enums with connection string support - Changes default
JsonWriteModefrom Binary to String (breaking change, but maintains compatibility) - Binary write mode now requires explicit type registration for POCOs
- Adds comprehensive test coverage for all mode combinations and roundtrip scenarios
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| ClickHouse.Driver/Json/JsonReadMode.cs | New enum defining Binary (default) and String read modes |
| ClickHouse.Driver/Json/JsonWriteMode.cs | New enum defining Binary and String write modes, with String as default |
| ClickHouse.Driver/Types/JsonType.cs | Implements mode-dependent read/write logic, enforces Binary mode restrictions |
| ClickHouse.Driver/TypeSettings.cs | Adds JsonReadMode and JsonWriteMode fields to settings record |
| ClickHouse.Driver/ADO/ClickHouseConnection.cs | Injects server settings based on JSON modes (output_format_binary_write_json_as_string, input_format_binary_read_json_as_string) |
| ClickHouse.Driver/ADO/ClickHouseConnectionStringBuilder.cs | Adds JsonReadMode and JsonWriteMode properties with enum parsing |
| ClickHouse.Driver/ADO/ClickHouseClientSettings.cs | Adds JsonReadMode and JsonWriteMode properties, updates Equals/GetHashCode |
| ClickHouse.Driver.Tests/Types/JsonTypeTests.cs | Updates existing tests to use Binary mode where needed, adds mode-specific test names |
| ClickHouse.Driver.Tests/Types/JsonModeTests.cs | New comprehensive test suite covering all mode combinations and scenarios |
| ClickHouse.Driver.Tests/BulkCopy/BulkCopyTests.cs | Updates JSON tests to use appropriate write modes |
| examples/DataTypes_005_JsonType.cs | New example demonstrating all JSON mode combinations with POCOs |
| RELEASENOTES.md | Documents breaking change with migration table and attribute behavior differences |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
mzitnik
left a comment
mzitnik
left a comment
There was a problem hiding this comment.
General question. Can we call InitAsync from WriteToServerAsync only once
await bulkCopy.InitAsync();
await bulkCopy.WriteToServerAsync([new object[] { 1u, data }]);
|
|
||
| // ClickHouse 25.3 returns numbers as strings in this scenario | ||
| if (TestUtilities.ServerVersion == Version.Parse("25.3")) | ||
| Assert.That(int.Parse(parsed["age"].ToString()), Is.EqualTo(30)); |
There was a problem hiding this comment.
Should we add implicit conversion to numbers if possible for this version (so it will be easy for user client )
There was a problem hiding this comment.
With this setting enabled, the client just returns a plain string. So it basically looks like:
- 25.3:
"{"a": "123"}" - Later version:
"{"a": 123}"
So there's no place for us to do a conversion here, it's up to the end user to handle the string in whatever way they want. We also don't have any type info.
Good idea, yeah we could hide the |
Adds config options for JSON read and write handling.
The settings control
input_format_binary_read_json_as_stringandoutput_format_binary_write_json_as_string, with corresponding handling differences on the client side.Writing behavior:
JsonWriteMode.String(default)JsonWriteMode.BinarystringArgumentExceptionJsonObjectToJsonString()ArgumentExceptionJsonNodeToJsonString()ArgumentExceptionJsonSerializer.Serialize()JsonSerializer.Serialize()ClickHouseJsonSerializationExceptionReading behavior:
String: sets output_format_binary_write_json_as_string=1 and just returns the string as it's sent by the server.
Binary (default): returns JsonNode. For the future at some point we'll add POCO deserialization as well.
The defaults (write string, read binary) are set to minimize impact on people upgrading from the previous version.
Also added a .None mode, for cases when users do not want any parameters added to the query (eg readonly, see the changes to that example).
Closes #93
Note
Introduces configurable JSON handling and propagates it through the driver.
JsonReadMode/JsonWriteModeenums with defaults (Read:Binary->JsonObject, Write:String-> serialize/forward strings);Nonemode avoids setting server paramsClickHouseConnectionStringBuilder,ClickHouseClientSettings,ClickHouseConnection/ClickHouseUriBuilder,TypeSettings, and public API surfaceJsonTypenow adapts: read returnsstringin read String mode; write serializes to string in write String mode; Binary write supports only registered POCOs and throws forstring/JsonNodeinput_format_binary_read_json_as_string/output_format_binary_write_json_as_stringvia URI when modes demandJsonModeTests, adjusted bulk copy JSON tests, serialization tests exclude Json; fixes int/long expectations; extensive roundtrip and error-path coverageJsonWriteMode=None); updates release notes accordinglyWritten by Cursor Bugbot for commit ceb8332. This will update automatically on new commits. Configure here.