wangsheng has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16143 )

Change subject: IMPALA-9741: Support querying Iceberg table by impala
......................................................................


Patch Set 17:

(4 comments)

Hi anjalinorwood, thanks for your 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? Th
Yes, you are right, partitioned table is useful for query plan. But we may not 
consider this in our first version, since it's hard to treat Iceberg table as 
an partitioned hdfs table.
But we will definitely do this in next version, including: compute incremental 
stats/query plan optimization for iceberg and so on. This patch is just a 
simple version to scan iceberg table.


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:     }
> The double negative is pretty hard to parse.
Done


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
yes, it is. I've already test by debug, sql like this: select * from table 
where 0=id can also pushdown predicate to iceberg.


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 n
I supported ORC format in original version, but when I test scan iceberg table 
with ORC, I found exception: https://issues.apache.org/jira/browse/IMPALA-9967
So I removed this, maybe supported more file format in other patch.



--
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: 17
Gerrit-Owner: wangsheng <sky...@163.com>
Gerrit-Reviewer: Anonymous Coward (606)
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <borokna...@cloudera.com>
Gerrit-Reviewer: wangsheng <sky...@163.com>
Gerrit-Comment-Date: Tue, 04 Aug 2020 12:40:27 +0000
Gerrit-HasComments: Yes

Reply via email to