[PR] [core] Skip rewrite when delete row count is 0 with dv [incubator-paimon]

["Zouxxyy (via GitHub)" <gi...@apache.org>]

Zouxxyy opened a new pull request, #3070:
URL: https://github.com/apache/incubator-paimon/pull/3070

   <!-- Please specify the module before the PR name: [core] ... or [flink] ... -->
   
   ### Purpose
   
   When upgrade is possible and delete row count is 0, we can avoid rewrite
   
   ### Tests
   
   <!-- List UT and IT cases to verify this change -->
   
   ### API and Format
   
   <!-- Does this change affect API or storage format -->
   
   ### Documentation
   
   <!-- Does this change introduce a new feature -->
   


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [core] Skip rewrite when delete row count is 0 with dv [incubator-paimon]

["JingsongLi (via GitHub)" <gi...@apache.org>]

JingsongLi commented on code in PR #3070:
URL: https://github.com/apache/incubator-paimon/pull/3070#discussion_r1533356340


##########
paimon-core/src/main/java/org/apache/paimon/mergetree/compact/LookupMergeTreeCompactRewriter.java:
##########
@@ -101,10 +104,11 @@ protected UpgradeStrategy upgradeChangelog(int outputLevel, DataFileMeta file) {
             return NO_CHANGELOG;
         }
 
-        // TODO In deletionVector mode, since drop delete is required, rewrite is always required.
-        // TODO wait https://github.com/apache/incubator-paimon/pull/2962
-        // TODO but should be careful to not be deleted by DeletionVectorsMaintainer!
-        if (dvMaintainer != null) {
+        // In deletionVector mode, since drop delete is required, when delete row count > 0 rewrite
+        // is required.
+        if (dvMaintainer != null
+                && file.deleteRowCount().isPresent()

Review Comment:
   file.deleteRowCount().map(cnt -> cnt > 0).orElse(true)



##########
paimon-core/src/main/java/org/apache/paimon/mergetree/compact/LookupMergeTreeCompactRewriter.java:
##########
@@ -85,6 +85,9 @@ public LookupMergeTreeCompactRewriter(
     @Override
     protected void notifyCompactBefore(List<DataFileMeta> files) {

Review Comment:
   notifyCompactBefore -> notifyRewriteCompactBefore
   
   And:
   ```
   if (rewriteCompactFile) {
      notifyCompactBefore(before);
   }
   ```
   
   And we can rename `ChangelogMergeTreeRewriter.rewriteChangelogCompaction` to `ChangelogMergeTreeRewriter.rewriteOrProduceChangelog`.



##########
paimon-core/src/main/java/org/apache/paimon/mergetree/compact/LookupMergeTreeCompactRewriter.java:
##########
@@ -101,10 +104,11 @@ protected UpgradeStrategy upgradeChangelog(int outputLevel, DataFileMeta file) {
             return NO_CHANGELOG;

Review Comment:
   NO_CHANGELOG -> NO_CHANGELOG_NO_REWRITE



##########
paimon-core/src/main/java/org/apache/paimon/mergetree/compact/MergeTreeCompactRewriter.java:
##########
@@ -98,5 +98,10 @@ protected <T> RecordReader<T> readerForMergeTree(
                 mergeSorter);
     }
 
+    /**
+     * Hook to handle the files before compact in rewriting stage. Note: In addition to normal

Review Comment:
   remove documentation, we already change the name.



-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [core] Skip rewrite when delete row count is 0 with dv [incubator-paimon]

["JingsongLi (via GitHub)" <gi...@apache.org>]

JingsongLi merged PR #3070:
URL: https://github.com/apache/incubator-paimon/pull/3070


-- 
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: issues-unsubscribe@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org