hudi-agent commented on code in PR #19703:
URL: https://github.com/apache/hudi/pull/19703#discussion_r3835449988
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/spark/sql/hudi/HoodieSqlCommonUtils.scala:
##########
@@ -406,24 +406,55 @@ object HoodieSqlCommonUtils extends SparkAdapterSupport {
private def makePartitionPath(partitionFields: Seq[String],
normalizedSpecs: Map[String, String],
enableEncodeUrl: Boolean,
- enableHiveStylePartitioning: Boolean): String
= {
+ enableHiveStylePartitioning: Boolean,
+ slashSeparatedDatePartitioning: Boolean):
String = {
+ // NOTE: Slash-separated date partitioning only kicks in for a table
partitioned by a single
+ // (date) column, mirroring the guard in
[[KeyGenUtils#getRecordPartitionPath]] that drives
+ // the write path -- these commands have to name the very same
directory the writer created.
+ // Hive-style partitioning is excluded because the config documents
the two as mutually
+ // exclusive, and the write paths do not agree on what the
combination should produce
+ // (tracked in HUDI issue #19669), so there is no single directory
to name here
+ val slashSeparateDates =
+ slashSeparatedDatePartitioning && !enableHiveStylePartitioning &&
partitionFields.length == 1
partitionFields.map { partitionColumn =>
val encodedPartitionValue = if (enableEncodeUrl) {
PartitionPathEncodeUtils.escapePathName(normalizedSpecs(partitionColumn))
} else {
normalizedSpecs(partitionColumn)
}
- if (enableHiveStylePartitioning)
s"$partitionColumn=$encodedPartitionValue" else encodedPartitionValue
+ if (enableHiveStylePartitioning) {
+ s"$partitionColumn=$encodedPartitionValue"
+ } else if (slashSeparateDates) {
+ slashSeparateDateValue(encodedPartitionValue)
Review Comment:
🤖 This encodes first and then slash-substitutes, which matches
`KeyGenUtils#getRecordPartitionPath`. But the Spark builtin key generators go
through `PartitionPathFormatterBase#combine`, and its single-field slash branch
does `replace('-','/')` on the raw value and skips URL encoding entirely (lines
64-66). So for a URL-encode-enabled table whose value actually needs escaping,
the writer would name an un-encoded slashed dir while this DDL names an encoded
one — the same silent no-op this PR is fixing. Dates won't trigger it, but is
it worth confirming which write path Spark tables actually use here?
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/spark/sql/hudi/HoodieSqlCommonUtils.scala:
##########
@@ -406,24 +406,55 @@ object HoodieSqlCommonUtils extends SparkAdapterSupport {
private def makePartitionPath(partitionFields: Seq[String],
normalizedSpecs: Map[String, String],
enableEncodeUrl: Boolean,
- enableHiveStylePartitioning: Boolean): String
= {
+ enableHiveStylePartitioning: Boolean,
+ slashSeparatedDatePartitioning: Boolean):
String = {
+ // NOTE: Slash-separated date partitioning only kicks in for a table
partitioned by a single
+ // (date) column, mirroring the guard in
[[KeyGenUtils#getRecordPartitionPath]] that drives
+ // the write path -- these commands have to name the very same
directory the writer created.
+ // Hive-style partitioning is excluded because the config documents
the two as mutually
+ // exclusive, and the write paths do not agree on what the
combination should produce
+ // (tracked in HUDI issue #19669), so there is no single directory
to name here
+ val slashSeparateDates =
Review Comment:
🤖 nit: `slashSeparateDates` reads as a verb phrase rather than a boolean
predicate — could you rename it to something like `useSlashSeparatedDates` so
it reads naturally in the `else if` branch below?
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
--
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]
