vinishjail97 commented on code in PR #835:
URL: https://github.com/apache/incubator-xtable/pull/835#discussion_r4161121797
##########
xtable-core/src/main/java/org/apache/xtable/hudi/BaseFileUpdatesExtractor.java:
##########
@@ -244,13 +243,32 @@ ReplaceMetadata convertDiff(
return ReplaceMetadata.of(partitionToReplacedFileIds, writeStatuses);
}
- private String getFileId(Path filePath) {
- String fileName = filePath.getName();
+ private String getFileId(InternalDataFile file) {
+ String fileName = new CachingPath(file.getPhysicalPath()).getName();
// if file was created by Hudi use original fileId, otherwise use the file
name as IDs
if (isFileCreatedByHudiWriter(fileName)) {
return FSUtils.getFileId(fileName);
}
- return fileName;
+ // Files under a file-group prefix keep the prefix as part of the fileId
so the prefix can be
+ // recovered when Hudi resolves the physical path of the externally
created file.
+ return file.getFileGroupPrefix().map(prefix -> prefix + "/" +
fileName).orElse(fileName);
+ }
+
+ /**
+ * Resolves the Hudi partition path for a file. A file-group prefix
directory (e.g. Paimon's
+ * {@code bucket-N}) is a file group within the partition (see Hudi PR
#17788), so it is stripped
+ * from the partition path.
+ */
+ private String truePartitionPath(Path tableBasePath, InternalDataFile file) {
Review Comment:
Renamed to `getHudiPartitionPath` in 0a1f510.
##########
xtable-api/src/main/java/org/apache/xtable/model/storage/InternalDataFile.java:
##########
@@ -52,4 +53,7 @@ public class InternalDataFile extends InternalFile {
@Builder.Default @NonNull List<ColumnStat> columnStats =
Collections.emptyList();
// last modified time in millis since epoch
long lastModified;
+ // directory between the partition path and the file name that groups files
within a partition,
+ // for example Paimon's bucket-N; empty when the file sits directly under
its partition
+ @Builder.Default @NonNull Optional<String> fileGroupPrefix =
Optional.empty();
Review Comment:
Renamed to `partitionSubdirectory` in 0a1f510. The Hudi-side layout marker
keeps the file-group prefix name, since that is the Hudi term
(apache/hudi#17788).
##########
xtable-core/src/main/java/org/apache/xtable/hudi/HudiConversionSource.java:
##########
@@ -163,19 +172,24 @@ public CommitsBacklog<HoodieInstant> getCommitsBacklog(
@Override
public boolean isIncrementalSyncSafeFrom(Instant instant) {
- return doesCommitExistsAsOfInstant(instant) &&
!isAffectedByCleanupProcess(instant);
+ HoodieInstant commitAtInstant = getCommitAtInstant(instant);
+ if (commitAtInstant == null) {
+ return false;
+ }
+ // On table version 8+ the checkpoint is a completion time, but the
cleaner retains commits by
+ // requested time, so check the cleaner against the requested time of the
synced commit.
+ Instant cleanerCheckInstant =
+ usesCompletionTimeOrdering()
+ ?
HudiInstantUtils.parseFromInstantTime(commitAtInstant.requestedTime())
+ : instant;
+ return !isAffectedByCleanupProcess(cleanerCheckInstant);
}
@Override
public String getCommitIdentifier(HoodieInstant commit) {
return commit.requestedTime();
}
- private boolean doesCommitExistsAsOfInstant(Instant instant) {
Review Comment:
Restored `doesCommitExistsAsOfInstant` in 0a1f510, and moved the
requested-time conversion for version 8+ into `isAffectedByCleanupProcess`.
##########
xtable-core/src/main/java/org/apache/xtable/hudi/HudiConversionTarget.java:
##########
@@ -599,18 +687,25 @@ private void markInstantsAsCleaned(
private void runArchiver(
HoodieJavaTable<?> table, HoodieWriteConfig config,
HoodieEngineContext engineContext) {
- // trigger archiver manually
+ // trigger archiver manually, selecting the archiver implementation that
matches the table's
+ // timeline layout (V1 for table version 6, V2/LSM for table version 9).
try {
- HoodieTimelineArchiver archiver = new TimelineArchiverV1(config,
table);
+ @SuppressWarnings({"unchecked", "rawtypes"})
Review Comment:
Removed the annotation in 0a1f510. `HoodieJavaTable<T>` has no bound on `T`,
but the archivers need `T extends HoodieAvroPayload`, so the raw cast stays as
on main.
--
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]
