xushiyan commented on a change in pull request #4452:
URL: https://github.com/apache/hudi/pull/4452#discussion_r775681342
##########
File path:
hudi-common/src/main/java/org/apache/hudi/common/table/timeline/HoodieTimeline.java
##########
@@ -281,6 +290,21 @@
*/
Option<byte[]> getInstantDetails(HoodieInstant instant);
+ /**
+ * Get WriteOperationType for instant.
+ */
+ Option<WriteOperationType> getWriteOperationType(HoodieInstant instant);
Review comment:
It looks weird to have an API like this at HoodieTimeline level; it's
done via org.apache.hudi.common.model.HoodieCommitMetadata#getOperationType and
the responsibility shouldn't fall on HoodieTimeline. At this stage, can we keep
this internal to `isDeletePartitionType()` instead of making an API?
##########
File path:
hudi-common/src/main/java/org/apache/hudi/common/table/timeline/HoodieDefaultTimeline.java
##########
@@ -100,6 +102,12 @@ public HoodieTimeline filterCompletedInstants() {
return new
HoodieDefaultTimeline(instants.stream().filter(HoodieInstant::isCompleted),
details);
}
+ @Override
+ public HoodieTimeline filterCompletedExcludeDeletePartitionInstants() {
Review comment:
Making the API name more explicit helps explain the use case.
```suggestion
public HoodieTimeline filterCompletedInstantsWithSchema() {
```
##########
File path:
hudi-common/src/main/java/org/apache/hudi/common/table/timeline/HoodieDefaultTimeline.java
##########
@@ -351,6 +359,28 @@ public boolean isBeforeTimelineStarts(String instant) {
return details.apply(instant);
}
+ @Override
+ public Option<WriteOperationType> getWriteOperationType(HoodieInstant
instant) {
+ try {
+ HoodieCommitMetadata commitMetadata = HoodieCommitMetadata
+ .fromBytes(getInstantDetails(instant).get(),
HoodieCommitMetadata.class);
+ return Option.of(commitMetadata.getOperationType());
+ } catch (Exception e) {
+ return Option.empty();
+ }
+ }
+
+ @Override
+ public boolean isDeletePartitionType(HoodieInstant instant) {
+ Option<WriteOperationType> operationType = getWriteOperationType(instant);
+ return operationType.isPresent() &&
WriteOperationType.DELETE_PARTITION.equals(operationType.get());
+ }
+
+ @Override
+ public boolean isNotDeletePartitionType(HoodieInstant instant) {
Review comment:
this API looks redundant; you can simply negate the filter condtion?
--
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]
