Copilot commented on code in PR #9496:
URL: https://github.com/apache/seatunnel/pull/9496#discussion_r2178853905
##########
seatunnel-connectors-v2/connector-hudi/src/main/java/org/apache/seatunnel/connectors/seatunnel/hudi/sink/HudiSinkFactory.java:
##########
@@ -115,6 +115,11 @@ public TableSink createSink(TableSinkFactoryContext
context) {
.put(
HudiSinkOptions.CDC_ENABLED.key(),
String.valueOf(hudiTableConfig.isCdcEnabled()));
+
+ catalogTable
+ .getOptions()
+ .put(HudiSinkOptions.PRECOMBINE_FIELD.key(),
hudiTableConfig.getPreCombineField());
+
Review Comment:
Guard against a null or empty pre-combine field here to avoid inserting a
null value into the options map, which could lead to a runtime NPE. For
example, only call `put(...)` when `hudiTableConfig.getPreCombineField()` is
non-null and non-empty.
```suggestion
if (StringUtils.isNoneEmpty(hudiTableConfig.getPreCombineField())) {
catalogTable
.getOptions()
.put(HudiSinkOptions.PRECOMBINE_FIELD.key(),
hudiTableConfig.getPreCombineField());
}
```
##########
seatunnel-connectors-v2/connector-hudi/src/main/java/org/apache/seatunnel/connectors/seatunnel/hudi/catalog/HudiCatalog.java:
##########
@@ -233,6 +234,7 @@ public void createTable(TablePath tablePath, CatalogTable
table, boolean ignoreI
.setPayloadClassName(HoodieAvroPayload.class.getName())
.setCDCEnabled(
Boolean.parseBoolean(table.getOptions().get(CDC_ENABLED.key())))
+
.setPreCombineField(table.getOptions().get(PRECOMBINE_FIELD.key()))
Review Comment:
If `PRECOMBINE_FIELD` is missing or null in the options, calling
`setPreCombineField(null)` could lead to an unexpected runtime error in the
Hudi builder. Consider retrieving the option with a default or guarding the
call.
##########
seatunnel-connectors-v2/connector-hudi/src/main/java/org/apache/seatunnel/connectors/seatunnel/hudi/config/HudiTableConfig.java:
##########
@@ -125,6 +129,7 @@ public static List<HudiTableConfig> of(ReadonlyConfig
connectorConfig) {
.opType(connectorConfig.get(OP_TYPE))
.recordKeyFields(connectorConfig.get(RECORD_KEY_FIELDS))
.partitionFields(connectorConfig.get(PARTITION_FIELDS))
+
.preCombineField(connectorConfig.get(PRECOMBINE_FIELD))
Review Comment:
Using `connectorConfig.get(PRECOMBINE_FIELD)` without a default will throw
if the option is not provided. If this field is meant to be optional, consider
using `getOptional(...)` or supplying a default value before mapping to avoid a
configuration error.
```suggestion
.preCombineField(connectorConfig.getOptional(PRECOMBINE_FIELD).orElse(null))
```
--
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]
