hudi-agent commented on code in PR #19785:
URL: https://github.com/apache/hudi/pull/19785#discussion_r3978678575


##########
hudi-common/src/main/java/org/apache/hudi/common/model/HoodieFileGroup.java:
##########
@@ -160,7 +160,21 @@ private boolean isFileSliceCommitted(FileSlice slice) {
       return false;
     }
 
-    return timeline.containsOrBeforeTimelineStarts(slice.getBaseInstantTime());
+    if (timeline.containsOrBeforeTimelineStarts(slice.getBaseInstantTime())) {
+      return true;
+    }
+
+    // An uncommitted base instant with a base file keeps the original 
semantics: the slice stays
+    // hidden and is never revived by a committed log, otherwise the 
uncommitted base file would be
+    // read in full (there is no read-side trimming for it before table 
version 8).
+    if (slice.getBaseFile().isPresent()) {

Review Comment:
   🤖 This thread looks superseded by the force-push: the head commit (36c2997) 
no longer touches `isFileSliceCommitted` at all — the fix moved into 
`HoodieFileGroup.addLogFiles`, which seeds the first slice at the earliest 
*completed* log so the pending one attaches via the existing pending-log rule. 
The base-file flavour is now an explicit design choice rather than a guard: 
`testUncommittedBaseFileSliceStaysHiddenDespiteCommittedLogs` asserts that an 
uncommitted base at t1 plus committed logs t2/t3 stays hidden. Might be worth a 
line in the PR description noting that the Flink bulk_insert(base) + 
streaming(log) NBCC case is deliberately left to rollback rather than covered 
here.



-- 
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]

Reply via email to