linliu-code commented on code in PR #19709:
URL: https://github.com/apache/hudi/pull/19709#discussion_r3836664129


##########
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 = {
     val partitionPath = originalKeyGen.map {
-      _.getKey(record).getPartitionPath
+      // Resolve the partition path on its own where the key generator exposes 
it. Going through
+      // BaseKeyGenerator#getKey would also compute and validate the record 
key, which a MOR partial
+      // update legitimately leaves unset: the merged record is materialised 
against
+      // WRITE_PARTIAL_UPDATE_SCHEMA and so carries only the columns named in 
UPDATE SET. Callers
+      // that want the record key validated still ask for it, via getKey or 
getRecordKey.
+      case baseKeyGen: BaseKeyGenerator => baseKeyGen.getPartitionPath(record)

Review Comment:
   Done, and you are right that my framing was wrong. I only ever exercised the 
type-mismatching shape (double into a null-or-long slot) and generalised "the 
intermediate state is loud" from it. Your `UPDATE SET name` case serializes 
cleanly and writes a wrong row, which makes this unsafe to land alone.
   
   Changed `Closes #19708` to `Addresses #19708`, and added a 
do-not-merge-alone note at the top pointing at #19713. The Risk Level section 
now shows both shapes side by side rather than claiming the intermediate state 
is always loud. Same correction applied to the internal companion PR.



-- 
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]

Reply via email to