Aias00 commented on code in PR #7262:
URL: https://github.com/apache/shenyu/pull/7262#discussion_r4110269193
##########
shenyu-plugin/shenyu-plugin-logging/shenyu-plugin-logging-clickhouse/src/main/java/org/apache/shenyu/plugin/logging/clickhouse/client/ClickHouseLogCollectClient.java:
##########
@@ -129,6 +131,8 @@ public boolean initClient0(@NonNull final
ClickHouseLogCollectConfig.ClickHouseL
final String password = config.getPassword();
final String ttl = StringUtils.defaultIfBlank(config.getTtl(), "30");
database = config.getDatabase();
+ boolean distributed = StringUtils.isNotBlank(config.getClusterName());
+ insertSql = String.format(distributed ?
ClickHouseLoggingConstant.PRE_INSERT_SQL :
ClickHouseLoggingConstant.LOCAL_PRE_INSERT_SQL, database);
Review Comment:
Two notes on this line, both non-blocking.
1. `LOCAL_PRE_INSERT_SQL` is built here from
`ClickHouseLoggingConstant.PRE_INSERT_SQL` by
`String.replace("request_log_distributed", "request_log")`
(`ClickHouseLoggingConstant.java:77`). Keeping one source of truth is good, but
`replace` rewrites *every* occurrence - correct today where there is exactly
one, silently wrong the day someone adds a join, a subselect or a second insert
target to that constant. A `%s` table placeholder, or a test asserting the
occurrence count is 1, would keep it honest.
2. `insertSql` is now computed once at init whereas master formatted it per
`consume0` call. Fine for the normal lifecycle, but if `consume0` ever runs
before a successful `initClient0` you now pass `null` as the SQL instead of the
old, obviously-broken `"null.request_log_distributed"`. Initialising the field
to `PRE_INSERT_SQL` or asserting non-null in `consume0` makes that failure mode
legible.
Routing itself is right: I searched the repo and `request_log_distributed`
is referenced nowhere outside the constants class, so nothing queries the
distributed table and pointing standalone deployments at `request_log` loses no
data. CI green.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
