hudi-agent commented on code in PR #19129:
URL: https://github.com/apache/hudi/pull/19129#discussion_r3503160737
##########
hudi-hadoop-common/src/main/java/org/apache/hudi/io/storage/hadoop/HoodieAvroHFileWriter.java:
##########
@@ -128,6 +130,11 @@ public boolean canWrite() {
return !isWrapperFileSystem || wrapperFs.get().getBytesWritten(file) <
maxFileSize;
}
+ @Override
+ public void addFooterMetadata(Map<String, String> footerMetadata) {
+ footerMetadata.forEach((key, value) -> writer.appendFileInfo(new
String(key.getBytes(), StandardCharsets.UTF_8), getUTF8Bytes(value)));
Review Comment:
🤖 Following up on the earlier UTF-8 thread: `key.getBytes()` (no charset
arg) encodes using the platform default charset, so `new String(key.getBytes(),
UTF_8)` is a no-op on UTF-8 JVMs but can actually mangle a non-ASCII key on
others. Since the file-info serializer already UTF-8-encodes the key
(`getUTF8Bytes(e.getKey())` in HFileFileInfoBlock), could you just pass `key`
directly here?
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
##########
hudi-hadoop-common/src/main/java/org/apache/hudi/io/storage/hadoop/HoodieAvroHFileWriter.java:
##########
@@ -128,6 +130,11 @@ public boolean canWrite() {
return !isWrapperFileSystem || wrapperFs.get().getBytesWritten(file) <
maxFileSize;
}
+ @Override
Review Comment:
🤖 nit: `new String(key.getBytes(), StandardCharsets.UTF_8)` is a puzzling
round-trip since `key` is already a `String` — if `appendFileInfo` accepts a
`String`, just passing `key` directly would be cleaner and avoid the implicit
platform-default-charset detour.
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
##########
hudi-common/src/main/java/org/apache/hudi/common/util/HFileUtils.java:
##########
@@ -101,7 +107,25 @@ public List<GenericRecord> readAvroRecords(HoodieStorage
storage, StoragePath fi
@Override
public Map<String, String> readFooter(HoodieStorage storage, boolean
required, StoragePath filePath, String... footerNames) {
- throw new UnsupportedOperationException("HFileUtils does not support
readFooter");
+ Map<String, String> footerVals = new HashMap<>();
+ try (HFileReader reader = HFileReaderFactory.builder()
+ .withStorage(storage)
+ .withPath(filePath)
+ .build()
+ .createHFileReader()) {
+ for (String footerName : footerNames) {
+ Option<byte[]> footerValue = reader.getMetaInfo(new
UTF8StringKey(footerName));
+ if (footerValue.isPresent()) {
+ footerVals.put(footerName, fromUTF8Bytes(footerValue.get()));
+ } else if (required) {
+ throw new MetadataNotFoundException(
Review Comment:
🤖 nit: the error message says "Could not find index in HFile footer" but
"index" reads like a data/Bloom index rather than a metadata key — something
like "Could not find metadata key '%s' in HFile footer" would be clearer about
what was missing.
<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]
