SteNicholas commented on code in PR #286:
URL: https://github.com/apache/flink-table-store/pull/286#discussion_r962595078


##########
flink-table-store-core/src/main/java/org/apache/flink/table/store/file/mergetree/compact/PartialUpdateMergeFunction.java:
##########
@@ -45,9 +49,12 @@ public void reset() {
     }
 
     @Override
-    public void add(RowData value) {
+    public void add(KeyValue kv) {
+        checkArgument(
+                kv.valueKind() == RowKind.INSERT || kv.valueKind() == 
RowKind.UPDATE_AFTER,
+                "Partial update only accept insert only records.");

Review Comment:
   What does `insert only records` mean? IMO, the user may not understand this 
message.



##########
flink-table-store-core/src/main/java/org/apache/flink/table/store/file/mergetree/compact/MergeFunctionHelper.java:
##########
@@ -19,48 +19,46 @@
 package org.apache.flink.table.store.file.mergetree.compact;
 
 import org.apache.flink.table.data.RowData;
+import org.apache.flink.table.store.file.KeyValue;
 
 /** Helper functions for the interaction with {@link MergeFunction}. */
 public class MergeFunctionHelper {
 
     private final MergeFunction mergeFunction;
 
-    private RowData rowData;
+    private KeyValue firstKV;

Review Comment:
   What about naming to `initialKV`?



##########
flink-table-store-core/src/main/java/org/apache/flink/table/store/file/mergetree/compact/ValueCountMergeFunction.java:
##########
@@ -41,8 +43,10 @@ public void reset() {
     }
 
     @Override
-    public void add(RowData value) {
-        total += count(value);
+    public void add(KeyValue kv) {
+        checkArgument(
+                kv.valueKind() == RowKind.INSERT, "Value count only accept 
insert only records.");

Review Comment:
   Ditto.



-- 
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-unsubscr...@flink.apache.org

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

Reply via email to