Steve Carlin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/23793 )

Change subject: IMPALA-14092 Part3: Enable predicate pushdown for paimon table.
......................................................................


Patch Set 6:

(2 comments)

Started on this.  Looks good, haven't reviewed everything yet, but I haven't 
seen anything stopping me from giving this a +1.  I do want to understand it a 
bit more before I can do that though, but it's getting late here in California

http://gerrit.cloudera.org:8080/#/c/23793/6/fe/src/main/java/org/apache/impala/planner/PaimonScanNode.java
File fe/src/main/java/org/apache/impala/planner/PaimonScanNode.java:

http://gerrit.cloudera.org:8080/#/c/23793/6/fe/src/main/java/org/apache/impala/planner/PaimonScanNode.java@119
PS6, Line 119:     if (!PaimonUtil.canApplyPredicatePushDown(paimonApiTable_)) 
return false;
nit: can we simplify this to just
return PaimonUtil.canApplyPredicatePushDown(paimonApiTable_);
?


http://gerrit.cloudera.org:8080/#/c/23793/6/fe/src/main/java/org/apache/impala/planner/paimon/PredicateExtractor.java
File fe/src/main/java/org/apache/impala/planner/paimon/PredicateExtractor.java:

http://gerrit.cloudera.org:8080/#/c/23793/6/fe/src/main/java/org/apache/impala/planner/paimon/PredicateExtractor.java@40
PS6, Line 40: public class PredicateExtractor {
style nit:

I'm a big fan of having all members as "final".

I'm ok with this since it's a small class, but an alternative would be:

- name the class ExtractedPredicates
- make extractPaimonPredicates a static method that returns an 
ExtractedPredicates
- keep the Lists local within extractPaimonPredicates
- Have a private ExtractedPredicates constructor which takes the lists from the 
static method.



--
To view, visit http://gerrit.cloudera.org:8080/23793
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iee07fa35de8381121a20b2976d6424626d705483
Gerrit-Change-Number: 23793
Gerrit-PatchSet: 6
Gerrit-Owner: ji chen <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Riza Suminto <[email protected]>
Gerrit-Reviewer: Steve Carlin <[email protected]>
Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]>
Gerrit-Reviewer: ji chen <[email protected]>
Gerrit-Comment-Date: Tue, 27 Jan 2026 04:42:01 +0000
Gerrit-HasComments: Yes

Reply via email to