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]