voonhous commented on code in PR #19713:
URL: https://github.com/apache/hudi/pull/19713#discussion_r3852162966
##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/spark/sql/hudi/dml/others/TestPartialUpdateForMergeInto.scala:
##########
@@ -732,12 +732,144 @@ class TestPartialUpdateForMergeInto extends
HoodieSparkSqlTestBase {
}
}
+ // A partial update names only the columns being changed, so the record key
is normally absent
+ // from the assignments. On MOR the global-index tagging stage merges the
incoming record with its
+ // existing version and then asks the key generator for the merged record's
partition path - and
+ // that merged record is materialised against WRITE_PARTIAL_UPDATE_SCHEMA,
which carries only the
+ // assigned columns. Resolving the partition path must therefore not also
require the record key.
+ //
+ // The index types split on whether that tagging stage runs at all:
GLOBAL_BLOOM and GLOBAL_SIMPLE
+ // set mayContainDuplicateLookup on MOR and so reach it, while the
record-index spellings pass
+ // false and short-circuit. Both cells are covered, so the fix is pinned
where it applies and the
+ // already-working path is guarded against regression. The source projects
the partition column in
+ // every case, to keep this independent of partition-column resolution
(ENG-46864).
+ Seq(
+ ("GLOBAL_BLOOM re-keying", Map(
+ "hoodie.index.type" -> "GLOBAL_BLOOM",
+ "hoodie.bloom.index.update.partition.path" -> "false")),
+ ("GLOBAL_SIMPLE re-keying", Map(
+ "hoodie.index.type" -> "GLOBAL_SIMPLE",
+ "hoodie.simple.index.update.partition.path" -> "false")),
+ ("RECORD_INDEX", Map(
+ "hoodie.index.type" -> "RECORD_INDEX",
+ "hoodie.record.index.update.partition.path" -> "false",
+ "hoodie.metadata.enable" -> "true",
+ "hoodie.metadata.record.index.enable" -> "true")),
+ ("GLOBAL_RECORD_LEVEL_INDEX", Map(
+ "hoodie.index.type" -> "GLOBAL_RECORD_LEVEL_INDEX",
+ "hoodie.record.index.update.partition.path" -> "false",
+ "hoodie.metadata.enable" -> "true",
+ "hoodie.metadata.record.index.enable" -> "true"))
Review Comment:
**Correction** to my second paragraph: the matrix does observe
`partitionResolvableFromRecord`. With the arm reverted the untagged record
counts as an insert, `UpsertPartitioner.java:189-216` opens a small-file UPDATE
bucket at the existing fileId, and
`BaseSparkDeltaCommitActionExecutor.java:76-79` diverts it to the COW merge
handle: zero log files, so `validateLogBlock(..., 1, ...)` fails at `:877`. The
duplicate-cell point and the `update.partition.path=true` cell stand. That flag
is not even needed to reach the delete+insert arm: while the RLI partition is
not yet built, `SparkMetadataTableGlobalRecordLevelIndex.java:79-88` falls back
to `GLOBAL_SIMPLE`, whose flag defaults to `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]
