-
Notifications
You must be signed in to change notification settings - Fork 49
Implemented writing JSON as string and converting struct to json #554
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
/windsurf review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces support for a new JSON type in ClickHouse sink connector, updating type dispatch, writer logic, and adding tests to cover JSON serialization in both schema-based and schemaless modes.
- Adds
JSON
toType
enum and dispatch logic inColumn
- Extends
ClickHouseWriter
to conditionally write JSON as string under binary format - Adds helper methods and tests for JSON record generation and end-to-end validation
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
File | Description |
---|---|
src/test/java/.../SchemalessTestData.java | New createJSONType to generate schemaless JSON SinkRecords |
src/test/java/.../SchemaTestData.java | New createJSONType to generate schema-based JSON SinkRecords |
src/test/java/.../ClickHouseTestHelpers.java | Bumped default ClickHouse version for tests |
src/test/java/.../ClickHouseSinkTaskWithSchemaTest.java | Added test for writing JSON with RowBinary+input_format_binary_read_json_as_string |
src/test/java/.../ClickHouseSinkTaskSchemalessTest.java | Added schemaless JSON writing test |
src/main/java/.../db/mapping/Type.java | Added JSON enum |
src/main/java/.../db/mapping/Column.java | Dispatches JSON type for JSON columns |
src/main/java/.../db/ClickHouseWriter.java | Implements JSON writing logic using GSON and new config flag |
Comments suppressed due to low confidence (2)
src/main/java/com/clickhouse/kafka/connect/sink/db/ClickHouseWriter.java:77
- The variable name
binaryFormatWrtiteJsonAsString
contains a typo; consider renaming it tobinaryFormatWriteJsonAsString
for clarity.
private boolean binaryFormatWrtiteJsonAsString = false;
src/main/java/com/clickhouse/kafka/connect/sink/db/ClickHouseWriter.java:556
- Consider adding a unit or integration test to cover this error path when
input_format_binary_read_json_as_string
is not set, ensuring this exception is thrown as expected.
throw new RuntimeException("Writing JSON in binary is not supported yet. Use `input_format_binary_read_json_as_string=1` in clickhouse settings to allow writing as string");
Summary
input_format_binary_read_json_as_string
Closes: #543
Checklist
Delete items not relevant to your PR: