Anonymous Coward (606) has posted comments on this change. ( http://gerrit.cloudera.org:8080/16143 )
Change subject: IMPALA-9741: Support querying Iceberg table by impala ...................................................................... Patch Set 16: (4 comments) The code looks fine. All of my comments are optional. Thank you for the opportunity to review. http://gerrit.cloudera.org:8080/#/c/16143/16//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/16143/16//COMMIT_MSG@29 PS16, Line 29: We achieved this function by treating the iceberg table as normal Are there plans to support read of Iceberg table as a partitioned table? This will help with collocated joins. http://gerrit.cloudera.org:8080/#/c/16143/16/fe/src/main/java/org/apache/impala/analysis/ShowFilesStmt.java File fe/src/main/java/org/apache/impala/analysis/ShowFilesStmt.java: http://gerrit.cloudera.org:8080/#/c/16143/16/fe/src/main/java/org/apache/impala/analysis/ShowFilesStmt.java@80 PS16, Line 80: "SHOW FILES not applicable to a non hdfs table and non iceberg table: %s", The double negative is pretty hard to parse. How about: "SHOW FILES is applicable only to a HDFS table"? http://gerrit.cloudera.org:8080/#/c/16143/16/fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java File fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java: http://gerrit.cloudera.org:8080/#/c/16143/16/fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java@139 PS16, Line 139: if (!(predicate.getChild(0) instanceof SlotRef)) return false; Can the predicate be of the form: '10 = p1'? In that case, should there be symmetric code to get slotRef and literalExpr? http://gerrit.cloudera.org:8080/#/c/16143/16/fe/src/main/java/org/apache/impala/util/IcebergUtil.java File fe/src/main/java/org/apache/impala/util/IcebergUtil.java: http://gerrit.cloudera.org:8080/#/c/16143/16/fe/src/main/java/org/apache/impala/util/IcebergUtil.java@114 PS16, Line 114: if ("PARQUET".equalsIgnoreCase(format)) return TIcebergFileFormat.PARQUET; Rest of the code seems to support Iceberg ORC file format. This code does not seem to support it? -- To view, visit http://gerrit.cloudera.org:8080/16143 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I856cfee4f3397d1a89cf17650e8d4fbfe1f2b006 Gerrit-Change-Number: 16143 Gerrit-PatchSet: 16 Gerrit-Owner: wangsheng <[email protected]> Gerrit-Reviewer: Anonymous Coward (606) Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Reviewer: wangsheng <[email protected]> Gerrit-Comment-Date: Mon, 03 Aug 2020 16:46:04 +0000 Gerrit-HasComments: Yes
