[GitHub] [incubator-paimon] JingsongLi commented on a diff in pull request #1434: [flink] integrate DELETE filter pushdown for flink-1.17

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

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


##########
paimon-flink/paimon-flink-common/src/main/java/org/apache/paimon/flink/sink/FlinkTableSink.java:
##########
@@ -112,17 +132,95 @@ public RowLevelUpdateInfo applyRowLevelUpdate(
     @Override
     public RowLevelDeleteInfo applyRowLevelDelete(
             @Nullable RowLevelModificationScanContext rowLevelModificationScanContext) {
+        if (supportDelete()) {

Review Comment:
   `void checkDeletable`?



##########
paimon-flink/paimon-flink-common/src/main/java/org/apache/paimon/flink/sink/FlinkTableSink.java:
##########
@@ -112,17 +132,95 @@ public RowLevelUpdateInfo applyRowLevelUpdate(
     @Override
     public RowLevelDeleteInfo applyRowLevelDelete(
             @Nullable RowLevelModificationScanContext rowLevelModificationScanContext) {
+        if (supportDelete()) {
+            return new RowLevelDeleteInfo() {};
+        }
+        // actually, this will not be executed now.
+        throw new UnsupportedOperationException(
+                String.format("table '%s' cannot support delete.", table.getClass().getName()));
+    }
+
+    /*
+    supported filters:
+    1. where primary key = x
+    2. where primary key = x or key = y
+    3. where primary key in (x, y, z)
+    4. where partition key = x
+    */
+    @Override
+    public boolean applyDeleteFilters(List<ResolvedExpression> list) {
+        if (supportDelete()) {
+            if (list.size() == 0) {
+                return false;
+            }
+
+            predicates = new ArrayList<>();

Review Comment:
   local variable, don't assign to class field when return false.



##########
paimon-flink/paimon-flink-common/src/main/java/org/apache/paimon/flink/sink/FlinkTableSink.java:
##########
@@ -135,4 +233,50 @@ public RowLevelDeleteInfo applyRowLevelDelete(
                             table.getClass().getName()));
         }
     }
+
+    private boolean shouldPushdownDeleteFilter(Predicate predicate, int predicateSize) {

Review Comment:
   `canPushdownDeleteFilter`



##########
paimon-flink/paimon-flink-common/src/main/java/org/apache/paimon/flink/sink/FlinkTableSink.java:
##########
@@ -135,4 +233,50 @@ public RowLevelDeleteInfo applyRowLevelDelete(
                             table.getClass().getName()));
         }
     }
+
+    private boolean shouldPushdownDeleteFilter(Predicate predicate, int predicateSize) {

Review Comment:
   Use `FunctionVisitor` to visit predicate?



-- 
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