codope commented on code in PR #11216:
URL: https://github.com/apache/hudi/pull/11216#discussion_r1599911496
##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/spark/sql/hudi/dml/TestInsertTable.scala:
##########
@@ -1056,6 +1056,50 @@ class TestInsertTable extends HoodieSparkSqlTestBase {
}
}
+ test("Test combine before bulk insert") {
Review Comment:
can we cover all combinations of `hoodie.combine.before.insert` and
`hoodie.populate.meta.fields`?
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/hudi/HoodieSparkSqlWriter.scala:
##########
@@ -470,7 +470,7 @@ class HoodieSparkSqlWriterInternal {
// Short-circuit if bulk_insert via row is enabled.
// scalastyle:off
- if (hoodieConfig.getBoolean(ENABLE_ROW_WRITER) && operation ==
WriteOperationType.BULK_INSERT) {
+ if (hoodieConfig.getBoolean(ENABLE_ROW_WRITER) && operation ==
WriteOperationType.BULK_INSERT &&
!hoodieConfig.getBooleanOrDefault(HoodieWriteConfig.COMBINE_BEFORE_INSERT)) {
Review Comment:
If meta fields are present, then i think we should still allow row writer
path for bulk insert. So, how about only disabling row writer when
`hoodie.populate.meta.fields` is false and `hoodie.combine.before.insert` is
true?
--
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]