From 2ced69439ff2067fe6ff6cad5dbb1fdf4bd13282 Mon Sep 17 00:00:00 2001 From: Jingsong Lee Date: Mon, 1 Apr 2024 18:35:36 +0800 Subject: [PATCH] [core] MergeFunction.getResult return not null (#3136) --- .../mergetree/compact/DeduplicateMergeFunction.java | 1 - .../paimon/mergetree/compact/FirstRowMergeFunction.java | 1 - .../mergetree/compact/FistRowMergeFunctionWrapper.java | 4 ---- .../compact/FullChangelogMergeFunctionWrapper.java | 4 ++-- .../apache/paimon/mergetree/compact/MergeFunction.java | 5 +---- .../mergetree/compact/PartialUpdateMergeFunction.java | 8 -------- .../compact/aggregate/AggregateMergeFunction.java | 1 - .../paimon/operation/KeyValueFileStoreReadTest.java | 5 ----- 8 files changed, 3 insertions(+), 26 deletions(-) diff --git a/paimon-core/src/main/java/org/apache/paimon/mergetree/compact/DeduplicateMergeFunction.java b/paimon-core/src/main/java/org/apache/paimon/mergetree/compact/DeduplicateMergeFunction.java index 9c1cf0f5e60c..3d6b341e3ec8 100644 --- a/paimon-core/src/main/java/org/apache/paimon/mergetree/compact/DeduplicateMergeFunction.java +++ b/paimon-core/src/main/java/org/apache/paimon/mergetree/compact/DeduplicateMergeFunction.java @@ -41,7 +41,6 @@ public void add(KeyValue kv) { } @Override - @Nullable public KeyValue getResult() { return latestKv; } diff --git a/paimon-core/src/main/java/org/apache/paimon/mergetree/compact/FirstRowMergeFunction.java b/paimon-core/src/main/java/org/apache/paimon/mergetree/compact/FirstRowMergeFunction.java index b955e0fea387..a00e09f0221d 100644 --- a/paimon-core/src/main/java/org/apache/paimon/mergetree/compact/FirstRowMergeFunction.java +++ b/paimon-core/src/main/java/org/apache/paimon/mergetree/compact/FirstRowMergeFunction.java @@ -56,7 +56,6 @@ public void add(KeyValue kv) { } } - @Nullable @Override public KeyValue getResult() { return first; diff --git a/paimon-core/src/main/java/org/apache/paimon/mergetree/compact/FistRowMergeFunctionWrapper.java b/paimon-core/src/main/java/org/apache/paimon/mergetree/compact/FistRowMergeFunctionWrapper.java index fe96658c9675..a0533082662d 100644 --- a/paimon-core/src/main/java/org/apache/paimon/mergetree/compact/FistRowMergeFunctionWrapper.java +++ b/paimon-core/src/main/java/org/apache/paimon/mergetree/compact/FistRowMergeFunctionWrapper.java @@ -56,10 +56,6 @@ public void add(KeyValue kv) { public ChangelogResult getResult() { reusedResult.reset(); KeyValue result = mergeFunction.getResult(); - if (result == null) { - return reusedResult; - } - if (contains.test(result.key())) { // empty return reusedResult; diff --git a/paimon-core/src/main/java/org/apache/paimon/mergetree/compact/FullChangelogMergeFunctionWrapper.java b/paimon-core/src/main/java/org/apache/paimon/mergetree/compact/FullChangelogMergeFunctionWrapper.java index fe1bf0dc8a75..9d5dab1ae357 100644 --- a/paimon-core/src/main/java/org/apache/paimon/mergetree/compact/FullChangelogMergeFunctionWrapper.java +++ b/paimon-core/src/main/java/org/apache/paimon/mergetree/compact/FullChangelogMergeFunctionWrapper.java @@ -100,11 +100,11 @@ public ChangelogResult getResult() { if (isInitialized) { KeyValue merged = mergeFunction.getResult(); if (topLevelKv == null) { - if (merged != null && merged.isAdd()) { + if (merged.isAdd()) { reusedResult.addChangelog(replace(reusedAfter, RowKind.INSERT, merged)); } } else { - if (merged == null || !merged.isAdd()) { + if (!merged.isAdd()) { reusedResult.addChangelog(replace(reusedBefore, RowKind.DELETE, topLevelKv)); } else if (!changelogRowDeduplicate || !valueEqualiser.equals(topLevelKv.value(), merged.value())) { diff --git a/paimon-core/src/main/java/org/apache/paimon/mergetree/compact/MergeFunction.java b/paimon-core/src/main/java/org/apache/paimon/mergetree/compact/MergeFunction.java index 2141cd64e358..dc2c9e4580c2 100644 --- a/paimon-core/src/main/java/org/apache/paimon/mergetree/compact/MergeFunction.java +++ b/paimon-core/src/main/java/org/apache/paimon/mergetree/compact/MergeFunction.java @@ -20,8 +20,6 @@ import org.apache.paimon.KeyValue; -import javax.annotation.Nullable; - /** * Merge function to merge multiple {@link KeyValue}s. * @@ -44,7 +42,6 @@ public interface MergeFunction { /** Add the given {@link KeyValue} to the merge function. */ void add(KeyValue kv); - /** Get current merged value. Return null if this merged result should be skipped. */ - @Nullable + /** Get current merged value. */ T getResult(); } diff --git a/paimon-core/src/main/java/org/apache/paimon/mergetree/compact/PartialUpdateMergeFunction.java b/paimon-core/src/main/java/org/apache/paimon/mergetree/compact/PartialUpdateMergeFunction.java index c1fc9293fd8f..9ed188709631 100644 --- a/paimon-core/src/main/java/org/apache/paimon/mergetree/compact/PartialUpdateMergeFunction.java +++ b/paimon-core/src/main/java/org/apache/paimon/mergetree/compact/PartialUpdateMergeFunction.java @@ -73,7 +73,6 @@ public class PartialUpdateMergeFunction implements MergeFunction { private InternalRow currentKey; private long latestSequenceNumber; - private boolean isEmpty; private GenericRow row; private KeyValue reused; @@ -93,7 +92,6 @@ public void reset() { this.currentKey = null; this.row = new GenericRow(getters.length); fieldAggregators.values().forEach(FieldAggregator::reset); - this.isEmpty = true; } @Override @@ -119,7 +117,6 @@ public void add(KeyValue kv) { } latestSequenceNumber = kv.sequenceNumber(); - isEmpty = false; if (fieldSequences.isEmpty()) { updateNonNullFields(kv); } else { @@ -203,12 +200,7 @@ private void retractWithSequenceGroup(KeyValue kv) { } @Override - @Nullable public KeyValue getResult() { - if (isEmpty) { - return null; - } - if (reused == null) { reused = new KeyValue(); } diff --git a/paimon-core/src/main/java/org/apache/paimon/mergetree/compact/aggregate/AggregateMergeFunction.java b/paimon-core/src/main/java/org/apache/paimon/mergetree/compact/aggregate/AggregateMergeFunction.java index fd874e903f5c..7f799e4dc865 100644 --- a/paimon-core/src/main/java/org/apache/paimon/mergetree/compact/aggregate/AggregateMergeFunction.java +++ b/paimon-core/src/main/java/org/apache/paimon/mergetree/compact/aggregate/AggregateMergeFunction.java @@ -80,7 +80,6 @@ public void add(KeyValue kv) { } } - @Nullable @Override public KeyValue getResult() { checkNotNull( diff --git a/paimon-core/src/test/java/org/apache/paimon/operation/KeyValueFileStoreReadTest.java b/paimon-core/src/test/java/org/apache/paimon/operation/KeyValueFileStoreReadTest.java index 07bf705e2bba..c464825566ce 100644 --- a/paimon-core/src/test/java/org/apache/paimon/operation/KeyValueFileStoreReadTest.java +++ b/paimon-core/src/test/java/org/apache/paimon/operation/KeyValueFileStoreReadTest.java @@ -319,12 +319,7 @@ public void add(KeyValue kv) { } @Override - @Nullable public KeyValue getResult() { - if (total == 0) { - return null; - } - if (reused == null) { reused = new KeyValue(); }