shangxinli commented on code in PR #20333:
URL: https://github.com/apache/hudi/pull/20333#discussion_r4272269024
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metadata/index/files/FilesIndexer.java:
##########
@@ -189,6 +192,9 @@ private static List<HoodieRecord>
convertMetadataToFilesPartitionRecords(HoodieC
// of the sizes as reported after every write, since
file-sizes are
// monotonically increasing (ie file-size never goes down,
unless deleted)
updatedFilesToSizesMapping.merge(fileName,
stat.getFileSizeInBytes(), Math::max);
+ if (stat.getSourceBasePath() != null) {
+ fileToSourceBasePath.put(fileName, stat.getSourceBasePath());
Review Comment:
Good catch, and it is worse than that - sourceBasePath already means the
bootstrap source table root in this codebase
(config.getBootstrapSourceBasePath()). Renamed to sourcePartitionPath and
documented on both sides as the absolute directory the file lives in, resolved
against the file name rather than the write stat relative path.
##########
hudi-common/src/main/java/org/apache/hudi/common/model/HoodieWriteStat.java:
##########
@@ -61,6 +61,14 @@ public class HoodieWriteStat extends HoodieReadStats {
@EqualsAndHashCode.Include
private String path;
+ /**
+ * Base path the file lives under, when the file is not under the table base
path. Set only for
+ * files registered into the table without being rewritten (REGISTER_ONLY
bootstrap); null
+ * otherwise, in which case {@link #path} is relative to the table base path
as usual.
+ */
+ @Nullable
+ private String sourceBasePath;
Review Comment:
Confirmed - JsonUtils disables FAIL_ON_UNKNOWN_PROPERTIES so it was being
dropped silently. Added the field to HoodieWriteStat in
HoodieCommitMetadata.avsc; the replace and compaction schemas reference that
record by name so they pick it up too.
##########
hudi-common/src/main/java/org/apache/hudi/metadata/HoodieMetadataPayload.java:
##########
@@ -505,7 +518,11 @@ public List<StoragePathInfo> getFileList(HoodieStorage
storage, StoragePath part
.map(e -> {
// NOTE: Since we know that the Metadata Table's Payload is simply a
file-name we're
// creating Hadoop's Path using more performant unsafe variant
- return new StoragePathInfo(new StoragePath(partitionPath,
e.getKey()), e.getValue().getSize(),
+ // A registered file lives outside the table, so it resolves against
the base path it was
+ // registered with rather than against this table's partition path.
+ StoragePath basePath = e.getValue().getSourceBasePath() == null
Review Comment:
Right, and that is exactly what the read-path PR in this stack fixes - it
keeps the listing keyed by relative partition path instead of re-deriving each
file partition from its parent dir. Out of scope here since nothing sets the
field yet.
##########
hudi-common/src/main/java/org/apache/hudi/metadata/MetadataPartitionType.java:
##########
@@ -303,7 +303,8 @@ private static void
constructFilesMetadataPayload(HoodieMetadataPayload payload,
payload.filesystemMetadata = metadata;
payload.filesystemMetadata.keySet().forEach(k -> {
GenericRecord v = payload.filesystemMetadata.get(k);
- payload.filesystemMetadata.put(k, new HoodieMetadataFileInfo((Long)
v.get("size"), (Boolean) v.get("isDeleted")));
+ payload.filesystemMetadata.put(k, new HoodieMetadataFileInfo((Long)
v.get("size"), (Boolean) v.get("isDeleted"),
+ v.get("sourceBasePath") == null ? null :
v.get("sourceBasePath").toString()));
Review Comment:
You are right on both counts - I checked the bytecode and
GenericData.Record.get(String) throws AvroRuntimeException on an unknown field
in 1.11.4, so this would have blown up on any record written before the change.
Now goes through getNestedFieldValue, with a test that builds a record against
the pre-change schema.
--
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]
