[ISSUE #478] Support ClickHouse source and sink by joeCarf · Pull Request #496 · apache/rocketmq-connect

joeCarf · 2023-05-06T16:20:00Z

What is the purpose of the change

support ClickHouse connectors (both source and sink)

issue link: #478

Brief changelog

XX

Verifying this change

XXXX

Follow this checklist to help us incorporate your contribution quickly and easily. Notice, it would be helpful if you could finish the following 5 checklist(the last one is not necessary)before request the community to review your PR.

Make sure there is a Github issue filed for the change (usually before you start working on it). Trivial changes like typos do not require a Github issue. Your pull request should address just this issue, without pulling in other changes - one PR resolves one issue.
Format the pull request title like [ISSUE #123] Fix UnknownException when host config not exist. Each commit in the pull request should have a meaningful subject line and body.
Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
Write necessary unit-test(over 80% coverage) to verify your logic correction, more mock a little better when cross module dependency exist. If the new feature or significant change is committed, please remember to add integration-test in test module.
Run mvn -B clean apache-rat:check findbugs:findbugs checkstyle:checkstyle to make sure basic checks pass. Run mvn clean install -DskipITs to make sure unit-test pass. Run mvn clean test-compile failsafe:integration-test to make sure integration-test pass.
If this contribution is large, please file an Apache Individual Contributor License Agreement.

joeCarf · 2023-05-07T01:10:36Z

@odbozhou hi, please take a look! :)

RockteMQ-AI

Summary

This PR modifies 13 file(s) with 1266 lines of diff. Changes look reasonable.

Automated review by github-manager-bot

RockteMQ-AI · 2026-08-05T15:04:37Z

@@ -0,0 +1,51 @@
+##### ClickHouseSourceConnector fully-qualified name


Large diff (1266 lines). Consider breaking into smaller, focused PRs for easier review.

RockteMQ-AI

Summary

PR received and logged for review. This PR requires detailed code review by a maintainer.

Diff size: 1266 lines
Author: joeCarf (CONTRIBUTOR)

Automated review by RockteMQ-AI

RockteMQ-AI

Summary

Review of PR #496: [ISSUE #478] Support ClickHouse source and sink

Findings: 15 issue(s) identified (4 critical).
CLA: unknown

Please address the inline comments above.

Automated review by github-manager-bot

Additional notes (not anchored to a changed line)

[CRITICAL] connectors/rocketmq-connect-clickhouse:1 — No tests are present anywhere in this PR. There are no unit or integration tests for the source task (including offset tracking), sink task, helper client retry logic, or config loading. This is a mandatory gap for a data integration connector where correctness of offset tracking and error handling is critical. (line outside diff)

RockteMQ-AI · 2026-08-23T20:11:12Z

+                }
+            });
+            ps.executeUpdate();
+


Infinite loop bug in insertJson: retryCount is never incremented. The loop while (retryCount < this.retry) will spin forever if insertJson returns false, causing the thread to hang indefinitely.

RockteMQ-AI · 2026-08-23T20:11:12Z

+    }
+
+    private Connection getConnection(String url, Properties properties) throws SQLException {
+        ClickHouseDataSource dataSource = new ClickHouseDataSource(url, properties);


insertJson silently swallows all exceptions and returns false with no logging. The caller has no way to distinguish a transient error from a permanent schema/data error, and the root cause is completely lost.

RockteMQ-AI · 2026-08-23T20:11:12Z

+                    recordList.add(r);
+                }
+                return recordList;
+


System.out.println used for connection diagnostics instead of the SLF4J logger already present in this class. This will not be suppressible via logging configuration and pollutes stdout in production.

RockteMQ-AI · 2026-08-23T20:11:12Z

+        if (config.getAccessToken() != null) {
+            return ClickHouseCredentials.fromAccessToken(config.getAccessToken());
+        }
+        throw new RuntimeException("Credentials cannot be empty!");


ping() creates a new ClickHouseClient on every call but only closes it on success. If ping returns false after all retries, the client opened in the last failed iteration is never closed, leaking a connection resource.

RockteMQ-AI · 2026-08-23T20:11:12Z

+            .build();
+
+        return this.server;
+    }


getCredentials throws a RuntimeException when neither username/password nor accessToken is set, but this is thrown inside the constructor during create(). The error message is not descriptive enough to guide operators. More importantly, a missing-credential config should be caught at connector validation time (in start()), not deferred to client construction.

RockteMQ-AI · 2026-08-23T20:11:12Z

+    }
+
+    public void load(KeyValue props) {
+        properties2Object(props, this);


Config key lookup lowercases the setter name (e.g. setClickHouseHost -> clickhousehost) but the constants file defines CLICKHOUSE_HOST = "clickhousehost". This works, but the mapping is fragile: adding a setter whose lowercase name doesn't match the constant (e.g. setUserName -> username vs CLICKHOUSE_USERNAME = "username") will silently fail to populate the field. userName maps to 'username' which does match, but this convention is undocumented and error-prone.

RockteMQ-AI · 2026-08-23T20:11:12Z

+
+public class ClickHouseSinkConfig extends ClickHouseBaseConfig {
+    public static final Set<String> SINK_REQUEST_CONFIG = new HashSet<String>() {
+        {


SINK_REQUEST_CONFIG only requires clickhousehost and clickhouseport, omitting database, username, and password. A connector started without credentials will pass validation but fail at runtime in getCredentials(). Minimum required fields should include database, username, and password (or accesstoken).

RockteMQ-AI · 2026-08-23T20:11:12Z

+            <groupId>io.openmessaging</groupId>
+            <artifactId>openmessaging-connector</artifactId>
+            <version>0.1.4</version>
+            <scope>compile</scope>


junit version is set to RELEASE, which is a Maven version alias that resolves to the latest available version at build time. This makes builds non-reproducible and can introduce unexpected breaking changes. Pin to a specific version (e.g. 4.13.2).

RockteMQ-AI · 2026-08-23T20:11:12Z

+        List<KeyValue> configs = new ArrayList<>();
+        for (int i = 0; i < maxTasks; i++) {
+            configs.add(this.keyValue);
+        }


taskConfigs adds the same shared KeyValue reference to every task slot. If tasks are run concurrently and any task mutates the KeyValue object, all tasks will be affected. Each task should receive an independent copy of the configuration.

RockteMQ-AI · 2026-08-23T20:11:12Z

+        List<KeyValue> configs = new ArrayList<>();
+        for (int i = 0; i < maxTasks; i++) {
+            configs.add(this.keyValue);
+        }


Same shared-reference issue as ClickHouseSinkConnector.taskConfigs: all task slots receive the same KeyValue object reference.

RockteMQ-AI · 2026-08-24T20:06:06Z

Issue Evaluation

Category: enhancement | Status: Evaluated (PR-like submission)

This issue references #478 and proposes adding ClickHouse connectors (both source and sink).

Note: This appears to be a PR submission. If you have implementation code ready, please submit it as a pull request directly.

Feasibility: Adding ClickHouse connectors is a valuable enhancement for data integration scenarios.

Age: This issue is from May 2023. If this feature is still desired, please confirm or submit a PR.

Automated evaluation by RockteMQ-AI

joeCarf added 8 commits May 5, 2023 17:23

initial commit

5117ede

commit

25e7ba5

rename

54c2517

finish source connector

1957ce8

finish sink connector

eb5033d

add license

969b395

Update README.md

ca0d454

Update connect-standalone.conf

31cd5e4

Merge branch 'apache:master' into dev/clickhouse-source

d5c7461

RockteMQ-AI reviewed Aug 5, 2026

View reviewed changes

RockteMQ-AI reviewed Aug 20, 2026

View reviewed changes

RockteMQ-AI suggested changes Aug 23, 2026

View reviewed changes

		@@ -0,0 +1,51 @@
		##### ClickHouseSourceConnector fully-qualified name

Uh oh!

Conversation

joeCarf commented May 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What is the purpose of the change

Brief changelog

Verifying this change

Uh oh!

joeCarf commented May 7, 2023

Uh oh!

RockteMQ-AI left a comment

Choose a reason for hiding this comment

Summary

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RockteMQ-AI left a comment

Choose a reason for hiding this comment

Summary

Uh oh!

RockteMQ-AI left a comment

Choose a reason for hiding this comment

Summary

Additional notes (not anchored to a changed line)

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RockteMQ-AI commented Aug 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

joeCarf commented May 6, 2023 •

edited

Loading