SEPURI-SAI-KRISHNA commented on code in PR #19703:
URL: https://github.com/apache/hudi/pull/19703#discussion_r3835813481
##########
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)
+ } else {
+ encodedPartitionValue
+ }
}.mkString("/")
}
+ /**
+ * Turns a `yyyy-MM-dd` formatted partition value into the `yyyy/MM/dd`
directory structure
+ * requested by `hoodie.datasource.write.slash.separated.date.partitioning`,
mirroring the
+ * substitution the write path performs in
`KeyGenUtils#getRecordPartitionPath`.
+ *
+ * A value with a leading dash is returned as-is: substituting would make
the partition path start
+ * with "/", and an absolute relative-partition-path is resolved
inconsistently -- the writer and
+ * the file-system view disagree on where such a partition lives. Such a
value is not a date to
+ * begin with, so nothing is lost by not slashing it.
+ */
+ private def slashSeparateDateValue(partitionValue: String): String = {
+ if (partitionValue.startsWith("-")) partitionValue else
partitionValue.replace('-', '/')
+ }
Review Comment:
Intentional, and you are right that it diverges from the writer:
`KeyGenUtils#getRecordPartitionPath` (L260-261) and `#getPartitionPath`
(L294-295) both substitute unconditionally, so `-01-05` is written as `/01/05`.
The reason I do not reproduce that here is that `/01/05` is not a directory
the writer reliably created. As a relative partition path with a leading
separator it resolves two different ways in our own code:
`FSUtils#constructAbsolutePath(String, String)` strips the leading `/` and
yields `<basePath>/01/05`, while `new StoragePath(basePath, "/01/05")` treats
the child as absolute and drops the base entirely. So there is no single
"directory the writer created" for the DDL to name — matching verbatim would
just be picking one of those two readings and hard-coding it.
The row-writer half of this is being fixed in #19648, which adds the same
leading-dash guard to `PartitionPathFormatterBase#combine`; the remaining
Avro/row-writer divergence is tracked in #19669. If that decision lands the
other way, the guard here and the one in the formatter should be lifted
together.
Also worth noting the input is unreachable for the feature's documented use:
slash-separated date partitioning targets `yyyy-MM-dd` values, which never lead
with a dash. Happy to make the DDL bug-compatible with the writer instead if a
committer prefers that.
--
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]
