amogh-jahagirdar commented on code in PR #13666:
URL: https://github.com/apache/iceberg/pull/13666#discussion_r2229844005
##########
core/src/main/java/org/apache/iceberg/deletes/BaseDVFileWriter.java:
##########
@@ -84,36 +84,39 @@ public void close() throws IOException {
CharSequenceSet referencedDataFiles = CharSequenceSet.empty();
List<DeleteFile> rewrittenDeleteFiles = Lists.newArrayList();
- PuffinWriter writer = newWriter();
-
- try (PuffinWriter closeableWriter = writer) {
- for (Deletes deletes : deletesByPath.values()) {
- String path = deletes.path();
- PositionDeleteIndex positions = deletes.positions();
- PositionDeleteIndex previousPositions =
loadPreviousDeletes.apply(path);
- if (previousPositions != null) {
- positions.merge(previousPositions);
- for (DeleteFile previousDeleteFile :
previousPositions.deleteFiles()) {
- // only DVs and file-scoped deletes can be discarded from the
table state
- if (ContentFileUtil.isFileScoped(previousDeleteFile)) {
- rewrittenDeleteFiles.add(previousDeleteFile);
+ // Only create PuffinWriter if there are deletes to write.
+ // This prevents creating empty Puffin files when no delete have
occurred.
Review Comment:
Minor: Could we structure it like the following just so it's a bit smaller
diff
```
if (result == null) {
List<DeleteFile> dvs = Lists.newArrayList();
CharSequenceSet referencedDataFiles = CharSequenceSet.empty();
List<DeleteFile> rewrittenDeleteFiles = Lists.newArrayList();
if (deletesByPath.isEmpty()) {
return new DeleteWriteResult(dvs, referencedDataFiles,
rewrittenDeleteFiles);
}
....exisitng logic
}
```
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
