voonhous commented on code in PR #19869:
URL: https://github.com/apache/hudi/pull/19869#discussion_r3966469834
##########
hudi-common/src/main/java/org/apache/hudi/metadata/BaseFileRecordParsingUtils.java:
##########
@@ -67,7 +69,9 @@ public static Iterator<HoodieRecord>
generateRLIMetadataHoodieRecordsForBaseFile
boolean isPartitionedRLI) {
String partition = writeStat.getPartitionPath();
String latestFileName = FSUtils.getFileNameFromPath(writeStat.getPath());
- String fileId = FSUtils.getFileId(latestFileName);
+ // a file written outside Hudi keeps its own name, which may contain
underscores, so the file id is parsed from the marker
+ String fileId =
FileNameParser.parseBaseFile(latestFileName).map(FileNameParser.BaseFileName::getFileId)
Review Comment:
+1
**major:** Confirmed as a default-config failure. `HoodieWriteConfig:3107`
defaults `_hoodie.writes.fileid.encoding` to UUID and the external fileId is
the file name (`BaseFileRecordParsingUtils:92`), so
`HoodieMetadataPayload:704-709` throws `Invalid UUID or index` on the first
commit of any keyless table; the functional test passes only because it pins
`withWritesFileIdEncoding(1)` at :196. The `_hoodie.` key is never persisted to
`hoodie.properties`, and XTable's `HudiConversionTarget` does not set it (nor
enable RLI), so whoever enables RLI on such a table, XTable or a later indexer,
hits the throw unless they know the internal key. HUDI-6607 (#9311) added
encoding 1 for exactly this shape, and `HoodieRecordIndexInfo` stores the
encoding per record, so readers are unaffected. Could it be derived as
`generateRecordKeys ? RECORD_INDEX_FIELD_FILEID_ENCODING_RAW_STRING :
getWritesFileIdEncoding()` at `BaseRecordIndexer:148` and `:266`, with the
functional test also run on the defa
ult config?
##########
hudi-common/src/main/java/org/apache/hudi/metadata/BaseFileRecordParsingUtils.java:
##########
@@ -67,7 +69,9 @@ public static Iterator<HoodieRecord>
generateRLIMetadataHoodieRecordsForBaseFile
boolean isPartitionedRLI) {
String partition = writeStat.getPartitionPath();
String latestFileName = FSUtils.getFileNameFromPath(writeStat.getPath());
- String fileId = FSUtils.getFileId(latestFileName);
+ // a file written outside Hudi keeps its own name, which may contain
underscores, so the file id is parsed from the marker
+ String fileId =
FileNameParser.parseBaseFile(latestFileName).map(FileNameParser.BaseFileName::getFileId)
Review Comment:
One more data point on the encoding requirement: XTable's
`HudiConversionTarget` does not set `_hoodie.writes.fileid.encoding` (it does
not enable the record index either; it pins table version 6 and only turns on
column stats for unpartitioned tables). So whoever enables RLI on such a table,
XTable or a later indexer, hits the default-config throw unless they know about
the internal key, which is one more reason to derive the encoding rather than
document it.
(Edited: the description has since been reworded and no longer says XTable
sets it; the first version of this comment quoted that line.)
--
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]
