SEPURI-SAI-KRISHNA commented on code in PR #19648:
URL: https://github.com/apache/hudi/pull/19648#discussion_r3820472332
##########
hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/keygen/PartitionPathFormatterBase.java:
##########
@@ -62,11 +62,11 @@ public final S combine(List<String> partitionPathFields,
Object... partitionPath
// Avoid creating [[StringBuilder]] in case there's just one
partition-path part,
// and Hive-style of partitioning is not required
if (!useHiveStylePartitioning && partitionPathParts.length == 1) {
- if (slashSeparatedDatePartitioning) {
- return ((S) ((String) toString(partitionPathParts[0])).replace('-',
'/'));
- } else {
- return tryEncode(handleEmpty(toString(partitionPathParts[0])));
- }
+ S partitionPathPart =
tryEncode(handleEmpty(toString(partitionPathParts[0])));
+ // NOTE: Slash-separated date partitioning only kicks in for a table
partitioned by a single
Review Comment:
You're right, and "all three write paths agree" was doing misleading work in
that NOTE — agreement isn't usefulness when the agreed-on layout can't be read
back. Reworded the last sentence:
```java
// [[org.apache.spark.sql.Row]] and
[[org.apache.spark.sql.catalyst.InternalRow]] paths alike.
// All three agreeing does not make that layout usable, though: the
extra fragments leave
// [[HoodieSparkUtils#doParsePartitionColumnValues]] unable to line
the path up with the
// partition columns, so reading such a table back fails. Tracked in
HUDI issue #19666
```
I've also widened #19666 to cover `CustomKeyGenerator` as its own failure
mode. Worth separating the two, since they fail differently:
`SimpleKeyGenerator`/`ComplexKeyGenerator` give a silent empty result on a
prefix-pruned query, while `CustomKeyGenerator` throws outright — I confirmed
the length check in `BaseHoodieTableFileIndex#getPartitionColumnValues` is
guarded on `shouldListLazily`, which is the default, so it's a hard
`HoodieException` on every read rather than wrong rows.
Agreed a partition-field-count check at table creation catches it: the count
is a table-level property and doesn't depend on how the key generator
decomposes it into sub-key-generators. Noted that in the issue's suggested fix.
--
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]
