lokeshj1703 commented on code in PR #9665:
URL: https://github.com/apache/hudi/pull/9665#discussion_r1321161861
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/spark/sql/hudi/ProvidesHoodieConfig.scala:
##########
@@ -208,19 +211,22 @@ trait ProvidesHoodieConfig extends Logging {
val isPartitionedTable = hoodieCatalogTable.partitionFields.nonEmpty
val combineBeforeInsert = hoodieCatalogTable.preCombineKey.nonEmpty &&
hoodieCatalogTable.primaryKeys.nonEmpty
- // try to use sql write operation instead of legacy insert mode. If only
insert mode is explicitly specified, w/o specifying
- // any value for sql write operation, leagcy configs will be honored. But
on all other cases (i.e when neither of the configs is set,
- // or when both configs are set, or when only sql write operation is set),
we honor sql write operation and ignore
- // the insert mode.
+ /*
+ * The sql write operation has higher precedence than the legacy insert
mode.
+ * Only when the legacy insert mode is explicitly set, without setting sql
write operation,
+ * legacy configs will be honored. On all other cases (i.e when both are
set, either is set,
+ * or when only the sql write operation is set), we honor the sql write
operation.
+ */
val useLegacyInsertModeFlow = insertModeSet && !sqlWriteOperationSet
var operation = combinedOpts.getOrElse(OPERATION.key,
if (useLegacyInsertModeFlow) {
// NOTE: Target operation could be overridden by the user, therefore
if it has been provided as an input
// we'd prefer that value over auto-deduced operation.
Otherwise, we deduce target operation type
deduceOperation(enableBulkInsert, isOverwritePartition,
isOverwriteTable, dropDuplicate,
- isNonStrictMode, isPartitionedTable, combineBeforeInsert,
insertMode, autoGenerateRecordKeys)
+ isNonStrictMode, isPartitionedTable, combineBeforeInsert,
insertMode, shouldAutoKeyGen)
} else {
- deduceSparkSqlInsertIntoWriteOperation(isOverwritePartition,
isOverwriteTable, sqlWriteOperation)
+ deduceSparkSqlInsertIntoWriteOperation(isOverwritePartition,
isOverwriteTable,
+ shouldAutoKeyGen, preCombineField, sqlWriteOperation)
Review Comment:
Should we also deduce preCombineField from combinedOpts? I see we specify
preCombineField in `TestInsertTable` insert into tests.
--
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]
