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