voonhous commented on code in PR #19709:
URL: https://github.com/apache/hudi/pull/19709#discussion_r3842325485
##########
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.
Review Comment:
Don't pin it, I was wrong to suggest it belongs here. Checked directly: the
`.toLong` throws for any non-numeric `TimestampBasedKeyGenerator` output,
including a complete record with the key present and a valid `dt`:
```
complete record, dt="2026-08-11", schema "dt timestamp" ->
NumberFormatException: For input string: "2026-08-11"
complete record, no sql partition schema -> OK '2026-08-11'
```
So a date-formatted delegate is incompatible with a `TIMESTAMP` partition
schema regardless of this change, and nothing pre-empted it on master either.
That wants its own issue rather than a test here.
`testTimestampDelegateResolvesAnAbsentPartitionFieldToTheEpoch` covers the
half this change actually widens, which was the ask. Resolving.
--
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]
