bvaradar commented on a change in pull request #2014:
URL: https://github.com/apache/hudi/pull/2014#discussion_r477439176
##########
File path: hudi-spark/src/main/java/org/apache/hudi/DataSourceUtils.java
##########
@@ -248,15 +249,18 @@ public static HoodieWriteClient
createHoodieClient(JavaSparkContext jssc, String
}
public static JavaRDD<WriteStatus> doWriteOperation(HoodieWriteClient
client, JavaRDD<HoodieRecord> hoodieRecords,
- String instantTime, String operation) throws HoodieException {
- if
(operation.equals(DataSourceWriteOptions.BULK_INSERT_OPERATION_OPT_VAL())) {
+ String instantTime, WriteOperationType operation) throws HoodieException
{
+ if (operation == WriteOperationType.BULK_INSERT) {
Review comment:
Suggestion : Instead of if-else, by using switch-case, we can take
advantage of checkstyle to ensure we are not missing handling of any operation
types .
##########
File path: hudi-spark/src/main/java/org/apache/hudi/DataSourceUtils.java
##########
@@ -248,15 +249,18 @@ public static HoodieWriteClient
createHoodieClient(JavaSparkContext jssc, String
}
public static JavaRDD<WriteStatus> doWriteOperation(HoodieWriteClient
client, JavaRDD<HoodieRecord> hoodieRecords,
- String instantTime, String operation) throws HoodieException {
- if
(operation.equals(DataSourceWriteOptions.BULK_INSERT_OPERATION_OPT_VAL())) {
+ String instantTime, WriteOperationType operation) throws HoodieException
{
Review comment:
Also, since we have changed this to enum, can you also update
DataSourceWriteOptions to reference this enum. It is good that both the enum
values and those defined in DataSourceWriteOptions are consistent. So, we can
do this without any backwards compatibility issues.
```
val OPERATION_OPT_KEY = "hoodie.datasource.write.operation"
val BULK_INSERT_OPERATION_OPT_VAL = WriteOperationType.BULK_INSERT.name
val INSERT_OPERATION_OPT_VAL = WriteOperationType.INSERT.name
val UPSERT_OPERATION_OPT_VAL = WriteOperationType.UPSERT.name
val DELETE_OPERATION_OPT_VAL = WriteOperationType.DELETE.name
val BOOTSTRAP_OPERATION_OPT_VAL = WriteOperationType.BOOTSTRAP.name
val DEFAULT_OPERATION_OPT_VAL = UPSERT_OPERATION_OPT_VAL
```
##########
File path: hudi-spark/src/main/java/org/apache/hudi/DataSourceUtils.java
##########
@@ -248,15 +249,18 @@ public static HoodieWriteClient
createHoodieClient(JavaSparkContext jssc, String
}
public static JavaRDD<WriteStatus> doWriteOperation(HoodieWriteClient
client, JavaRDD<HoodieRecord> hoodieRecords,
- String instantTime, String operation) throws HoodieException {
- if
(operation.equals(DataSourceWriteOptions.BULK_INSERT_OPERATION_OPT_VAL())) {
+ String instantTime, WriteOperationType operation) throws HoodieException
{
+ if (operation == WriteOperationType.BULK_INSERT) {
Option<BulkInsertPartitioner> userDefinedBulkInsertPartitioner =
createUserDefinedBulkInsertPartitioner(client.getConfig());
return client.bulkInsert(hoodieRecords, instantTime,
userDefinedBulkInsertPartitioner);
- } else if
(operation.equals(DataSourceWriteOptions.INSERT_OPERATION_OPT_VAL())) {
+ } else if (operation == WriteOperationType.INSERT) {
return client.insert(hoodieRecords, instantTime);
} else {
// default is upsert
+ if (operation != WriteOperationType.UPSERT) {
Review comment:
@sreeram26 : I think it is better to throw an exception in this case. We
saw some cases where uses intended to use bulk insert but had a typo in the
operation type.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
