Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10611 )

Change subject: IMPALA-7128 (part 1) Refactor interfaces for Db, View, Table, 
Partition
......................................................................


Patch Set 2:

(3 comments)

thanks for the explanations. the only open item is to move away from naming 
based on "hdfs"... I'm in favor of it but if we want to make a pass at that 
sort of thing afterwards, I'm ok with that as well.

http://gerrit.cloudera.org:8080/#/c/10611/2/fe/src/main/java/org/apache/impala/catalog/FeHdfsTable.java
File fe/src/main/java/org/apache/impala/catalog/FeHdfsTable.java:

http://gerrit.cloudera.org:8080/#/c/10611/2/fe/src/main/java/org/apache/impala/catalog/FeHdfsTable.java@36
PS2, Line 36: Note: despite the "HDFS" nomenclature, this table type is also 
used for
            :  * interacting with S3 or other Hadoop-compatible filesystems.
> Done
yes, I think its preferable to just do this now as additions are being made.


http://gerrit.cloudera.org:8080/#/c/10611/2/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java
File fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java:

http://gerrit.cloudera.org:8080/#/c/10611/2/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java@964
PS2, Line 964: KeyValueComparator
> Partitions are used in Collections.sort, which requires the class to be com
thanks for the explanation, makes sense.


http://gerrit.cloudera.org:8080/#/c/10611/2/fe/src/test/java/org/apache/impala/catalog/CatalogObjectToFromThriftTest.java
File 
fe/src/test/java/org/apache/impala/catalog/CatalogObjectToFromThriftTest.java:

http://gerrit.cloudera.org:8080/#/c/10611/2/fe/src/test/java/org/apache/impala/catalog/CatalogObjectToFromThriftTest.java@220
PS2, Line 220: HdfsPartition
nit: add a todo for the downcast.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id55f7d2e94d81e66ce720acb6315f15a89621b31
Gerrit-Change-Number: 10611
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon <t...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-Reviewer: Vuk Ercegovac <vercego...@cloudera.com>
Gerrit-Comment-Date: Thu, 07 Jun 2018 23:46:03 +0000
Gerrit-HasComments: Yes

Reply via email to