Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/23613 )
Change subject: IMPALA-14092 Part2: Support querying of paimon data table via JNI ...................................................................... Patch Set 12: Code-Review+1 (8 comments) Code looks OK from FE side. Please double check codestyle and docs where ever it can helps with understanding the code. http://gerrit.cloudera.org:8080/#/c/23613/11//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/23613/11//COMMIT_MSG@35 PS11, Line 35: And PaimonJniScanner will pass the arrow offheap : record batch memory pointer to the BE backend. > Done??the batch size can be adjusted, will implmenent in the next revision. Done http://gerrit.cloudera.org:8080/#/c/23613/11/be/src/exec/paimon/paimon-jni-scan-node.h File be/src/exec/paimon/paimon-jni-scan-node.h: http://gerrit.cloudera.org:8080/#/c/23613/11/be/src/exec/paimon/paimon-jni-scan-node.h@51 PS11, Line 51: /// 2. Backend: Creates an PaimonJniScanner object on the Java heap. > yes, 1 PaimonJniScanner per scan fragment instance? Ack http://gerrit.cloudera.org:8080/#/c/23613/12/be/src/exec/paimon/paimon-jni-scan-node.cc File be/src/exec/paimon/paimon-jni-scan-node.cc: http://gerrit.cloudera.org:8080/#/c/23613/12/be/src/exec/paimon/paimon-jni-scan-node.cc@143 PS12, Line 143: DCHECK_NOTNULL(is_empty_batch); clang-tidy complain here. paimon-jni-scan-node.cc:143:20: warning: expression result unused [clang-diagnostic-unused-value] This should work. DCHECK(is_empty_batch != nullptr); http://gerrit.cloudera.org:8080/#/c/23613/12/fe/src/main/java/org/apache/impala/catalog/paimon/PaimonUtil.java File fe/src/main/java/org/apache/impala/catalog/paimon/PaimonUtil.java: http://gerrit.cloudera.org:8080/#/c/23613/12/fe/src/main/java/org/apache/impala/catalog/paimon/PaimonUtil.java@879 PS12, Line 879: /** nit: add a whitespace before. http://gerrit.cloudera.org:8080/#/c/23613/11/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/23613/11/fe/src/main/java/org/apache/impala/planner/PaimonScanNode.java@242 PS11, Line 242: numNodes_ = Math.max(totalNodes, 1); : numInstances_ = Math.max(totalInstances, 1); : } : : @Override : public void computeNodeResourceProfile(TQueryOptions queryOptions) { : // current batch size is from query options, so estimated bytes : > ?. current calculation is similiar with memoryEstimateForFetchingColumns in Ack http://gerrit.cloudera.org:8080/#/c/23613/12/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/23613/12/fe/src/main/java/org/apache/impala/planner/PaimonScanNode.java@86 PS12, Line 86: fieldIdMap fieldIdSet_ ? Please leave a comment what this Set is used for. http://gerrit.cloudera.org:8080/#/c/23613/12/fe/src/main/java/org/apache/impala/planner/paimon/PaimonSplit.java File fe/src/main/java/org/apache/impala/planner/paimon/PaimonSplit.java: http://gerrit.cloudera.org:8080/#/c/23613/12/fe/src/main/java/org/apache/impala/planner/paimon/PaimonSplit.java@28 PS12, Line 28: public class PaimonSplit implements Serializable { : : private final Split split_; : private final ArrayList<Predicate> predicates_; Please add class and field documentations here. http://gerrit.cloudera.org:8080/#/c/23613/11/fe/src/main/java/org/apache/impala/util/paimon/PaimonJniScanner.java File fe/src/main/java/org/apache/impala/util/paimon/PaimonJniScanner.java: http://gerrit.cloudera.org:8080/#/c/23613/11/fe/src/main/java/org/apache/impala/util/paimon/PaimonJniScanner.java@114 PS11, Line 114: // get mem limit : allocator_mem_limit_ = paimonJniScanParam > Done Done -- To view, visit http://gerrit.cloudera.org:8080/23613 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie679a89a8cc21d52b583422336b9f747bdf37384 Gerrit-Change-Number: 23613 Gerrit-PatchSet: 12 Gerrit-Owner: ji chen <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Reviewer: ji chen <[email protected]> Gerrit-Comment-Date: Tue, 02 Dec 2025 17:40:16 +0000 Gerrit-HasComments: Yes
