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

Change subject: IMPALA-8504: Support CREATE TABLE statement with Kudu/HMS 
integration
......................................................................


Patch Set 8:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/13318/7/fe/src/main/java/org/apache/impala/catalog/KuduTable.java
File fe/src/main/java/org/apache/impala/catalog/KuduTable.java:

http://gerrit.cloudera.org:8080/#/c/13318/7/fe/src/main/java/org/apache/impala/catalog/KuduTable.java@171
PS7, Line 171: Error determining if Kudu's integration wi
> I find this awkwardly worded, maybe 'Error determining if'
Done


http://gerrit.cloudera.org:8080/#/c/13318/7/fe/src/main/java/org/apache/impala/catalog/KuduTable.java@173
PS7, Line 173:
> unnecessary
Done


http://gerrit.cloudera.org:8080/#/c/13318/7/fe/src/test/java/org/apache/impala/analysis/AuditingTest.java
File fe/src/test/java/org/apache/impala/analysis/AuditingTest.java:

http://gerrit.cloudera.org:8080/#/c/13318/7/fe/src/test/java/org/apache/impala/analysis/AuditingTest.java@56
PS7, Line 56:         new TAccessEvent("functional.alltypes", 
TCatalogObjectType.TABLE, "SELECT")
> Probably better to leave off unrelated formatting changes like this. It mak
Good point, though I have to change the lines below due to they exceeds the 
line length limit(90).


http://gerrit.cloudera.org:8080/#/c/13318/7/fe/src/test/java/org/apache/impala/analysis/AuditingTest.java@212
PS7, Line 212:     Assert.assertEquals(accessEvents, Sets.newHashSet(
> Why this change?
Good catch, accidentally changed this.


http://gerrit.cloudera.org:8080/#/c/13318/7/fe/src/test/java/org/apache/impala/customcluster/CustomClusterRunner.java
File fe/src/test/java/org/apache/impala/customcluster/CustomClusterRunner.java:

http://gerrit.cloudera.org:8080/#/c/13318/7/fe/src/test/java/org/apache/impala/customcluster/CustomClusterRunner.java@49
PS7, Line 49:
> This should be removed
Done


http://gerrit.cloudera.org:8080/#/c/13318/7/fe/src/test/java/org/apache/impala/customservice/CustomServiceRunner.java
File fe/src/test/java/org/apache/impala/customservice/CustomServiceRunner.java:

http://gerrit.cloudera.org:8080/#/c/13318/7/fe/src/test/java/org/apache/impala/customservice/CustomServiceRunner.java@32
PS7, Line 32: minicluster
> minicluster component
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I465673d749221bd5f3772814b1c22c2673a53f5c
Gerrit-Change-Number: 13318
Gerrit-PatchSet: 8
Gerrit-Owner: Hao Hao <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Andrew Wong <[email protected]>
Gerrit-Reviewer: Grant Henke <[email protected]>
Gerrit-Reviewer: Hao Hao <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Mike Percy <[email protected]>
Gerrit-Reviewer: Thomas Marshall <[email protected]>
Gerrit-Comment-Date: Wed, 29 May 2019 21:04:13 +0000
Gerrit-HasComments: Yes

Reply via email to