Copilot commented on code in PR #9605:
URL: https://github.com/apache/seatunnel/pull/9605#discussion_r2227075322
##########
seatunnel-connectors-v2/connector-clickhouse/src/main/java/org/apache/seatunnel/connectors/seatunnel/clickhouse/util/ClickhouseCatalogUtil.java:
##########
@@ -36,7 +36,8 @@ public String columnToConnectorType(Column column) {
if (column.getSinkType() != null) {
columnType = column.getSinkType();
} else {
- columnType =
ClickhouseTypeConverter.INSTANCE.reconvert(column).getColumnType();
+ columnType = column.isNullable() ? "Nullable("+
ClickhouseTypeConverter.INSTANCE.reconvert(column).getColumnType() + ")":
+
ClickhouseTypeConverter.INSTANCE.reconvert(column).getColumnType();
Review Comment:
The `ClickhouseTypeConverter.INSTANCE.reconvert(column).getColumnType()`
call is duplicated in both branches of the ternary operator, causing
unnecessary repeated computation. Extract this to a variable.
```suggestion
String reconvertedColumnType =
ClickhouseTypeConverter.INSTANCE.reconvert(column).getColumnType();
columnType = column.isNullable() ? "Nullable(" +
reconvertedColumnType + ")" : reconvertedColumnType;
```
##########
seatunnel-connectors-v2/connector-clickhouse/src/main/java/org/apache/seatunnel/connectors/seatunnel/clickhouse/util/ClickhouseCatalogUtil.java:
##########
@@ -36,7 +36,8 @@ public String columnToConnectorType(Column column) {
if (column.getSinkType() != null) {
columnType = column.getSinkType();
} else {
- columnType =
ClickhouseTypeConverter.INSTANCE.reconvert(column).getColumnType();
+ columnType = column.isNullable() ? "Nullable("+
ClickhouseTypeConverter.INSTANCE.reconvert(column).getColumnType() + ")":
+
ClickhouseTypeConverter.INSTANCE.reconvert(column).getColumnType();
Review Comment:
[nitpick] The ternary operator creates a long, hard-to-read line. Consider
extracting the base column type to a variable or using if-else statements for
better readability.
```suggestion
if (column.isNullable()) {
columnType = "Nullable(" +
ClickhouseTypeConverter.INSTANCE.reconvert(column).getColumnType() + ")";
} else {
columnType =
ClickhouseTypeConverter.INSTANCE.reconvert(column).getColumnType();
}
```
--
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]