danny0405 commented on code in PR #19706:
URL: https://github.com/apache/hudi/pull/19706#discussion_r3849505641
##########
hudi-common/src/main/java/org/apache/hudi/common/table/read/HoodieFileGroupReader.java:
##########
@@ -283,7 +283,7 @@ private ClosableIterator<T>
makeBootstrapBaseFileIterator(HoodieBaseFile baseFil
}
PartitionPathParser partitionPathParser = new PartitionPathParser();
Object[] partitionValues =
partitionPathParser.getPartitionFieldVals(partitionPathFields,
inputSplit.getPartitionPath(),
- readerContext.getSchemaHandler().getTableSchema());
+ readerContext.getSchemaHandler().getTableSchema(),
metaClient.getTableConfig().getSlashSeparatedDatePartitioning());
Review Comment:
Could we cover this config propagation through the
bootstrap/file-group-reader path (or the nearest existing reader test)? Every
new test calls the four-argument parser overload directly, so the suite would
still pass if this call accidentally used the legacy overload and the reported
bootstrap regression returned. Since this line is what makes the parser fix
reachable in production, a caller-level regression test would protect the fix
end to end.
##########
hudi-common/src/main/java/org/apache/hudi/common/table/PartitionPathParser.java:
##########
@@ -43,18 +43,37 @@ public class PartitionPathParser {
public Object[] getPartitionFieldVals(Option<String[]> partitionFields,
String partitionPath,
HoodieSchema writerSchema) {
+ return getPartitionFieldVals(partitionFields, partitionPath, writerSchema,
false);
+ }
+
+ /**
+ * @param slashSeparatedDatePartitioning whether the table was written with
+ * {@code hoodie.datasource.write.slash.separated.date.partitioning},
in which case a
+ * partition value spans several path segments rather than one.
+ */
+ public Object[] getPartitionFieldVals(Option<String[]> partitionFields,
+ String partitionPath,
+ HoodieSchema writerSchema,
+ boolean
slashSeparatedDatePartitioning) {
if (!partitionFields.isPresent()) {
return new Object[0];
}
- return getPartitionValues(partitionFields.get(), partitionPath,
writerSchema);
+ return getPartitionValues(partitionFields.get(), partitionPath,
writerSchema, slashSeparatedDatePartitioning);
}
private static Object[] getPartitionValues(String[] partitionFields,
String partitionPath,
- HoodieSchema schema) {
+ HoodieSchema schema,
+ boolean
slashSeparatedDatePartitioning) {
String[] parts = partitionPath.split("/");
int pathSegment = 0;
boolean hasDateField = false;
+ // NOTE: The writer only slash-separates a table partitioned by a single
column -- see the guard
+ // in [[KeyGenUtils#getRecordPartitionPath]] -- so that is the only
shape whose value is
+ // known to span several segments here. Multi-field slash
partitioning produces a layout
+ // that cannot be lined up with the partition columns at all; that
is tracked in HUDI
+ // issue #19666 and deliberately left alone
+ boolean valueSpansSegments = slashSeparatedDatePartitioning &&
partitionFields.length == 1;
Review Comment:
Could we phrase the comment above as the shape this parser intentionally
supports, rather than saying writers only produce single-column slash layouts?
Legacy `CustomKeyGenerator` tables did slash-separate multiple fields, and
current `master` still documents a HoodieStreamer first-write validation bypass
(see #19666/#19648). This guard is sensible because that layout is ambiguous,
but the current wording may mislead future readers about data that can exist on
disk.
--
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]
