linliu-code commented on code in PR #19709:
URL: https://github.com/apache/hudi/pull/19709#discussion_r3839847864
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/spark/sql/hudi/command/SqlKeyGenerator.scala:
##########
@@ -99,7 +99,15 @@ class SqlKeyGenerator(props: TypedProperties) extends
BuiltinKeyGenerator(props)
override def getPartitionPath(record: GenericRecord): String = {
Review Comment:
Checked reachability empirically: it is not reached, so no
`ArrayIndexOutOfBoundsException` occurs and `MergeIntoKeyGenerator` does not
need the size guard for this path.
Ran pk-less + MOR + `GLOBAL_BLOOM` with `update.partition.path=false` and a
partial `UPDATE SET`, in two arms, one assigning the partition column and one
omitting it (both assigning the ordering field, which `checkUpdatingActions`
requires). Both succeed with the correct row:
assigns-partition-col -> 1,a,nyc,200,2026-08-11,dt=2026-08-11
omits-partition-col -> 1,a,nyc,200,2026-08-11,dt=2026-08-11
I had predicted the first would throw on the ordinal read, so I instrumented
`mergeForPartitionUpdatesAndDeletionsIfNeeded` and `inferPartitionPath` to find
out why it did not. The entry-point log never fired for either arm, so the
pk-less merge does not enter the global-index merge stage at all, and neither
this change nor `MergeIntoKeyGenerator.getPartitionPath` sees a partial record.
What I did not establish is which gate excludes it, whether the global
lookup finds no existing location for the auto-generated key, or something
earlier. So I would call this unreachable-as-verified rather than
unreachable-by-construction. Happy to add the guard defensively if you would
rather not rely on that, though it would be untestable from SQL as things stand.
--
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]